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

[Aaron Tomlin]

On Thu, Aug 27, 2026 at 08:54:18AM +0200, Vincent Guittot wrote:
> On Thu, 27 Aug 2026 at 00:42, Aaron Tomlin <atomlin@xxxxxxxxxxx> 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));
> > +               cpumask_set_cpu(rq->cpu, rd->online);
> >                 rq->online = 1;
> >
> >                 for_each_class(class) {
> > @@ -8562,6 +8564,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) {
> > @@ -8569,7 +8572,8 @@ void set_rq_offline(struct rq *rq)
> >                                 class->rq_offline(rq);
> >                 }
> >
> > -               cpumask_clear_cpu(rq->cpu, rq->rd->online);
> > +               rd = rcu_dereference_protected(rq->rd, lockdep_is_held(&rq->__lock));
> > +               cpumask_clear_cpu(rq->cpu, rd->online);
> >                 rq->online = 0;
> >         }
> >  }
> > @@ -8577,10 +8581,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;
> >
> >         rq_lock_irqsave(rq, &rf);
> > -       if (rq->rd) {
> > -               BUG_ON(!cpumask_test_cpu(cpu, rq->rd->span));
> > +       rd = rcu_dereference_protected(rq->rd, lockdep_is_held(&rq->__lock));
> > +       if (rd) {
> > +               BUG_ON(!cpumask_test_cpu(cpu, rd->span));
> >                 set_rq_online(rq);
> >         }
> >         rq_unlock_irqrestore(rq, &rf);
> > @@ -8589,10 +8595,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_protected(rq->rd, lockdep_is_held(&rq->__lock));
> > +       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 857dbe3519a8..026d8b499dcf 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");
> > -       return &cpu_rq(i)->rd->dl_bw;
> > +       return &rcu_dereference_sched(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_sched(cpu_rq(i)->rd);
> >
> >         RCU_LOCKDEP_WARN(!rcu_read_lock_sched_held(),
> >                          "sched RCU must be held");
> > @@ -159,13 +159,13 @@ static inline unsigned long dl_bw_capacity(int i)
> >                 RCU_LOCKDEP_WARN(!rcu_read_lock_sched_held(),
> >                                  "sched RCU must be held");
> >
> > -               return __dl_bw_capacity(cpu_rq(i)->rd->span);
> > +               return __dl_bw_capacity(rcu_dereference_sched(cpu_rq(i)->rd)->span);
> >         }
> >  }
> >
> >  bool dl_bw_visited(int cpu, u64 cookie)
> >  {
> > -       struct root_domain *rd = cpu_rq(cpu)->rd;
> > +       struct root_domain *rd = rcu_dereference_sched(cpu_rq(cpu)->rd);
> >
> >         if (rd->visit_cookie == cookie)
> >                 return true;
> > @@ -533,15 +533,18 @@ void init_dl_rq(struct dl_rq *dl_rq)
> >
> >  static inline int dl_overloaded(struct rq *rq)
> >  {
> > -       return atomic_read(&rq->rd->dlo_count);
> > +       return atomic_read(&rcu_dereference_sched(rq->rd)->dlo_count);
> >  }
> >
> >  static inline void dl_set_overload(struct rq *rq)
> >  {
> > +       struct root_domain *rd;
> > +
> >         if (!rq->online)
> >                 return;
> >
> > -       cpumask_set_cpu(rq->cpu, rq->rd->dlo_mask);
> > +       rd = rcu_dereference_sched(rq->rd);
> > +       cpumask_set_cpu(rq->cpu, rd->dlo_mask);
> >         /*
> >          * Must be visible before the overload count is
> >          * set (as in sched_rt.c).
> > @@ -549,16 +552,19 @@ static inline void dl_set_overload(struct rq *rq)
> >          * Matched by the barrier in pull_dl_task().
> >          */
> >         smp_wmb();
> > -       atomic_inc(&rq->rd->dlo_count);
> > +       atomic_inc(&rd->dlo_count);
> >  }
> >
> >  static inline void dl_clear_overload(struct rq *rq)
> >  {
> > +       struct root_domain *rd;
> > +
> >         if (!rq->online)
> >                 return;
> >
> > -       atomic_dec(&rq->rd->dlo_count);
> > -       cpumask_clear_cpu(rq->cpu, rq->rd->dlo_mask);
> > +       rd = rcu_dereference_sched(rq->rd);
> > +       atomic_dec(&rd->dlo_count);
> > +       cpumask_clear_cpu(rq->cpu, rd->dlo_mask);
> >  }
> >
> >  #define __node_2_pdl(node) \
> > @@ -699,14 +705,14 @@ static struct rq *dl_task_offline_migration(struct rq *rq, struct task_struct *p
> >          * since p is still hanging out in the old (now moved to default) root
> >          * domain.
> >          */
> > -       dl_b = &rq->rd->dl_bw;
> > +       dl_b = &rcu_dereference_sched(rq->rd)->dl_bw;
> >         raw_spin_lock(&dl_b->lock);
> > -       __dl_sub(dl_b, p->dl.dl_bw, cpumask_weight(rq->rd->span));
> > +       __dl_sub(dl_b, p->dl.dl_bw, cpumask_weight(rcu_dereference_sched(rq->rd)->span));
> >         raw_spin_unlock(&dl_b->lock);
> >
> > -       dl_b = &later_rq->rd->dl_bw;
> > +       dl_b = &rcu_dereference_sched(later_rq->rd)->dl_bw;
> >         raw_spin_lock(&dl_b->lock);
> > -       __dl_add(dl_b, p->dl.dl_bw, cpumask_weight(later_rq->rd->span));
> > +       __dl_add(dl_b, p->dl.dl_bw, cpumask_weight(rcu_dereference_sched(later_rq->rd)->span));
> >         raw_spin_unlock(&dl_b->lock);
> >
> >         set_task_cpu(p, later_rq->cpu);
> > @@ -2222,9 +2228,9 @@ static void inc_dl_deadline(struct dl_rq *dl_rq, u64 deadline)
> >         if (dl_rq->earliest_dl.curr == 0 ||
> >             dl_time_before(deadline, dl_rq->earliest_dl.curr)) {
> >                 if (dl_rq->earliest_dl.curr == 0)
> > -                       cpupri_set(&rq->rd->cpupri, rq->cpu, CPUPRI_HIGHER);
> > +                       cpupri_set(&rcu_dereference_sched(rq->rd)->cpupri, rq->cpu, CPUPRI_HIGHER);
> >                 dl_rq->earliest_dl.curr = deadline;
> > -               cpudl_set(&rq->rd->cpudl, rq->cpu, deadline);
> > +               cpudl_set(&rcu_dereference_sched(rq->rd)->cpudl, rq->cpu, deadline);
> >         }
> >  }
> >
> > @@ -2239,14 +2245,15 @@ static void dec_dl_deadline(struct dl_rq *dl_rq, u64 deadline)
> >         if (!dl_rq->dl_nr_running) {
> >                 dl_rq->earliest_dl.curr = 0;
> >                 dl_rq->earliest_dl.next = 0;
> > -               cpudl_clear(&rq->rd->cpudl, rq->cpu, rq->online);
> > -               cpupri_set(&rq->rd->cpupri, rq->cpu, rq->rt.highest_prio.curr);
> > +               cpudl_clear(&rcu_dereference_sched(rq->rd)->cpudl, rq->cpu, rq->online);
> > +               cpupri_set(&rcu_dereference_sched(rq->rd)->cpupri, rq->cpu,
> > +                          rq->rt.highest_prio.curr);
> >         } else {
> >                 struct rb_node *leftmost = rb_first_cached(&dl_rq->root);
> >                 struct sched_dl_entity *entry = __node_2_dle(leftmost);
> >
> >                 dl_rq->earliest_dl.curr = entry->deadline;
> > -               cpudl_set(&rq->rd->cpudl, rq->cpu, entry->deadline);
> > +               cpudl_set(&rcu_dereference_sched(rq->rd)->cpudl, rq->cpu, entry->deadline);
> >         }
> >  }
> >
> > @@ -2686,12 +2693,14 @@ static void migrate_task_rq_dl(struct task_struct *p, int new_cpu __maybe_unused
> >
> >  static void check_preempt_equal_dl(struct rq *rq, struct task_struct *p)
> >  {
> > +       struct root_domain *rd = rcu_dereference_sched(rq->rd);
> > +
> >         /*
> >          * Current can't be migrated, useless to reschedule,
> >          * let's hope p can move out.
> >          */
> >         if (rq->curr->nr_cpus_allowed == 1 ||
> > -           !cpudl_find(&rq->rd->cpudl, rq->donor, NULL))
> > +           !cpudl_find(&rd->cpudl, rq->donor, NULL))
> >                 return;
> >
> >         /*
> > @@ -2699,7 +2708,7 @@ static void check_preempt_equal_dl(struct rq *rq, struct task_struct *p)
> >          * see if it is pushed or pulled somewhere else.
> >          */
> >         if (p->nr_cpus_allowed != 1 &&
> > -           cpudl_find(&rq->rd->cpudl, p, NULL))
> > +           cpudl_find(&rd->cpudl, p, NULL))
> >                 return;
> >
> >         resched_curr(rq);
> > @@ -2948,7 +2957,7 @@ static int find_later_rq(struct task_struct *task)
> >          * We have to consider system topology and task affinity
> >          * first, then we can look for a suitable CPU.
> >          */
> > -       if (!cpudl_find(&task_rq(task)->rd->cpudl, task, later_mask))
> > +       if (!cpudl_find(&rcu_dereference_sched(task_rq(task)->rd)->cpudl, task, later_mask))
> >                 return -1;
> >
> >         /*
> > @@ -3232,7 +3241,7 @@ static void pull_dl_task(struct rq *this_rq)
> >          */
> >         smp_rmb();
> >
> > -       for_each_cpu(cpu, this_rq->rd->dlo_mask) {
> > +       for_each_cpu(cpu, rcu_dereference_sched(this_rq->rd)->dlo_mask) {
> >                 if (this_cpu == cpu)
> >                         continue;
> >
> > @@ -3358,7 +3367,7 @@ bool dl_task_needs_bw_move(struct task_struct *p,
> >         if (!dl_task(p))
> >                 return false;
> >
> > -       return !cpumask_intersects(task_rq(p)->rd->span, new_mask);
> > +       return !cpumask_intersects(rcu_dereference_sched(task_rq(p)->rd)->span, new_mask);
> >  }
> >
> >  /* Assumes rq->lock is held */
> > @@ -3368,9 +3377,9 @@ static void rq_online_dl(struct rq *rq)
> >                 dl_set_overload(rq);
> >
> >         if (rq->dl.dl_nr_running > 0)
> > -               cpudl_set(&rq->rd->cpudl, rq->cpu, rq->dl.earliest_dl.curr);
> > +               cpudl_set(&rcu_dereference_sched(rq->rd)->cpudl, rq->cpu, rq->dl.earliest_dl.curr);
> >         else
> > -               cpudl_clear(&rq->rd->cpudl, rq->cpu, true);
> > +               cpudl_clear(&rcu_dereference_sched(rq->rd)->cpudl, rq->cpu, true);
> >  }
> >
> >  /* Assumes rq->lock is held */
> > @@ -3379,7 +3388,7 @@ static void rq_offline_dl(struct rq *rq)
> >         if (rq->dl.overloaded)
> >                 dl_clear_overload(rq);
> >
> > -       cpudl_clear(&rq->rd->cpudl, rq->cpu, false);
> > +       cpudl_clear(&rcu_dereference_sched(rq->rd)->cpudl, rq->cpu, false);
> >  }
> >
> >  void __init init_sched_dl_class(void)
> > @@ -3440,10 +3449,10 @@ void dl_add_task_root_domain(struct task_struct *p)
> >         cpu = cpumask_first_and(cpu_active_mask, msk);
> >         BUG_ON(cpu >= nr_cpu_ids);
> >         rq = cpu_rq(cpu);
> > -       dl_b = &rq->rd->dl_bw;
> > +       dl_b = &rcu_dereference_sched(rq->rd)->dl_bw;
> >
> >         raw_spin_lock(&dl_b->lock);
> > -       __dl_add(dl_b, p->dl.dl_bw, cpumask_weight(rq->rd->span));
> > +       __dl_add(dl_b, p->dl.dl_bw, cpumask_weight(rcu_dereference_sched(rq->rd)->span));
> >         raw_spin_unlock(&dl_b->lock);
> >         raw_spin_unlock_irqrestore(&p->pi_lock, rf.flags);
> >  }
> > @@ -3504,7 +3513,7 @@ void dl_clear_root_domain(struct root_domain *rd)
> >
> >  void dl_clear_root_domain_cpu(int cpu)
> >  {
> > -       dl_clear_root_domain(cpu_rq(cpu)->rd);
> > +       dl_clear_root_domain(rcu_dereference_sched(cpu_rq(cpu)->rd));
> >  }
> >
> >  static void switched_from_dl(struct rq *rq, struct task_struct *p)
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index f79fcba4afec..51b28440d05d 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -7865,13 +7865,10 @@ static inline void set_rd_overutilized(struct root_domain *rd, bool flag)
> >
> >  static inline void check_update_overutilized_status(struct rq *rq)
> >  {
> > -       /*
> > -        * overutilized field is used for load balancing decisions only
> > -        * if energy aware scheduler is being used
> > -        */
> 
> There is no reason to remove the comment above

Agreed. That was an unintentional drop while updating the function to use
rcu_dereference_sched().

Kind regards,
-- 
Aaron Tomlin