Messages in this thread | | | From | Chen Yu <> | Date | Thu, 4 Aug 2022 17:59:59 +0800 | Subject | Re: [PATCH 1/5] sched/fair: ignore SIS_UTIL when has idle core |
| |
On Thu, Jul 14, 2022 at 3:11 PM Abel Wu <wuyun.abel@bytedance.com> wrote: > > > On 7/14/22 2:19 PM, Yicong Yang Wrote: > > On 2022/7/12 16:20, Abel Wu wrote: > >> When SIS_UTIL is enabled, SIS domain scan will be skipped if > >> the LLC is overloaded. Since the overloaded status is checked > >> in the load balancing at LLC level, the interval is llc_size > >> miliseconds. The duration might be long enough to affect the > >> overall system throughput if idle cores are out of reach in > >> SIS domain scan. > >> > >> Signed-off-by: Abel Wu <wuyun.abel@bytedance.com> > >> --- > >> kernel/sched/fair.c | 15 +++++++++------ > >> 1 file changed, 9 insertions(+), 6 deletions(-) > >> > >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > >> index a78d2e3b9d49..cd758b3616bd 100644 > >> --- a/kernel/sched/fair.c > >> +++ b/kernel/sched/fair.c > >> @@ -6392,16 +6392,19 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool > >> struct sched_domain *this_sd; > >> u64 time = 0; > >> > >> - this_sd = rcu_dereference(*this_cpu_ptr(&sd_llc)); > >> - if (!this_sd) > >> - return -1; > >> - > >> cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr); > >> > >> - if (sched_feat(SIS_PROP) && !has_idle_core) { > >> + if (has_idle_core) > >> + goto scan; > >> + > >> + if (sched_feat(SIS_PROP)) { > >> u64 avg_cost, avg_idle, span_avg; > >> unsigned long now = jiffies; > >> > >> + this_sd = rcu_dereference(*this_cpu_ptr(&sd_llc)); > >> + if (!this_sd) > >> + return -1; > >> + > > > > I don't follow the change here. True that this_sd is used only in SIS_PROP, but it seems irrelevant with your > > commit. Does the position of this make any performance difference? > > No, this change doesn't make much difference to performance. Are > you suggesting that I should make this a separate patch? > I took a look at this patch again before I start a OLTP test. I thought the position change of dereference sd_llc might not be closely connected with current change as Yicong mentioned. Besides, after moving the dereference inside SIS_PROP, we might do cpumask_and() no matter whether sd_llc is valid or not, which might be of extra cost?
thanks, Chenyu
> Thanks, > Abel > > > > > Thanks. > > > >> /* > >> * If we're busy, the assumption that the last idle period > >> * predicts the future is flawed; age away the remaining > >> @@ -6436,7 +6439,7 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool > >> return -1; > >> } > >> } > >> - > >> +scan: > >> for_each_cpu_wrap(cpu, cpus, target + 1) { > >> if (has_idle_core) { > >> i = select_idle_core(p, cpu, cpus, &idle_cpu); > >>
-- Thanks, Chenyu
| |