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

[Shrikanth Hegde <sshegde@linux.ibm.com>]

On 4/21/25 11:24, K Prateek Nayak wrote:
> 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@amd.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.
> 

Yes, that's the reason you didn't run into.
for me, CONFIG_CPUMASK_OFFSTACK is not set.

>>
>> 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.

Or you need to use like below. This also works (Not tested on x86)

struct cpumask*  overloaded_mask;

> 
> 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)
>