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

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Aaron,

On 10/30/2026 7:08 AM, 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. For code clarity,
> introduce the rcu_dereference_root_domain() helper macro to validate
> access under sched_domains_mutex or active RCU-sched read-side critical
> sections. While mechanically identical to
> rcu_dereference_sched_domain(), defining rcu_dereference_root_domain()
> preserves the natural symmetry of rq->rd and rq->sd in struct rq, while
> keeping grep-ability straightforward. Update readers and accessors
> across kernel/sched/ to use rcu_dereference_root_domain().
> 
> This ensures proper memory ordering, enables Sparse static analysis
> validation, and avoids false Lockdep warnings under CONFIG_PROVE_RCU.
> 
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
>  kernel/sched/core.c     | 24 ++++++++-----
>  kernel/sched/deadline.c | 77 ++++++++++++++++++++++-------------------
>  kernel/sched/fair.c     | 27 +++++++++------
>  kernel/sched/rt.c       | 64 +++++++++++++++++++---------------
>  kernel/sched/sched.h    |  7 ++--
>  kernel/sched/syscalls.c |  8 ++---
>  kernel/sched/topology.c | 13 +++----
>  7 files changed, 127 insertions(+), 93 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index d7c3b43158de..4ead07fd0f6d 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8542,8 +8542,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_root_domain(rq->rd);

nit. Using rcu_dereference_root_domain() everywhere is a bit mouthful.
How about a couple helpers like like:

#define rq_rd(rq)	rcu_dereference_root_domain(rq->rd)
#define cpu_rd(cpu)	rcu_dereference_root_domain(cpu_rq(cpu)->rd)

... and those makes the usage much shorter. Thoughts?

I've attached a coccinelle script at the end that I used to do the
conversion.

