Messages in this thread | | | Date | Thu, 14 Sep 2023 18:43:01 +0800 | From | Chen Yu <> | Subject | Re: [RFC PATCH 2/2] sched/fair: skip the cache hot CPU in select_idle_cpu() |
| |
Hi Prateek,
On 2023-09-14 at 11:00:02 +0530, K Prateek Nayak wrote: > Hello Chenyu, > > One question ... > > On 9/11/2023 8:20 AM, Chen Yu wrote: > > [..snip..] > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > > index e20f50726ab8..fe3b760c9654 100644 > > --- a/kernel/sched/fair.c > > +++ b/kernel/sched/fair.c > > [..more snip..] > > @@ -7052,10 +7072,14 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu > > int cpu; > > > > for_each_cpu(cpu, cpu_smt_mask(core)) { > > - if (!available_idle_cpu(cpu)) { > > + bool cache_hot = sched_feat(SIS_CACHE) ? > > + sched_clock_cpu(cpu) < cpu_rq(cpu)->cache_hot_timeout : false; > > + > > + if (!available_idle_cpu(cpu) || cache_hot) { > > idle = false; > > if (*idle_cpu == -1) { > > - if (sched_idle_cpu(cpu) && cpumask_test_cpu(cpu, p->cpus_ptr)) { > > + if (sched_idle_cpu(cpu) && cpumask_test_cpu(cpu, p->cpus_ptr) && > > + !cache_hot) { > > Here, the CPU is running a SCHED_IDLE task ... > > > *idle_cpu = cpu; > > break; > > } > > ... but just below this, there are following lines to cache the idle_cpu: > > } > if (*idle_cpu == -1 && cpumask_test_cpu(cpu, p->cpus_ptr)) > *idle_cpu = cpu; > > Would it make sense to also add the same "cache_hot" check here when we > come across an idle CPU during the search for an idle core? Something > like: > > - if (*idle_cpu == -1 && cpumask_test_cpu(cpu, p->cpus_ptr))
When we reached above code, the following condition should be true: (available_idle_cpu(cpu) && !cache_hot) because the previous 'if' statement is false. So I guess we already has !cache_hot ?
> + if (*idle_cpu == -1 && !cache_hot && cpumask_test_cpu(cpu, p->cpus_ptr)) > *idle_cpu = cpu; > > Implications with the above change: > > If the entire core is idle, "select_idle_core()" will return the core > and the search will bail out in "select_idle_cpu()". Otherwise, the > cache-hot idle CPUs encountered during the search for idle core will be > ignored now and if "idle_cpu" is not -1, it contains an idle CPU that is > not cache-hot. > > Thoughts? >
Yes, agree, we want to skip the cache-hot idle CPU if that entire core is not idle in your case.
thanks, Chenyu
| |