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

Andrea Righi
2026-04-28T10:43:35Z

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