[PATCH] sched/fair: Restore RCU read lock in set_cpu_sd_state_{busy,idle}()

[Andrea Righi]

Commit c9d93a73ce87 ("sched/fair: Drop redundant RCU read lock in NOHZ
kick path") removed the rcu_read_lock()/unlock() pair from
set_cpu_sd_state_busy() and set_cpu_sd_state_idle() on the assumption
that all callers run in a safe context for rcu_dereference_all(): IRQs
disabled or cpus_write_lock() held.

That assumption is wrong for the CPU hotplug teardown path. When CPUs
are taken offline, set_cpu_sd_state_busy() is invoked via:

 cpuhp/N kthread
   cpuhp_thread_fun()
     cpuhp_invoke_callback()
       sched_cpu_deactivate()
         nohz_balance_exit_idle()
           set_cpu_sd_state_busy()
             rcu_dereference_all(per_cpu(sd_llc, cpu))

The cpuhp kthread holds cpu_hotplug_lock (percpu-rwsem) but runs with
preemption and IRQs enabled. As a result, lockdep correctly reports a
suspicious RCU usage on CPU offline, e.g.:

  # echo 0 > /sys/devices/system/cpu/cpu1/online

  =============================
  WARNING: suspicious RCU usage
  -----------------------------
  kernel/sched/fair.c:12793 suspicious rcu_dereference_check() usage!
  ...
  2 locks held by cpuhp/1/20:
   #0: (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x42/0x1ae
   #1: (cpuhp_state-down){+.+.}-{0:0}, at: cpuhp_thread_fun+0x72/0x1ae

  Call Trace:
    lockdep_rcu_suspicious
    nohz_balance_exit_idle
    sched_cpu_deactivate
    cpuhp_invoke_callback
    cpuhp_thread_fun
    smpboot_thread_fn

Restore the rcu_read_lock()/unlock() pair in both helpers;
nohz_balancer_kick() is left as is, since its IRQ-disabled context is
genuinely sufficient.

Fixes: c9d93a73ce87 ("sched/fair: Drop redundant RCU read lock in NOHZ kick path")
Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
Closes: https://lore.kernel.org/all/38fe0a1d-1a48-435a-910a-c278024d9ac9@xxxxxxxxxxx/
Signed-off-by: Andrea Righi <arighi@xxxxxxxxxx>
---
 kernel/sched/fair.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index aac24cfddecdf..381f378db71fa 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -14070,6 +14070,8 @@ static void nohz_balancer_kick(struct rq *rq)
 static void set_cpu_sd_state_busy(int cpu)
 {
 	struct sched_domain *sd;
+
+	rcu_read_lock();
 	sd = rcu_dereference_all(per_cpu(sd_llc, cpu));
 
 	/*
@@ -14077,10 +14079,12 @@ static void set_cpu_sd_state_busy(int cpu)
 	 * domain has no shared object there is nothing to clear or account.
 	 */
 	if (!sd || !sd->shared || !sd->nohz_idle)
-		return;
+		goto unlock;
 	sd->nohz_idle = 0;
 
 	atomic_inc(&sd->shared->nr_busy_cpus);
+unlock:
+	rcu_read_unlock();
 }
 
 void nohz_balance_exit_idle(struct rq *rq)
@@ -14099,14 +14103,18 @@ void nohz_balance_exit_idle(struct rq *rq)
 static void set_cpu_sd_state_idle(int cpu)
 {
 	struct sched_domain *sd;
+
+	rcu_read_lock();
 	sd = rcu_dereference_all(per_cpu(sd_llc, cpu));
 
 	/* See set_cpu_sd_state_busy(): nohz_idle is only used with sd->shared. */
 	if (!sd || !sd->shared || sd->nohz_idle)
-		return;
+		goto unlock;
 	sd->nohz_idle = 1;
 
 	atomic_dec(&sd->shared->nr_busy_cpus);
+unlock:
+	rcu_read_unlock();
 }
 
 /*

base-commit: b07a332d9cbbd9cc9cfa923a21bd061cfb69bea5
-- 
2.54.0