Skip to contentSkip to conversation

arm64: Fix issues with CPU hotplug and RCU

PATCH In review Will Deacon 3 participants 5 messages Nov 6, 2020 – Nov 19, 2020

Subsystems (2)
lore
Share & tools
lore

Will Deacon

author maintainer
#0 lore raw

cover arm64: Fix issues with CPU hotplug and RCU

Hi folks,

Here are a couple of patches following on from:

https://lore.kernel.org/r/20201105222242.GA8842@willie-the-truck

which address issues when CPU onlining fails but RCU is left none the wiser. Tested under QEMU.

If Paul is happy with the second patch, then I can take both of these via arm64 as fixes for 5.11.

Cheers,

Will

Cc Catalin Marinas <[email protected]>
Cc Qian Cai <[email protected]>
Cc "Paul E. McKenney" <[email protected]>

--->8

Will Deacon (2):
arm64: psci: Avoid printing in cpu_psci_cpu_die()
arm64: smp: Tell RCU about CPUs that fail to come online

 arch/arm64/kernel/psci.c | 2 --
 arch/arm64/kernel/smp.c  | 1 +
 kernel/rcu/tree.c        | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)
Signature · 1 line
2.29.1.341.ge80a0c044ae-goog
author maintainer
#1 lore raw

PATCH 1/2 arm64: psci: Avoid printing in cpu_psci_cpu_die()

cpu_psci_cpu_die() is called in the context of the dying CPU, which will no longer be online or tracked by RCU. It is therefore not generally safe to call printk() if the PSCI "cpu off" request fails, so remove the pr_crit() invocation.

Cc Qian Cai <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Cc Catalin Marinas <[email protected]>
---
 arch/arm64/kernel/psci.c | 2 --
 1 file changed, 2 deletions(-)
arch/arm64/kernel/psci.c +0 -2
index 43ae4e0c968f..6a4f3e37c3b4 100644
@@ -75,8 +75,6 @@ static void cpu_psci_cpu_die(unsigned int cpu)
 		    PSCI_0_2_POWER_STATE_TYPE_SHIFT;
 
 	ret = psci_ops.cpu_off(state);
-
-	pr_crit("unable to power off CPU%u (%d)\n", cpu, ret);
 }
 
 static int cpu_psci_cpu_kill(unsigned int cpu)
Signature · 1 line
2.29.1.341.ge80a0c044ae-goog
#2 lore raw

Re: [PATCH 1/2] arm64: psci: Avoid printing in cpu_psci_cpu_die()

I love your patch! Perhaps something to improve:

Hi Will,

I love your patch! Perhaps something to improve:

[auto build test WARNING on arm/for-next]
[also build test WARNING on soc/for-next xlnx/master kvmarm/next] [cannot apply to arm64/for-next/core v5.10-rc4 next-20201119] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Will-Deacon/arm64-Fix-issues-with-CPU-hotplug-and-RCU/20201109-092901
base:   git://git.armlinux.org.uk/~rmk/linux-arm.git for-next
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/b994fd4f2e7cee6f64d38036bd76c6c9644cd2b1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Will-Deacon/arm64-Fix-issues-with-CPU-hotplug-and-RCU/20201109-092901 git checkout b994fd4f2e7cee6f64d38036bd76c6c9644cd2b1
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64

If you fix the issue, kindly add following tag as appropriate

All warnings (new ones prefixed by >>):

arch/arm64/kernel/psci.c: In function 'cpu_psci_cpu_die':

>> arch/arm64/kernel/psci.c:69:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
      69 |  int ret;
         |      ^~~

vim +/ret +69 arch/arm64/kernel/psci.c

831ccf79b46e02f Mark Rutland    2013-10-24  66  
831ccf79b46e02f Mark Rutland    2013-10-24  67  static void cpu_psci_cpu_die(unsigned int cpu)
831ccf79b46e02f Mark Rutland    2013-10-24  68  {
831ccf79b46e02f Mark Rutland    2013-10-24 @69  	int ret;
831ccf79b46e02f Mark Rutland    2013-10-24  70  	/*
831ccf79b46e02f Mark Rutland    2013-10-24  71  	 * There are no known implementations of PSCI actually using the
831ccf79b46e02f Mark Rutland    2013-10-24  72  	 * power state field, pass a sensible default for now.
831ccf79b46e02f Mark Rutland    2013-10-24  73  	 */
c8cc42737788537 Mark Rutland    2015-04-30  74  	u32 state = PSCI_POWER_STATE_TYPE_POWER_DOWN <<
c8cc42737788537 Mark Rutland    2015-04-30  75  		    PSCI_0_2_POWER_STATE_TYPE_SHIFT;
831ccf79b46e02f Mark Rutland    2013-10-24  76  
831ccf79b46e02f Mark Rutland    2013-10-24  77  	ret = psci_ops.cpu_off(state);
831ccf79b46e02f Mark Rutland    2013-10-24  78  }

c814ca029e1015b Ashwin Chaugule 2014-05-07 79

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

author maintainer
#3 lore raw

