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

[Shrikanth Hegde]

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

That would give us the get/put routines for all cases.