Re: [PATCH v10 tip/sched/core 1/6] sched: Annotate rq->rd with __rcu and update lockless readers
From: K Prateek Nayak
Date: Fri Oct 30 2026 - 00:41:39 EST
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@xxxxxxxxxxx>
> ---
> 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)