linux-kernel - Re: [RFC PATCH 3/5] sched/fair: Update overloaded mask in presence of pushable task

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <616187d5-e178-4fa5-a0a2-1509f11d1a37@amd.com>
Date: Mon, 21 Apr 2025 11:24:23 +0530
From: K Prateek Nayak <kprateek.nayak@....com>
To: Shrikanth Hegde <sshegde@...ux.ibm.com>
CC: Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
	Juri Lelli <juri.lelli@...hat.com>, Vincent Guittot
	<vincent.guittot@...aro.org>, <linux-kernel@...r.kernel.org>, "Dietmar
 Eggemann" <dietmar.eggemann@....com>, Steven Rostedt <rostedt@...dmis.org>,
	Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>, "Valentin
 Schneider" <vschneid@...hat.com>, "Gautham R. Shenoy"
	<gautham.shenoy@....com>, Swapnil Sapkal <swapnil.sapkal@....com>
Subject: Re: [RFC PATCH 3/5] sched/fair: Update overloaded mask in presence of
 pushable task

Hello Shrikanth,

On 4/21/2025 10:50 AM, Shrikanth Hegde wrote:
> 
> 
> On 4/9/25 16:45, K Prateek Nayak wrote:
> 
> Hi Prateek. Feel free to cc me in the future versions.


Will do! Thank you for taking a look at the series.

> This seems interesting way if it can get us rid of newidle balance overheads.
> 
>> In presence of pushable tasks on the CPU, set it on the newly introduced
>> "overloaded+mask" in sched_domain_shared struct. This will be used by
>> the newidle balance to limit the scanning to these overloaded CPUs since
>> they contain tasks that could be run on the newly idle target.
>>
>> Signed-off-by: K Prateek Nayak <kprateek.nayak@....com>
>> ---
>>   kernel/sched/fair.c | 24 ++++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 98d3ed2078cd..834fcdd15cac 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -8559,6 +8559,24 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
>>       return target;
>>   }
>> +static inline void update_overloaded_mask(int cpu, bool contains_pushable)
>> +{
>> +    struct sched_domain_shared *sd_share = rcu_dereference(per_cpu(sd_llc_shared, cpu));
>> +    cpumask_var_t overloaded_mask;
>> +
>> +    if (!sd_share)
>> +        return;
>> +
>> +    overloaded_mask = sd_share->overloaded_mask;
>> +    if (!overloaded_mask)
>> +        return;
>> +
>> +    if (contains_pushable)
>> +        cpumask_set_cpu(cpu, overloaded_mask);
>> +    else
>> +        cpumask_clear_cpu(cpu, overloaded_mask);
>> +}
>> +
> 
> I was getting below error when compiling. Noticed that overloaded_mask is a local update and it wouldn't
> update the actual overloaded_mask.

Interesting! Question: Do you have "CONFIG_CPUMASK_OFFSTACK" enabled in
your config? (me makes a note to test this too in the next iteration)
Looking at the arch specific Kconfigs, there is a slight difference in
how this is toggled on x86 vs powerpc and I'm wondering if that is why
I haven't seen this warning in my testing.

> 
> Compilation Error:
> kernel/sched/fair.c: In function ‘update_overloaded_mask’:
> kernel/sched/fair.c:8570:25: error: assignment to expression with array type
>   8570 |         overloaded_mask = sd_share->overloaded_mask;
>        |                         ^
> kernel/sched/fair.c:8571:13: warning: the address of ‘overloaded_mask’ will always evaluate as ‘true’ [-Waddress]
>   8571 |         if (!overloaded_mask)
> 
> 
> 
> Made the below change. Also you would need rcu protection for sd_share i think.

You are right! Thank you for the pointers and spotting my oversight.
Aaron too pointed some shortcomings here. I'll make sure to test
these bits more next time (LOCKDEP, hotplug, and
!CONFIG_CPUMASK_OFFSTACK)

-- 
Thanks and Regards,
Prateek

> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index bca3db5d0ac0..818d4769ec55 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8562,19 +8562,18 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
>   static inline void update_overloaded_mask(int cpu, bool contains_pushable)
>   {
>          struct sched_domain_shared *sd_share = rcu_dereference(per_cpu(sd_llc_shared, cpu));
> -       cpumask_var_t overloaded_mask;
> 
>          if (!sd_share)
>                  return;
> 
> -       overloaded_mask = sd_share->overloaded_mask;
> -       if (!overloaded_mask)
> +       if (!sd_share->overloaded_mask)
>                  return;
> 
> +       guard(rcu)();
>          if (contains_pushable)
> -               cpumask_set_cpu(cpu, overloaded_mask);
> +               cpumask_set_cpu(cpu, sd_share->overloaded_mask);
>          else
> -               cpumask_clear_cpu(cpu, overloaded_mask);
> +               cpumask_clear_cpu(cpu, sd_share->overloaded_mask);
>   }
> 
>>   static inline bool fair_push_task(struct task_struct *p)
>>   {
>>       if (!task_on_rq_queued(p))
>> @@ -8606,11 +8624,17 @@ static inline void fair_queue_pushable_tasks(struct rq *rq)
>>   static void fair_remove_pushable_task(struct rq *rq, struct task_struct *p)
>>   {
>>       plist_del(&p->pushable_tasks, &rq->cfs.pushable_tasks);
>> +
>> +    if (!has_pushable_tasks(rq))
>> +        update_overloaded_mask(rq->cpu, false);
>>   }
>>   static void fair_add_pushable_task(struct rq *rq, struct task_struct *p)
>>   {
>>       if (fair_push_task(p)) {
>> +        if (!has_pushable_tasks(rq))
>> +            update_overloaded_mask(rq->cpu, true);
>> +
>>           plist_del(&p->pushable_tasks, &rq->cfs.pushable_tasks);
>>           plist_node_init(&p->pushable_tasks, p->prio);
>>           plist_add(&p->pushable_tasks, &rq->cfs.pushable_tasks);
>