Re: [PATCH 1/6] sched/fair: Use guard(rcu) for sched_domain RCU sections

[Andrea Righi <arighi@nvidia.com>]

Hi Prateek,

On Tue, Apr 28, 2026 at 02:03:59PM +0530, K Prateek Nayak wrote:
> Hello Andrea,
> 
> On 4/28/2026 10:46 AM, Andrea Righi wrote:
> > Use the scoped guard(rcu)() helper to safely access sched_domain
> > pointers.
> > 
> > No functional change intended, this is preparation for topology work
> > where sched_domain lifetimes are easier to reason about with explicit,
> > scope-bounded RCU critical sections.
> > 
> > Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
> > Signed-off-by: Andrea Righi <arighi@nvidia.com>
> > ---
> >  kernel/sched/fair.c | 141 ++++++++++++++++++++++----------------------
> >  1 file changed, 71 insertions(+), 70 deletions(-)
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 69361c63353ad..fc0828150c780 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -8083,6 +8083,8 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
> >  	 */
> >  	lockdep_assert_irqs_disabled();
> >  
> > +	guard(rcu)();
> 
> Since IRQs are disabled, we don't need an addition RCU read lock here.
> See a03fee333a2f ("sched/fair: Remove superfluous rcu_read_lock()")

Ack.

> 
> > +
> >  	if (choose_idle_cpu(target, p) &&
> >  	    asym_fits_cpu(task_util, util_min, util_max, target))
> >  		return target;
> > @@ -12701,55 +12703,16 @@ static void kick_ilb(unsigned int flags)
> >  }
> >  
> >  /*
> > - * Current decision point for kicking the idle load balancer in the presence
> > - * of idle CPUs in the system.
> > + * Decide whether the ILB needs a stats and/or balance kick based on
> > + * sched_domain state.
> >   */
> > -static void nohz_balancer_kick(struct rq *rq)
> > +static bool nohz_balancer_needs_kick(struct rq *rq)
> >  {
> > -	unsigned long now = jiffies;
> >  	struct sched_domain_shared *sds;
> >  	struct sched_domain *sd;
> >  	int nr_busy, i, cpu = rq->cpu;
> > -	unsigned int flags = 0;
> > -
> > -	if (unlikely(rq->idle_balance))
> > -		return;
> > -
> > -	/*
> > -	 * We may be recently in ticked or tickless idle mode. At the first
> > -	 * busy tick after returning from idle, we will update the busy stats.
> > -	 */
> > -	nohz_balance_exit_idle(rq);
> > -
> > -	if (READ_ONCE(nohz.has_blocked_load) &&
> > -	    time_after(now, READ_ONCE(nohz.next_blocked)))
> > -		flags = NOHZ_STATS_KICK;
> > -
> > -	/*
> > -	 * Most of the time system is not 100% busy. i.e nohz.nr_cpus > 0
> > -	 * Skip the read if time is not due.
> > -	 *
> > -	 * If none are in tickless mode, there maybe a narrow window
> > -	 * (28 jiffies, HZ=1000) where flags maybe set and kick_ilb called.
> > -	 * But idle load balancing is not done as find_new_ilb fails.
> > -	 * That's very rare. So read nohz.nr_cpus only if time is due.
> > -	 */
> > -	if (time_before(now, nohz.next_balance))
> > -		goto out;
> >  
> > -	/*
> > -	 * None are in tickless mode and hence no need for NOHZ idle load
> > -	 * balancing
> > -	 */
> > -	if (unlikely(cpumask_empty(nohz.idle_cpus_mask)))
> > -		return;
> > -
> > -	if (rq->nr_running >= 2) {
> > -		flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
> > -		goto out;
> > -	}
> > -
> > -	rcu_read_lock();
> > +	guard(rcu)();
> 
> and since this is only called from:
> 
>   sched_tick() /* IRQs disabled */
>     sched_balance_trigger()
>       nohz_balancer_kick()
> 
> with IRQs disabled, we can get rid of that rcu_read_lock() entirely.

Yeah, all makes sense. I'll update the patch dropping rcu_read_lock/unlock()
completely.

Is it worth adding a lockdep_assert_irqs_disabled()?

Thanks,
-Andrea