CPU-hotplug updates for 4.2

Hello!

The following series improves RCU's handling of CPU hotplug. It also contains a couple of ARM placeholder patches in case someone gets caught out by tighter diagnostics.

1. Eliminate a HOTPLUG_CPU #ifdef in favor of IS_ENABLED()

2. Adjust rcu_read_unlock_special()'s acquisition of the
task's lock to start allowing for the fact that blocked tasks are no longer migrated to the root rcu_node structure. If the warning does not trigger, a later patch will eliminate the loop.

3. Rmove dead code from force_qs_rnp().

4. Remove redundant offline check.

5. Correctly initialize ->rcu_qs_ctr_snap at online time to get the
code out of its current "working by accident" state.

6. Move smpboot_unpark_threads() handling into a notifier for
uniformity.

7. [Placeholder for ARM64] Use common code for ARM64 CPU-offline
notification.

8. [Placeholder for ARM] Use common code for ARM CPU-offline
notification.

9. Remove extraneous __cpuinit, courtesy of Paul Gortmaker.

Thanx, Paul

------------------------------------------------------------------------

 b/arch/arm/kernel/smp.c    |    6 +----
 b/arch/arm64/kernel/smp.c  |    6 +----
 b/kernel/cpu.c             |    4 +--
 b/kernel/rcu/tree.c        |   47 ++++++++++++++-------------------------------
 b/kernel/rcu/tree_plugin.h |    9 +++++---
 5 files changed, 27 insertions(+), 45 deletions(-)

From: "Paul E. McKenney" <[email protected]>

This commit removes the open-coded CPU-offline notification with new common code. In particular, this change avoids calling scheduler code using RCU from an offline CPU that RCU is ignoring. This is a minimal change. A more intrusive change might invoke the cpu_check_up_prepare() and cpu_set_state_online() functions at CPU-online time, which would allow onlining throw an error if the CPU did not go offline properly.

---
 arch/arm64/kernel/smp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

index 2cb008177252..a899c1beff54 100644
@@ -253,15 +253,13 @@ static int op_cpu_kill(unsigned int cpu)
 	return cpu_ops[cpu]->cpu_kill(cpu);
 }
 