PATCH 2/2 arm64: smp: Tell RCU about CPUs that fail to come online

In reply to Will Deacon

Commit ce3d31ad3cac ("arm64/smp: Move rcu_cpu_starting() earlier") ensured that RCU is informed early about incoming CPUs that might end up calling into printk() before they are online. However, if such a CPU fails the early CPU feature compatibility checks in check_local_cpu_capabilities(), then it will be powered off or parked without informing RCU, leading to an endless stream of stalls:

  | rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
  | rcu:	2-O...: (0 ticks this GP) idle=002/1/0x4000000000000000 softirq=0/0 fqs=2593
  | (detected by 0, t=5252 jiffies, g=9317, q=136)
  | Task dump for CPU 2:
  | task:swapper/2       state:R  running task     stack:    0 pid:    0 ppid:     1 flags:0x00000028
  | Call trace:
  | ret_from_fork+0x0/0x30

Ensure that the dying CPU invokes rcu_report_dead() prior to being powered off or parked.

Cc Qian Cai <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
---
 arch/arm64/kernel/smp.c | 1 +
 kernel/rcu/tree.c       | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)
arch/arm64/kernel/smp.c +1 -0
index 09c96f57818c..18e9727d3f64 100644
@@ -413,6 +413,7 @@ void cpu_die_early(void)
 
 	/* Mark this CPU absent */
 	set_cpu_present(cpu, 0);
+	rcu_report_dead(cpu);
 
 	if (IS_ENABLED(CONFIG_HOTPLUG_CPU)) {
 		update_cpu_boot_status(CPU_KILL_ME);
kernel/rcu/tree.c +1 -1
index 2a52f42f64b6..bd04b09b84b3 100644
@@ -4077,7 +4077,6 @@ void rcu_cpu_starting(unsigned int cpu)
 	smp_mb(); /* Ensure RCU read-side usage follows above initialization. */
 }
 
-#ifdef CONFIG_HOTPLUG_CPU
 /*
  * The outgoing function has no further need of RCU, so remove it from
  * the rcu_node tree's ->qsmaskinitnext bit masks.
@@ -4117,6 +4116,7 @@ void rcu_report_dead(unsigned int cpu)
 	rdp->cpu_started = false;
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
 /*
  * The outgoing CPU has just passed through the dying-idle state, and we
  * are being invoked from the CPU that was IPIed to continue the offline
Signature · 1 line
2.29.1.341.ge80a0c044ae-goog
maintainer Reviewed-by on v1 2/2
#4 lore raw

Re: [PATCH 2/2] arm64: smp: Tell RCU about CPUs that fail to come online

Quoted 15 lines · Will Deacon

Commit ce3d31ad3cac ("arm64/smp: Move rcu_cpu_starting() earlier") ensured that RCU is informed early about incoming CPUs that might end up calling into printk() before they are online. However, if such a CPU fails the early CPU feature compatibility checks in check_local_cpu_capabilities(), then it will be powered off or parked without informing RCU, leading to an endless stream of stalls:

  | rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
  | rcu:	2-O...: (0 ticks this GP) idle=002/1/0x4000000000000000 softirq=0/0 fqs=2593
  | (detected by 0, t=5252 jiffies, g=9317, q=136)
  | Task dump for CPU 2:
  | task:swapper/2       state:R  running task     stack:    0 pid:    0 ppid:     1 flags:0x00000028
  | Call trace:
  | ret_from_fork+0x0/0x30
Quoted 7 lines

Ensure that the dying CPU invokes rcu_report_dead() prior to being powered off or parked.

Cc Qian Cai <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Quoted 40 lines
---
 arch/arm64/kernel/smp.c | 1 +
 kernel/rcu/tree.c       | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)
arch/arm64/kernel/smp.c +1 -0
index 09c96f57818c..18e9727d3f64 100644
@@ -413,6 +413,7 @@ void cpu_die_early(void)
 
 	/* Mark this CPU absent */
 	set_cpu_present(cpu, 0);
+	rcu_report_dead(cpu);
 
 	if (IS_ENABLED(CONFIG_HOTPLUG_CPU)) {
 		update_cpu_boot_status(CPU_KILL_ME);
kernel/rcu/tree.c +1 -1
index 2a52f42f64b6..bd04b09b84b3 100644
@@ -4077,7 +4077,6 @@ void rcu_cpu_starting(unsigned int cpu)
 	smp_mb(); /* Ensure RCU read-side usage follows above initialization. */
 }
 
-#ifdef CONFIG_HOTPLUG_CPU
 /*
  * The outgoing function has no further need of RCU, so remove it from
  * the rcu_node tree's ->qsmaskinitnext bit masks.
@@ -4117,6 +4116,7 @@ void rcu_report_dead(unsigned int cpu)
 	rdp->cpu_started = false;
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
 /*
  * The outgoing CPU has just passed through the dying-idle state, and we
  * are being invoked from the CPU that was IPIed to continue the offline
Signature · 1 line
2.29.1.341.ge80a0c044ae-goog