> +		cpumask_set_cpu(rq->cpu, rd->online);
>  		rq->online = 1;
>  
>  		for_each_class(class) {
> @@ -8557,6 +8559,7 @@ void set_rq_offline(struct rq *rq)
>  {
>  	if (rq->online) {
>  		const struct sched_class *class;
> +		struct root_domain *rd;
>  
>  		update_rq_clock(rq);
>  		for_each_class(class) {
> @@ -8564,7 +8567,8 @@ void set_rq_offline(struct rq *rq)
>  				class->rq_offline(rq);
>  		}
>  
> -		cpumask_clear_cpu(rq->cpu, rq->rd->online);
> +		rd = rcu_dereference_root_domain(rq->rd);

nit. If there is no rq_lock() / rcu dependnecy, maybe you can
initialize rd up top where you declare it for.

> +		cpumask_clear_cpu(rq->cpu, rd->online);
>  		rq->online = 0;
>  	}
>  }
> @@ -8572,10 +8576,12 @@ void set_rq_offline(struct rq *rq)
>  static inline void sched_set_rq_online(struct rq *rq, int cpu)
>  {
>  	struct rq_flags rf;
> +	struct root_domain *rd;

nit. reverse x-mas declaration style would be better IMO.

>  
>  	rq_lock_irqsave(rq, &rf);

... and since you are touching these bits, maybe you can also
covert these to guard(rq_lock_irqsave)(rq) and save on needing to
have explicit rq_flags.

> -	if (rq->rd) {
> -		BUG_ON(!cpumask_test_cpu(cpu, rq->rd->span));
> +	rd = rcu_dereference_root_domain(rq->rd);
> +	if (rd) {
> +		BUG_ON(!cpumask_test_cpu(cpu, rd->span));
>  		set_rq_online(rq);
>  	}
>  	rq_unlock_irqrestore(rq, &rf);
> @@ -8584,10 +8590,12 @@ static inline void sched_set_rq_online(struct rq *rq, int cpu)
>  static inline void sched_set_rq_offline(struct rq *rq, int cpu)
>  {
>  	struct rq_flags rf;
> +	struct root_domain *rd;
>  
>  	rq_lock_irqsave(rq, &rf);
> -	if (rq->rd) {
> -		BUG_ON(!cpumask_test_cpu(cpu, rq->rd->span));
> +	rd = rcu_dereference_root_domain(rq->rd);
> +	if (rd) {
> +		BUG_ON(!cpumask_test_cpu(cpu, rd->span));
>  		set_rq_offline(rq);
>  	}
>  	rq_unlock_irqrestore(rq, &rf);
> @@ -9009,8 +9017,8 @@ void __init sched_init(void)
>  #endif
>  		rq->next_class = &idle_sched_class;
>  
> -		rq->sd = NULL;
> -		rq->rd = NULL;
> +		RCU_INIT_POINTER(rq->sd, NULL);
> +		RCU_INIT_POINTER(rq->rd, NULL);
>  		rq->cpu_capacity = SCHED_CAPACITY_SCALE;
>  		rq->balance_callback = &balance_push_callback;
>  		rq->active_balance = 0;
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index c0ebdcde5fe5..2dcb3722c317 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -122,12 +122,12 @@ static inline struct dl_bw *dl_bw_of(int i)
>  {
>  	RCU_LOCKDEP_WARN(!rcu_read_lock_sched_held(),
>  			 "sched RCU must be held");

Do we need this warbing given we are using rcu_dereference_all_check()
which checks for RCU too?

> -	return &cpu_rq(i)->rd->dl_bw;
> +	return &rcu_dereference_root_domain(cpu_rq(i)->rd)->dl_bw;
>  }
>  
>  static inline int dl_bw_cpus(int i)
>  {
> -	struct root_domain *rd = cpu_rq(i)->rd;
> +	struct root_domain *rd = rcu_dereference_root_domain(cpu_rq(i)->rd);
>  
>  	RCU_LOCKDEP_WARN(!rcu_read_lock_sched_held(),
>  			 "sched RCU must be held");

this warning may not be required either.

[..snip..]

> @@ -2167,6 +2167,9 @@ queue_balance_callback(struct rq *rq,
>  	rq->balance_callback = head;
>  }
>  
> +#define rcu_dereference_root_domain(p) \

                                       ^
This argument here can be 'rd' for better clarity
                                   v

> +	rcu_dereference_all_check((p), lockdep_is_held(&sched_domains_mutex))
> +
>  #define rcu_dereference_sched_domain(p) \
>  	rcu_dereference_all_check((p), lockdep_is_held(&sched_domains_mutex))
>  

-- 
Thanks and Regards,
Prateek

Assisted-by: LLM

root_domain_helpers.cocci
--------------------------
// SPDX-License-Identifier: GPL-2.0-only
/// Replace open-coded rcu_dereference_root_domain() with rq_rd()/cpu_rd().
///
///   rcu_dereference_root_domain(cpu_rq(cpu)->rd)  ->  cpu_rd(cpu)
///   rcu_dereference_root_domain(rq->rd)           ->  rq_rd(rq)
//
// Confidence: High
// Options: --include-headers

virtual patch
virtual report

@initialize:python@
@@
import re
import linecache

HELPER_DEF = re.compile(r'^\s*#\s*define\s+(rq_rd|cpu_rd)\b')

def in_helper_define(p):
    """True if the match is inside the #define of rq_rd/cpu_rd
    (handles backslash-continued macro bodies too)."""
    f, n = p[0].file, int(p[0].line)
    while n > 1 and linecache.getline(f, n - 1).rstrip().endswith('\\'):
        n -= 1
    return bool(HELPER_DEF.match(linecache.getline(f, n)))

// ---------------------------------------------------------------
// Rule 1: cpu_rq(cpu)->rd  ->  cpu_rd(cpu)   (must run before rule 2)
// ---------------------------------------------------------------
@cpu_m@
expression cpu;
position p;
@@
rcu_dereference_root_domain@p(cpu_rq(cpu)->rd)

@script:python cpu_filter@
p << cpu_m.p;
@@
if in_helper_define(p):
    cocci.include_match(False)

@cpu_fix depends on patch@
expression cpu;
position cpu_m.p;
@@
- rcu_dereference_root_domain@p(cpu_rq(cpu)->rd)
+ cpu_rd(cpu)

// ---------------------------------------------------------------
// Rule 2: rq->rd  ->  rq_rd(rq)
// ---------------------------------------------------------------
@rq_m@
expression rq;
position p;
@@
rcu_dereference_root_domain@p(rq->rd)

@script:python rq_filter@
p << rq_m.p;
@@
if in_helper_define(p):
    cocci.include_match(False)

@rq_fix depends on patch@
expression rq;
position rq_m.p;
@@
- rcu_dereference_root_domain@p(rq->rd)
+ rq_rd(rq)

// ---------------------------------------------------------------
// Report mode
// ---------------------------------------------------------------
@script:python depends on report@
p << cpu_m.p;
@@
coccilib.report.print_report(p[0], "use cpu_rd() helper")

@script:python depends on report@
p << rq_m.p;
@@
coccilib.report.print_report(p[0], "use rq_rd() helper")
---

Build and boot tested after applying with:

spatch --python python3 --sp-file root_domain_helpers.cocci -D patch \
       --dir kernel/sched --include-headers --in-place -j $(nproc)