-static DECLARE_COMPLETION(cpu_died);
-
 /*
  * called on the thread which is asking for a CPU to be shutdown -
  * waits until shutdown has completed, or it is timed out.
  */
 void __cpu_die(unsigned int cpu)
 {
-	if (!wait_for_completion_timeout(&cpu_died, msecs_to_jiffies(5000))) {
+	if (!cpu_wait_death(cpu, 5)) {
 		pr_crit("CPU%u: cpu didn't die\n", cpu);
 		return;
 	}

@@ -294,7 +292,7 @@ void cpu_die(void)
 	local_irq_disable();
 
 	/* Tell __cpu_die() that this CPU is now safe to dispose of */
-	complete(&cpu_died);
+	(void)cpu_report_death();
 
 	/*
 	 * Actually shutdown the CPU. This must never fail. The specific hotplug

1.8.1.5

From: "Paul E. McKenney" <[email protected]>

This commit removes the open-coded CPU-offline notification with new common code. In particular, this change avoids calling scheduler code using RCU from an offline CPU that RCU is ignoring. This is a minimal change. A more intrusive change might invoke the cpu_check_up_prepare() and cpu_set_state_online() functions at CPU-online time, which would allow onlining throw an error if the CPU did not go offline properly.

---
 arch/arm/kernel/smp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

index cca5b8758185..8ef0ef0287ee 100644
@@ -218,15 +218,13 @@ int __cpu_disable(void)
 	return 0;
 }
 
-static DECLARE_COMPLETION(cpu_died);
-
 /*
  * called on the thread which is asking for a CPU to be shutdown -
  * waits until shutdown has completed, or it is timed out.
  */
 void __cpu_die(unsigned int cpu)
 {
-	if (!wait_for_completion_timeout(&cpu_died, msecs_to_jiffies(5000))) {
+	if (!cpu_wait_death(cpu, 5)) {
 		pr_err("CPU%u: cpu didn't die\n", cpu);
 		return;
 	}

@@ -272,7 +270,7 @@ void __ref cpu_die(void)
 	 * this returns, power and/or clocks can be removed at any point
 	 * from this CPU and its cache by platform_cpu_kill().
 	 */
-	complete(&cpu_died);
+	(void)cpu_report_death();
 
 	/*
 	 * Ensure that the cache lines associated with that completion are

1.8.1.5

From: "Paul E. McKenney" <[email protected]>

The rcu_data structure's ->rcu_qs_ctr_snap field is initialized at CPU-online time from the current CPU's element of the per-CPU rcu_qs_ctr variable. Unfortunately, this is at CPU_UP_PREPARE time, so has nothing to do with the CPU being onlined. This commit therefore initializes this variable from the incoming CPU's element of rcu_qs_ctr.

---
 kernel/rcu/tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 1eb1029a5464..48d002d4faac 100644
@@ -3761,7 +3761,7 @@ rcu_init_percpu_data(int cpu, struct rcu_state *rsp)
 	rdp->gpnum = rnp->completed; /* Make CPU later note any new GP. */
 	rdp->completed = rnp->completed;
 	rdp->passed_quiesce = false;
-	rdp->rcu_qs_ctr_snap = __this_cpu_read(rcu_qs_ctr);
+	rdp->rcu_qs_ctr_snap = per_cpu(rcu_qs_ctr, cpu);
 	rdp->qs_pending = false;
 	trace_rcu_grace_period(rsp->name, rdp->gpnum, TPS("cpuonl"));
 	raw_spin_unlock_irqrestore(&rnp->lock, flags);

1.8.1.5

From: "Paul E. McKenney" <[email protected]>

Because force_qs_rnp() is invoked only from the force-quiescent-state code which runs only in the context of the grace-period kthread, a grace period must always be in progress throughout force_qs_rnp()'s execution. This commit therefore removes the rcu_gp_in_progress() check and the associated dead code.

---
 kernel/rcu/tree.c | 4 ----
 1 file changed, 4 deletions(-)

index 7c323e320075..20aeb6f9fe38 100644
@@ -2718,10 +2718,6 @@ static void force_qs_rnp(struct rcu_state *rsp,
 		mask = 0;
 		raw_spin_lock_irqsave(&rnp->lock, flags);
 		smp_mb__after_unlock_lock();
-		if (!rcu_gp_in_progress(rsp)) {
-			raw_spin_unlock_irqrestore(&rnp->lock, flags);
-			return;
-		}
 		if (rnp->qsmask == 0) {
 			if (rcu_state_p == &rcu_sched_state ||
 			    rsp != rcu_state_p ||

1.8.1.5

From: Paul Gortmaker <[email protected]>

We removed __cpuinit support (leaving no-op stubs) quite some time ago. However a new instance was added in commit 00df35f991914db6b8bde8cf0980 ("cpu: Defer smpboot kthread unparking until CPU known to scheduler")

Since we want to clobber the stubs soon, get this removed now.

---
 kernel/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 260eb7db281e..9c9c9fab16cc 100644
@@ -479,7 +479,7 @@ static struct notifier_block smpboot_thread_notifier = {
 	.priority = CPU_PRI_SMPBOOT,
 };
 
-void __cpuinit smpboot_thread_init(void)
+void smpboot_thread_init(void)
 {
 	register_cpu_notifier(&smpboot_thread_notifier);
 }

1.8.1.5

From: "Paul E. McKenney" <[email protected]>

Because offline CPUs are propagated up the rcu_node tree's ->qsmaskinit bits just before each grace period starts, the ->qsmaskinit bit cannot be clear when the corresponding ->qsmask bit is set. Furthermore, this condition used to correspond to a CPU that was on its way offline, and making RCU's notion of an offline CPU more precise has eliminated this situation. This commit therefore removes the now-redundant offline check from force_qs_rnp().

---
 kernel/rcu/tree.c | 2 --
 1 file changed, 2 deletions(-)

index 20aeb6f9fe38..1eb1029a5464 100644
@@ -2747,8 +2747,6 @@ static void force_qs_rnp(struct rcu_state *rsp,
 		bit = 1;
 		for (; cpu <= rnp->grphi; cpu++, bit <<= 1) {
 			if ((rnp->qsmask & bit) != 0) {
-				if ((rnp->qsmaskinit & bit) == 0)
-					*isidle = false; /* Pending hotplug. */
 				if (f(per_cpu_ptr(rsp->rda, cpu), isidle, maxj))
 					mask |= bit;
 			}

1.8.1.5

From: "Paul E. McKenney" <[email protected]>

This commit removes a HOTPLUG_CPU #ifdef, replacing it with IS_ENABLED()-protected return statements. This relies on the optimizer to remove any resulting dead code.

---
 kernel/rcu/tree.c | 39 ++++++++++++++-------------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

index 4d3299577d7b..7c323e320075 100644
@@ -2333,8 +2333,6 @@ rcu_check_quiescent_state(struct rcu_state *rsp, struct rcu_data *rdp)
 	rcu_report_qs_rdp(rdp->cpu, rsp, rdp);
 }
 
-#ifdef CONFIG_HOTPLUG_CPU
-
 /*
  * Send the specified CPU's RCU callbacks to the orphanage.  The
  * specified CPU must be offline, and the caller must hold the

@@ -2345,7 +2343,7 @@ rcu_send_cbs_to_orphanage(int cpu, struct rcu_state *rsp,
 			  struct rcu_node *rnp, struct rcu_data *rdp)
 {
 	/* No-CBs CPUs do not have orphanable callbacks. */
-	if (rcu_is_nocb_cpu(rdp->cpu))
+	if (!IS_ENABLED(CONFIG_HOTPLUG_CPU) || rcu_is_nocb_cpu(rdp->cpu))
 		return;
 
 	/*

@@ -2404,7 +2402,8 @@ static void rcu_adopt_orphan_cbs(struct rcu_state *rsp, unsigned long flags)
 	struct rcu_data *rdp = raw_cpu_ptr(rsp->rda);
 
 	/* No-CBs CPUs are handled specially. */
-	if (rcu_nocb_adopt_orphan_cbs(rsp, rdp, flags))
+	if (!IS_ENABLED(CONFIG_HOTPLUG_CPU) ||
+	    rcu_nocb_adopt_orphan_cbs(rsp, rdp, flags))
 		return;
 
 	/* Do the accounting first. */

@@ -2451,6 +2450,9 @@ static void rcu_cleanup_dying_cpu(struct rcu_state *rsp)
 	RCU_TRACE(struct rcu_data *rdp = this_cpu_ptr(rsp->rda));
 	RCU_TRACE(struct rcu_node *rnp = rdp->mynode);
 
+	if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))
+		return;
+
 	RCU_TRACE(mask = rdp->grpmask);
 	trace_rcu_grace_period(rsp->name,
 			       rnp->gpnum + 1 - !!(rnp->qsmask & mask),

@@ -2479,7 +2481,8 @@ static void rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf)
 	long mask;
 	struct rcu_node *rnp = rnp_leaf;
 
-	if (rnp->qsmaskinit || rcu_preempt_has_tasks(rnp))
+	if (!IS_ENABLED(CONFIG_HOTPLUG_CPU) ||
+	    rnp->qsmaskinit || rcu_preempt_has_tasks(rnp))
 		return;
 	for (;;) {
 		mask = rnp->grpmask;

@@ -2510,6 +2513,9 @@ static void rcu_cleanup_dying_idle_cpu(int cpu, struct rcu_state *rsp)
 	struct rcu_data *rdp = per_cpu_ptr(rsp->rda, cpu);
 	struct rcu_node *rnp = rdp->mynode;  /* Outgoing CPU's rdp & rnp. */
 
+	if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))
+		return;
+
 	/* Remove outgoing CPU from mask in the leaf rcu_node structure. */
 	mask = rdp->grpmask;
 	raw_spin_lock_irqsave(&rnp->lock, flags);

@@ -2531,6 +2537,9 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp)
 	struct rcu_data *rdp = per_cpu_ptr(rsp->rda, cpu);
 	struct rcu_node *rnp = rdp->mynode;  /* Outgoing CPU's rdp & rnp. */
 
+	if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))
+		return;
+
 	/* Adjust any no-longer-needed kthreads. */
 	rcu_boost_kthread_setaffinity(rnp, -1);
 

@@ -2545,26 +2554,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp)
 		  cpu, rdp->qlen, rdp->nxtlist);
 }
 
