Re: [PATCH v7 1/6] sched: Annotate rq->rd with __rcu and update lockless readers

[Aaron Tomlin]

On Thu, Aug 27, 2026 at 09:30:36AM +0200, Peter Zijlstra wrote:
> On Wed, Aug 26, 2026 at 06:42:33PM -0400, Aaron Tomlin wrote:
> > The root_domain pointer rd field in struct rq is updated dynamically
> > using RCU, and its memory reclamation is deferred via call_rcu() in
> > rq_attach_root(). However, struct rq's rd field was missing the __rcu
> > compiler annotation, and several lockless readers across the scheduler
> > subsystem accessed rq->rd directly without using RCU dereference
> > primitives.
> > 
> > Add the __rcu annotation to struct rq's rd field in kernel/sched/sched.h.
> > Update lockless readers across kernel/sched/ to use rcu_dereference(),
> > rcu_dereference_sched() or rcu_access_pointer() appropriately. This
> > ensures proper data-dependency barriers on all architectures, enables
> > Sparse static analysis validation, and documents RCU read-side ownership
> > contracts.
> > 
> > Signed-off-by: Aaron Tomlin <atomlin@xxxxxxxxxxx>
> > ---
> >  kernel/sched/core.c     | 24 ++++++++++-----
> >  kernel/sched/deadline.c | 67 +++++++++++++++++++++++------------------
> >  kernel/sched/fair.c     | 29 +++++++++---------
> >  kernel/sched/rt.c       | 64 ++++++++++++++++++++++-----------------
> >  kernel/sched/sched.h    |  2 +-
> >  kernel/sched/topology.c | 11 ++++---
> >  6 files changed, 113 insertions(+), 84 deletions(-)
> > 
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 2e7cde033a31..86de58f5825b 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -8547,8 +8547,10 @@ void set_rq_online(struct rq *rq)
> >  {
> >  	if (!rq->online) {
> >  		const struct sched_class *class;
> > +		struct root_domain *rd;
> >  
> > -		cpumask_set_cpu(rq->cpu, rq->rd->online);
> > +		rd = rcu_dereference_protected(rq->rd, lockdep_is_held(&rq->__lock));
> 
> This seems wrong; rcu_dereference_protected() is only supposed to be
> used during the update. And rq->lock is very much not the update side
> lock of the topology.
> 
> Also, &rq->__lock is wrong.
> 
> And this is far too verbose to endlessly repeat.

Hi Peter,

Indeed. Thank you for pointing this out.

    1.  sched_domains_mutex (not rq->lock) is the update lock for topology
        and root domains; acquired in partition_sched_domains() before the
        call to partition_sched_domains_locked()

    2.  Directly referencing &rq->__lock breaks the rq_lock abstraction

    3.  Repeating verbose RCU dereference boilerplate everywhere clutters
        the code

To solve this cleanly, I propose introducing a concise helper, for the root
domain in kernel/sched/sched.h mirroring rcu_dereference_sched_domain(p):

    #define rcu_dereference_root_domain(p) \
        rcu_dereference_all_check((p), lockdep_is_held(&sched_domains_mutex))

This covers both the update side (under sched_domains_mutex) and read-side
contexts (under rq_lock(), preemption-disabled, or RCU read locks) via
rcu_read_lock_any_held(), while keeping the call sites clean and
eliminating Sparse warnings.

Would you prefer rcu_dereference_root_domain(p)?


Kind regards,
-- 
Aaron Tomlin

Attachment: signature.asc
Description: PGP signature