Re: [PATCH 1/2] sched/fair: consider hk_mask early in triggering ilb

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

On 3/20/26 7:42 PM, Shrikanth Hegde wrote:
> 
> 
> On 3/20/26 5:13 PM, Peter Zijlstra wrote:
>> On Fri, Mar 20, 2026 at 02:49:30PM +0530, Shrikanth Hegde wrote:
>>>
>>>
>>> On 3/20/26 9:07 AM, K Prateek Nayak wrote:
>>>> Hello Shrikanth,
>>>>
>>>> On 3/19/2026 12:23 PM, Shrikanth Hegde wrote:
>>>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>>>> index b19aeaa51ebc..02cca2c7a98d 100644
>>>>> --- a/kernel/sched/fair.c
>>>>> +++ b/kernel/sched/fair.c
>>>>> @@ -7392,6 +7392,7 @@ static inline unsigned int 
>>>>> cfs_h_nr_delayed(struct rq *rq)
>>>>>    static DEFINE_PER_CPU(cpumask_var_t, load_balance_mask);
>>>>>    static DEFINE_PER_CPU(cpumask_var_t, select_rq_mask);
>>>>>    static DEFINE_PER_CPU(cpumask_var_t, should_we_balance_tmpmask);
>>>>> +static DEFINE_PER_CPU(cpumask_var_t, kick_ilb_tmpmask);
>>>>
>>>> nit. We can rename and reuse select_rq_mask. Wakeups happen with IRQs
>>>> disabled and kick happens from the hrtimer handler so it should be safe
>>>> to reuse that and save some space.
>>>>
>>>> Thoughts?
>>>
>>> May be. but it could be a confusing name. sched_tmpmask?
>>>
>>> We could similar stuff already to load_balance_mask, select_rq_mask.
>>> So, i would prefer to keep it separate.
>>
>> But then we keep growing this ad infinitum.
>>
>> The more sensible option is to name them after the context and have
>> get/put accessors that (for PROVE_LOCKING builds or so) verify the
>> context and maybe even 'lock' them to make sure nobody is trying to use
>> one for two things at the same time.
>>


Maybe we can define one cpumask which all irq disabled section could use?
i.e for try_to_wake_up and nohz_balancer_kick?

Is that what you meant?

>> That should make it clearer whats what and improve reuse, no?
>>
> 
> We have these:
> 
> deadline.c:static DEFINE_PER_CPU(cpumask_var_t, local_cpu_mask_dl);
> ext_idle.c:static DEFINE_PER_CPU(cpumask_var_t, local_idle_cpumask);
> ext_idle.c:static DEFINE_PER_CPU(cpumask_var_t, local_llc_idle_cpumask);
> ext_idle.c:static DEFINE_PER_CPU(cpumask_var_t, local_numa_idle_cpumask);
> fair.c:static DEFINE_PER_CPU(cpumask_var_t, load_balance_mask);
> fair.c:static DEFINE_PER_CPU(cpumask_var_t, select_rq_mask);
> fair.c:static DEFINE_PER_CPU(cpumask_var_t, should_we_balance_tmpmask);
> rt.c:static DEFINE_PER_CPU(cpumask_var_t, local_cpu_mask)
> 
> Here,
> 1. load_balance_mask and should_we_balance_tmpmask are used in the same 
> context
>     all it cares is - I need two cpumask to play with.
> 
> 2. local_cpu_mask, local_cpu_mask_dl, select_rq_mask are used in 
> independent context - all it cares is
>     all it cares is - I need one cpumask to play with.
> 
> 3. ext_idle.c: local_* are used the same context.
>     all it cares is - I need three cpumask to play with
>     technically it is doable with one.(but that's a separate story)
> 
> 1 and 2 are in interrupt disable section, 3 i am not completely sure.
> 
> 
> I am wondering something like would make sense?
> 
> 
> DEFINE_PER_CPU(cpumask_var_t, sched_tmp_cpumask1);
> DEFINE_PER_CPU(cpumask_var_t, sched_tmp_cpumask2);
> DEFINE_PER_CPU(cpumask_var_t, sched_tmp_cpumask3);
> 
> Request for a tmp cpumask with number?
> i.e 2 would say sched_request_tmpmask(0)
>      1 would sat sched_request_tmpmask(0), and then 
> sched_request_tmpmask(1)
>      3 would say sched_request_tmpmask(0), sched_request_tmpmask(1) and 
> sched_request_tmpmask(2)
> 
> Do this if interrupts are disabled.
> if interrupt are enabled, then maybe do allocation/free instead?
> 

1 is in softirq. It need not have interrupts disabled.
So doing allocation there is too costly.