-#else /* #ifdef CONFIG_HOTPLUG_CPU */
-
-static void rcu_cleanup_dying_cpu(struct rcu_state *rsp)
-{
-}
-
-static void __maybe_unused rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf)
-{
-}
-
-static void rcu_cleanup_dying_idle_cpu(int cpu, struct rcu_state *rsp)
-{
-}
-
-static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp)
-{
-}
-
-#endif /* #else #ifdef CONFIG_HOTPLUG_CPU */
-
 /*
  * Invoke any RCU callbacks that have made it to the end of their grace
  * period.  Thottle as specified by rdp->blimit.

1.8.1.5

From: "Paul E. McKenney" <[email protected]>

Commit 00df35f99191 (cpu: Defer smpboot kthread unparking until CPU known to scheduler) put the online path's call to smpboot_unpark_threads() into a CPU-hotplug notifier. This commit places the offline-failure paths call into the same notifier for the sake of uniformity.

Note that it is not currently possible to place the offline path's call to smpboot_park_threads() into an existing notifier because the CPU_DYING notifiers run in a restricted environment, and the CPU_UP_PREPARE notifiers run too soon.

---
 kernel/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 94bbe4695232..260eb7db281e 100644
@@ -398,7 +398,6 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen)
 	err = __stop_machine(take_cpu_down, &tcd_param, cpumask_of(cpu));
 	if (err) {
 		/* CPU didn't die: tell everyone.  Can't complain. */
-		smpboot_unpark_threads(cpu);
 		cpu_notify_nofail(CPU_DOWN_FAILED | mod, hcpu);
 		goto out_release;
 	}

@@ -463,6 +462,7 @@ static int smpboot_thread_call(struct notifier_block *nfb,
 
 	switch (action & ~CPU_TASKS_FROZEN) {
 
+	case CPU_DOWN_FAILED:
 	case CPU_ONLINE:
 		smpboot_unpark_threads(cpu);
 		break;

1.8.1.5

From: "Paul E. McKenney" <[email protected]>

Tasks are no longer migrated away from a given rcu_node structure when all CPUs corresponding to that rcu_node structure have gone offline. This means that rcu_read_unlock_special() no longer needs to loop retrying rcu_node ->lock acquisition because the current task is guaranteed to stay put.

This commit takes a small and paranoid step towards relying on this guarantee by placing a WARN_ON_ONCE() just after the early exit from the lock-acquisition loop.

---
 kernel/rcu/tree_plugin.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index 58b1ebdc4387..c8340e929eb4 100644
@@ -307,9 +307,11 @@ void rcu_read_unlock_special(struct task_struct *t)
 		t->rcu_read_unlock_special.b.blocked = false;
 
 		/*
-		 * Remove this task from the list it blocked on.  The
-		 * task can migrate while we acquire the lock, but at
-		 * most one time.  So at most two passes through loop.
+		 * Remove this task from the list it blocked on.  The task
+		 * now remains queued on the rcu_node corresponding to
+		 * the CPU it first blocked on, so the first attempt to
+		 * acquire the task's rcu_node's ->lock will succeed.
+		 * Keep the loop and add a WARN_ON() out of sheer paranoia.
 		 */
 		for (;;) {
 			rnp = t->rcu_blocked_node;

@@ -317,6 +319,7 @@ void rcu_read_unlock_special(struct task_struct *t)
 			smp_mb__after_unlock_lock();
 			if (rnp == t->rcu_blocked_node)
 				break;
+			WARN_ON_ONCE(1);
 			raw_spin_unlock(&rnp->lock); /* irqs remain disabled. */
 		}
 		empty_norm = !rcu_preempt_blocked_readers_cgp(rnp);

1.8.1.5

Hi Paul,

Sorry for being late on this one.

I don't have a better idea, so:

Is this queued via -tip already, or can we pick it into the arm64 tree for 4.2?

Cheers,

Will

No problem! In fact, just in time -- I was going to bug you about this later today. ;-)

Feel free to take it into the arm64 tree. Just let me know, and I will drop it from my tree.

Now I only need to bug the arm32 guys. ;-)

Thanx, Paul

Queued for 4.2. Thanks.

Catalin

Very good, dropped from my tree.

Thanx, Paul

Hi Paul,

Thanks, this seems to fix the intermittent "suspicious RCU usage" warnings I've been seeing during suspend to RAM on r8a7791/koelsch, which has a dual-core Cortex A15:

Disabling non-boot CPUs ...

===============================
[ INFO: suspicious RCU usage. ]
4.4.0-rc4-koelsch #2123 Tainted: G W
-------------------------------
kernel/sched/fair.c:4938 suspicious rcu_dereference_check() usage!

other info that might help us debug this:

RCU used illegally from offline CPU!
rcu_scheduler_active = 1, debug_locks = 0
3 locks held by swapper/1/0:
#0: ((cpu_died).wait.lock){......}, at: [<c006077c>] complete+0x14/0x44 #1: (&p->pi_lock){-.-.-.}, at: [<c004b8a8>] try_to_wake_up+0x24/0x340 #2: (rcu_read_lock){......}, at: [<c0052740>]

select_task_rq_fair+0xb4/0x820

    stack backtrace:
    CPU: 1 PID: 0 Comm: swapper/1 Tainted: G        W
4.4.0-rc4-koelsch #2123
    Hardware name: Generic R8A7791 (Flattened Device Tree)
    [<c0017390>] (unwind_backtrace) from [<c0013094>] (show_stack+0x10/0x14)
    [<c0013094>] (show_stack) from [<c01ee718>] (dump_stack+0x70/0x8c)
    [<c01ee718>] (dump_stack) from [<c0052810>]
(select_task_rq_fair+0x184/0x820)
    [<c0052810>] (select_task_rq_fair) from [<c004ba54>]
(try_to_wake_up+0x1d0/0x340)
    [<c004ba54>] (try_to_wake_up) from [<c0060070>] (__wake_up_common+0x4c/0x78)
    [<c0060070>] (__wake_up_common) from [<c00600ac>]
(__wake_up_locked+0x10/0x18)
    [<c00600ac>] (__wake_up_locked) from [<c006079c>] (complete+0x34/0x44)
    [<c006079c>] (complete) from [<c00159f8>] (arch_cpu_idle_dead+0x2c/0x8c)
    [<c00159f8>] (arch_cpu_idle_dead) from [<c0060a24>]
(cpu_startup_entry+0x84/0x228)
    [<c0060a24>] (cpu_startup_entry) from [<4000a4ac>] (0x4000a4ac)
    CPU1: shutdown

But I understand from the various other threads about this issue that this patch is not going upstream, as it papers over the real issue?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds