linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] reboot: support hotplug CPUs before reboot
@ 2020-01-14  6:30 Hsin-Yi Wang
  2020-01-14  9:45 ` Vitaly Kuznetsov
  2020-01-17  8:23 ` kbuild test robot
  0 siblings, 2 replies; 4+ messages in thread
From: Hsin-Yi Wang @ 2020-01-14  6:30 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Josh Poimboeuf, Ingo Molnar, Peter Zijlstra, Jiri Kosina,
	Pavankumar Kondeti, Vitaly Kuznetsov, Aaro Koskinen,
	Greg Kroah-Hartman, Guenter Roeck, Stephen Boyd, linux-kernel

Currently system reboots uses architecture specific codes (smp_send_stop)
to offline non reboot CPUs. Most architecture's implementation is looping
through all non reboot online CPUs and call ipi function to each of them. Some
architecture like arm64, arm, and x86... would set offline masks to cpu without
really offline them. This causes some race condition and kernel warning comes
out sometimes when system reboots.

This patch adds a config ARCH_OFFLINE_CPUS_ON_REBOOT, which would hotplug cpus in
migrate_to_reboot_cpu(). If non reboot cpus are all offlined here, the loop for
checking online cpus would be an empty loop. If architecture don't enable this
config, or some cpus somehow fails to offline, it would fallback to ipi
function.

Opt in this config for architectures that support CONFIG_HOTPLUG_CPU.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
Change from v3:
* Opt in config for architectures that support CONFIG_HOTPLUG_CPU
* Merge function offline_secondary_cpus() and freeze_secondary_cpus()
  with an additional flag.

Change from v2:
* Add another config instead of configed by CONFIG_HOTPLUG_CPU
---
 arch/Kconfig                          |  5 +++++
 arch/arm/Kconfig                      |  1 +
 arch/arm64/Kconfig                    |  1 +
 arch/arm64/kernel/hibernate.c         |  2 +-
 arch/csky/Kconfig                     |  1 +
 arch/ia64/Kconfig                     |  1 +
 arch/mips/Kconfig                     |  1 +
 arch/parisc/Kconfig                   |  1 +
 arch/powerpc/Kconfig                  |  1 +
 arch/s390/Kconfig                     |  1 +
 arch/sh/Kconfig                       |  1 +
 arch/sparc/Kconfig                    |  1 +
 arch/x86/Kconfig                      |  1 +
 arch/xtensa/Kconfig                   |  1 +
 drivers/power/reset/sc27xx-poweroff.c |  2 +-
 include/linux/cpu.h                   |  9 ++++++---
 kernel/cpu.c                          | 17 +++++++++++------
 kernel/reboot.c                       |  8 ++++++++
 18 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 48b5e103bdb0..2ba3d62c98c5 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -255,6 +255,11 @@ config ARCH_HAS_UNCACHED_SEGMENT
 	select ARCH_HAS_DMA_PREP_COHERENT
 	bool
 
+# Select to do a full hotplug on secondary CPUs before reboot.
+config ARCH_OFFLINE_CPUS_ON_REBOOT
+	bool "Support for hotplug CPUs before reboot"
+	depends on HOTPLUG_CPU
+
 # Select if arch init_task must go in the __init_task_data section
 config ARCH_TASK_STRUCT_ON_STACK
 	bool
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 69950fb5be64..d53cc8cb47e3 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -28,6 +28,7 @@ config ARM
 	select ARCH_KEEP_MEMBLOCK if HAVE_ARCH_PFN_VALID || KEXEC
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_NO_SG_CHAIN if !ARM_HAS_SG_CHAIN
+	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
 	select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7
 	select ARCH_SUPPORTS_ATOMIC_RMW
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9af26ac75d19..9f913bc5c1f6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -61,6 +61,7 @@ config ARM64
 	select ARCH_INLINE_SPIN_UNLOCK_IRQ if !PREEMPTION
 	select ARCH_INLINE_SPIN_UNLOCK_IRQRESTORE if !PREEMPTION
 	select ARCH_KEEP_MEMBLOCK
+	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
 	select ARCH_USE_CMPXCHG_LOCKREF
 	select ARCH_USE_QUEUED_RWLOCKS
 	select ARCH_USE_QUEUED_SPINLOCKS
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 590963c9c609..f7245dfa09d9 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -581,5 +581,5 @@ int hibernate_resume_nonboot_cpu_disable(void)
 		return -ENODEV;
 	}
 
-	return freeze_secondary_cpus(sleep_cpu);
+	return freeze_secondary_cpus(sleep_cpu, false);
 }
diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
index 4acef4088de7..0f03e5c3f2fc 100644
--- a/arch/csky/Kconfig
+++ b/arch/csky/Kconfig
@@ -5,6 +5,7 @@ config CSKY
 	select ARCH_HAS_DMA_PREP_COHERENT
 	select ARCH_HAS_SYNC_DMA_FOR_CPU
 	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
+	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
 	select ARCH_USE_BUILTIN_BSWAP
 	select ARCH_USE_QUEUED_RWLOCKS if NR_CPUS>2
 	select COMMON_CLK
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index bab7cd878464..f12b4b11ee98 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -10,6 +10,7 @@ config IA64
 	bool
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_MIGHT_HAVE_PC_SERIO
+	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
 	select ACPI
 	select ACPI_NUMA if NUMA
 	select ARCH_SUPPORTS_ACPI
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index b6b5f83af169..9bb2556d21fc 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -8,6 +8,7 @@ config MIPS
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
 	select ARCH_HAS_FORTIFY_SOURCE
+	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
 	select ARCH_SUPPORTS_UPROBES
 	select ARCH_USE_BUILTIN_BSWAP
 	select ARCH_USE_CMPXCHG_LOCKREF if 64BIT
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 71034b54d74e..41609f00b057 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -13,6 +13,7 @@ config PARISC
 	select ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
 	select ARCH_NO_SG_CHAIN
+	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
 	select ARCH_SUPPORTS_MEMORY_FAILURE
 	select RTC_CLASS
 	select RTC_DRV_GENERIC
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 658e0324d256..a6b76dd82a2d 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -142,6 +142,7 @@ config PPC
 	select ARCH_KEEP_MEMBLOCK
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_MIGHT_HAVE_PC_SERIO
+	select ARCH_OFFLINE_CPUS_ON_REBOOT	if HOTPLUG_CPU
 	select ARCH_OPTIONAL_KERNEL_RWX		if ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_SUPPORTS_ATOMIC_RMW
 	select ARCH_USE_BUILTIN_BSWAP
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 287714d51b47..19eec37b1682 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -102,6 +102,7 @@ config S390
 	select ARCH_INLINE_WRITE_UNLOCK_IRQ
 	select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
 	select ARCH_KEEP_MEMBLOCK
+	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
 	select ARCH_SAVE_PAGE_KEYS if HIBERNATION
 	select ARCH_STACKWALK
 	select ARCH_SUPPORTS_ATOMIC_RMW
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 9ece111b0254..4ed1e0ca83a2 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -18,6 +18,7 @@ config SUPERH
 	select ARCH_HAVE_CUSTOM_GPIO_H
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG if (GUSA_RB || CPU_SH4A)
 	select ARCH_HAS_GCOV_PROFILE_ALL
+	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
 	select PERF_USE_VMALLOC
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_KERNEL_GZIP
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index e8c3ea01c12f..f31700309621 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -30,6 +30,7 @@ config SPARC
 	select RTC_SYSTOHC
 	select HAVE_ARCH_JUMP_LABEL if SPARC64
 	select GENERIC_IRQ_SHOW
+	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
 	select ARCH_WANT_IPC_PARSE_VERSION
 	select GENERIC_PCI_IOMAP
 	select HAVE_NMI_WATCHDOG if SPARC64
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b595ecb21a0f..e8edab974f67 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -85,6 +85,7 @@ config X86
 	select ARCH_MIGHT_HAVE_ACPI_PDC		if ACPI
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_MIGHT_HAVE_PC_SERIO
+	select ARCH_OFFLINE_CPUS_ON_REBOOT	if HOTPLUG_CPU
 	select ARCH_STACKWALK
 	select ARCH_SUPPORTS_ACPI
 	select ARCH_SUPPORTS_ATOMIC_RMW
diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
index 1c645172b4b5..c862dfa69ed9 100644
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -7,6 +7,7 @@ config XTENSA
 	select ARCH_HAS_SYNC_DMA_FOR_CPU if MMU
 	select ARCH_HAS_SYNC_DMA_FOR_DEVICE if MMU
 	select ARCH_HAS_UNCACHED_SEGMENT if MMU
+	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
 	select ARCH_USE_QUEUED_RWLOCKS
 	select ARCH_USE_QUEUED_SPINLOCKS
 	select ARCH_WANT_FRAME_POINTERS
diff --git a/drivers/power/reset/sc27xx-poweroff.c b/drivers/power/reset/sc27xx-poweroff.c
index 29fb08b8faa0..d6cdf837235c 100644
--- a/drivers/power/reset/sc27xx-poweroff.c
+++ b/drivers/power/reset/sc27xx-poweroff.c
@@ -30,7 +30,7 @@ static void sc27xx_poweroff_shutdown(void)
 #ifdef CONFIG_PM_SLEEP_SMP
 	int cpu = smp_processor_id();
 
-	freeze_secondary_cpus(cpu);
+	freeze_secondary_cpus(cpu, false);
 #endif
 }
 
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 1ca2baf817ed..9c62274a4db9 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -137,11 +137,14 @@ static inline void cpu_hotplug_done(void) { cpus_write_unlock(); }
 static inline void get_online_cpus(void) { cpus_read_lock(); }
 static inline void put_online_cpus(void) { cpus_read_unlock(); }
 
+#if defined(CONFIG_PM_SLEEP_SMP) || defined(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
+extern int freeze_secondary_cpus(int primary, bool reboot);
+#endif
+
 #ifdef CONFIG_PM_SLEEP_SMP
-extern int freeze_secondary_cpus(int primary);
 static inline int disable_nonboot_cpus(void)
 {
-	return freeze_secondary_cpus(0);
+	return freeze_secondary_cpus(0, false);
 }
 extern void enable_nonboot_cpus(void);
 
@@ -152,7 +155,7 @@ static inline int suspend_disable_secondary_cpus(void)
 	if (IS_ENABLED(CONFIG_PM_SLEEP_SMP_NONZERO_CPU))
 		cpu = -1;
 
-	return freeze_secondary_cpus(cpu);
+	return freeze_secondary_cpus(cpu, false);
 }
 static inline void suspend_enable_secondary_cpus(void)
 {
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 9c706af713fb..87d2c6f5ce4c 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1209,10 +1209,10 @@ int cpu_up(unsigned int cpu)
 }
 EXPORT_SYMBOL_GPL(cpu_up);
 
-#ifdef CONFIG_PM_SLEEP_SMP
+#if defined(CONFIG_PM_SLEEP_SMP) || defined(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
 static cpumask_var_t frozen_cpus;
 
-int freeze_secondary_cpus(int primary)
+int freeze_secondary_cpus(int primary, bool reboot)
 {
 	int cpu, error = 0;
 
@@ -1237,20 +1237,25 @@ int freeze_secondary_cpus(int primary)
 		if (cpu == primary)
 			continue;
 
-		if (pm_wakeup_pending()) {
+#ifdef CONFIG_PM_SLEEP
+		if (!reboot && pm_wakeup_pending()) {
 			pr_info("Wakeup pending. Abort CPU freeze\n");
 			error = -EBUSY;
 			break;
 		}
+#endif
 
-		trace_suspend_resume(TPS("CPU_OFF"), cpu, true);
+		if (!reboot)
+			trace_suspend_resume(TPS("CPU_OFF"), cpu, true);
 		error = _cpu_down(cpu, 1, CPUHP_OFFLINE);
-		trace_suspend_resume(TPS("CPU_OFF"), cpu, false);
+		if (!reboot)
+			trace_suspend_resume(TPS("CPU_OFF"), cpu, false);
 		if (!error)
 			cpumask_set_cpu(cpu, frozen_cpus);
 		else {
 			pr_err("Error taking CPU%d down: %d\n", cpu, error);
-			break;
+			if (!reboot)
+				break;
 		}
 	}
 
diff --git a/kernel/reboot.c b/kernel/reboot.c
index c4d472b7f1b4..e3bf515dc2bc 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -7,6 +7,7 @@
 
 #define pr_fmt(fmt)	"reboot: " fmt
 
+#include <linux/cpu.h>
 #include <linux/ctype.h>
 #include <linux/export.h>
 #include <linux/kexec.h>
@@ -220,7 +221,9 @@ void migrate_to_reboot_cpu(void)
 	/* The boot cpu is always logical cpu 0 */
 	int cpu = reboot_cpu;
 
+#if !IS_ENABLED(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
 	cpu_hotplug_disable();
+#endif
 
 	/* Make certain the cpu I'm about to reboot on is online */
 	if (!cpu_online(cpu))
@@ -231,6 +234,11 @@ void migrate_to_reboot_cpu(void)
 
 	/* Make certain I only run on the appropriate processor */
 	set_cpus_allowed_ptr(current, cpumask_of(cpu));
+
+#if IS_ENABLED(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
+	/* Hotplug other cpus if possible */
+	freeze_secondary_cpus(cpu, true);
+#endif
 }
 
 /**
-- 
2.25.0.rc1.283.g88dfdc4193-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] reboot: support hotplug CPUs before reboot
  2020-01-14  6:30 [PATCH v4] reboot: support hotplug CPUs before reboot Hsin-Yi Wang
@ 2020-01-14  9:45 ` Vitaly Kuznetsov
  2020-01-14 10:19   ` Hsin-Yi Wang
  2020-01-17  8:23 ` kbuild test robot
  1 sibling, 1 reply; 4+ messages in thread
From: Vitaly Kuznetsov @ 2020-01-14  9:45 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: Josh Poimboeuf, Ingo Molnar, Peter Zijlstra, Jiri Kosina,
	Pavankumar Kondeti, Aaro Koskinen, Greg Kroah-Hartman,
	Guenter Roeck, Stephen Boyd, linux-kernel, Thomas Gleixner

Hsin-Yi Wang <hsinyi@chromium.org> writes:

> Currently system reboots uses architecture specific codes (smp_send_stop)
> to offline non reboot CPUs. Most architecture's implementation is looping
> through all non reboot online CPUs and call ipi function to each of them. Some
> architecture like arm64, arm, and x86... would set offline masks to cpu without
> really offline them. This causes some race condition and kernel warning comes
> out sometimes when system reboots.
>
> This patch adds a config ARCH_OFFLINE_CPUS_ON_REBOOT, which would hotplug cpus in
> migrate_to_reboot_cpu(). If non reboot cpus are all offlined here, the loop for
> checking online cpus would be an empty loop. If architecture don't enable this
> config, or some cpus somehow fails to offline, it would fallback to ipi
> function.
>
> Opt in this config for architectures that support CONFIG_HOTPLUG_CPU.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> Change from v3:
> * Opt in config for architectures that support CONFIG_HOTPLUG_CPU
> * Merge function offline_secondary_cpus() and freeze_secondary_cpus()
>   with an additional flag.

I haven't commented on the idea itself, actually (I'm not sure I'm the
right person though but as I'm already looking at the code...)

On x86, the custom IPI/NMI solution we have now is more 'lightweight'
and here you suggest we switch to the full-blown CPUHP machinery
instead. We seem to be already using it for suspend/hibernation,
however, I would expect it to be way less tested: on servers, for
example, less people do suspend/resume than reboot :-)

The existing IPI/NMI code stays in place so nothing (in theory) should
break -- unles some cpuhp_ hook does something really weird. But your
change will definitely contribute to improving these hooks' quality in
the long run :-)

I have nothing to say on other arches and I don't see anyone from
e.g. ARM on the CC: list, not good.

>
> Change from v2:
> * Add another config instead of configed by CONFIG_HOTPLUG_CPU
> ---
>  arch/Kconfig                          |  5 +++++
>  arch/arm/Kconfig                      |  1 +
>  arch/arm64/Kconfig                    |  1 +
>  arch/arm64/kernel/hibernate.c         |  2 +-
>  arch/csky/Kconfig                     |  1 +
>  arch/ia64/Kconfig                     |  1 +
>  arch/mips/Kconfig                     |  1 +
>  arch/parisc/Kconfig                   |  1 +
>  arch/powerpc/Kconfig                  |  1 +
>  arch/s390/Kconfig                     |  1 +
>  arch/sh/Kconfig                       |  1 +
>  arch/sparc/Kconfig                    |  1 +
>  arch/x86/Kconfig                      |  1 +
>  arch/xtensa/Kconfig                   |  1 +
>  drivers/power/reset/sc27xx-poweroff.c |  2 +-
>  include/linux/cpu.h                   |  9 ++++++---
>  kernel/cpu.c                          | 17 +++++++++++------
>  kernel/reboot.c                       |  8 ++++++++
>  18 files changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 48b5e103bdb0..2ba3d62c98c5 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -255,6 +255,11 @@ config ARCH_HAS_UNCACHED_SEGMENT
>  	select ARCH_HAS_DMA_PREP_COHERENT
>  	bool
>  
> +# Select to do a full hotplug on secondary CPUs before reboot.
> +config ARCH_OFFLINE_CPUS_ON_REBOOT
> +	bool "Support for hotplug CPUs before reboot"
> +	depends on HOTPLUG_CPU
> +

Nit: what we're actually doing is not 'hotplug', it's either 'hotunplug' or
'offlining'.

>  # Select if arch init_task must go in the __init_task_data section
>  config ARCH_TASK_STRUCT_ON_STACK
>  	bool
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 69950fb5be64..d53cc8cb47e3 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -28,6 +28,7 @@ config ARM
>  	select ARCH_KEEP_MEMBLOCK if HAVE_ARCH_PFN_VALID || KEXEC
>  	select ARCH_MIGHT_HAVE_PC_PARPORT
>  	select ARCH_NO_SG_CHAIN if !ARM_HAS_SG_CHAIN
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
>  	select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX
>  	select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7
>  	select ARCH_SUPPORTS_ATOMIC_RMW
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9af26ac75d19..9f913bc5c1f6 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -61,6 +61,7 @@ config ARM64
>  	select ARCH_INLINE_SPIN_UNLOCK_IRQ if !PREEMPTION
>  	select ARCH_INLINE_SPIN_UNLOCK_IRQRESTORE if !PREEMPTION
>  	select ARCH_KEEP_MEMBLOCK
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
>  	select ARCH_USE_CMPXCHG_LOCKREF
>  	select ARCH_USE_QUEUED_RWLOCKS
>  	select ARCH_USE_QUEUED_SPINLOCKS
> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> index 590963c9c609..f7245dfa09d9 100644
> --- a/arch/arm64/kernel/hibernate.c
> +++ b/arch/arm64/kernel/hibernate.c
> @@ -581,5 +581,5 @@ int hibernate_resume_nonboot_cpu_disable(void)
>  		return -ENODEV;
>  	}
>  
> -	return freeze_secondary_cpus(sleep_cpu);
> +	return freeze_secondary_cpus(sleep_cpu, false);
>  }
> diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
> index 4acef4088de7..0f03e5c3f2fc 100644
> --- a/arch/csky/Kconfig
> +++ b/arch/csky/Kconfig
> @@ -5,6 +5,7 @@ config CSKY
>  	select ARCH_HAS_DMA_PREP_COHERENT
>  	select ARCH_HAS_SYNC_DMA_FOR_CPU
>  	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
>  	select ARCH_USE_BUILTIN_BSWAP
>  	select ARCH_USE_QUEUED_RWLOCKS if NR_CPUS>2
>  	select COMMON_CLK
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index bab7cd878464..f12b4b11ee98 100644
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -10,6 +10,7 @@ config IA64
>  	bool
>  	select ARCH_MIGHT_HAVE_PC_PARPORT
>  	select ARCH_MIGHT_HAVE_PC_SERIO
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
>  	select ACPI
>  	select ACPI_NUMA if NUMA
>  	select ARCH_SUPPORTS_ACPI
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index b6b5f83af169..9bb2556d21fc 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -8,6 +8,7 @@ config MIPS
>  	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>  	select ARCH_HAS_UBSAN_SANITIZE_ALL
>  	select ARCH_HAS_FORTIFY_SOURCE
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
>  	select ARCH_SUPPORTS_UPROBES
>  	select ARCH_USE_BUILTIN_BSWAP
>  	select ARCH_USE_CMPXCHG_LOCKREF if 64BIT
> diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
> index 71034b54d74e..41609f00b057 100644
> --- a/arch/parisc/Kconfig
> +++ b/arch/parisc/Kconfig
> @@ -13,6 +13,7 @@ config PARISC
>  	select ARCH_HAS_STRICT_KERNEL_RWX
>  	select ARCH_HAS_UBSAN_SANITIZE_ALL
>  	select ARCH_NO_SG_CHAIN
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
>  	select ARCH_SUPPORTS_MEMORY_FAILURE
>  	select RTC_CLASS
>  	select RTC_DRV_GENERIC
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 658e0324d256..a6b76dd82a2d 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -142,6 +142,7 @@ config PPC
>  	select ARCH_KEEP_MEMBLOCK
>  	select ARCH_MIGHT_HAVE_PC_PARPORT
>  	select ARCH_MIGHT_HAVE_PC_SERIO
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT	if HOTPLUG_CPU
>  	select ARCH_OPTIONAL_KERNEL_RWX		if ARCH_HAS_STRICT_KERNEL_RWX
>  	select ARCH_SUPPORTS_ATOMIC_RMW
>  	select ARCH_USE_BUILTIN_BSWAP
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 287714d51b47..19eec37b1682 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -102,6 +102,7 @@ config S390
>  	select ARCH_INLINE_WRITE_UNLOCK_IRQ
>  	select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
>  	select ARCH_KEEP_MEMBLOCK
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
>  	select ARCH_SAVE_PAGE_KEYS if HIBERNATION
>  	select ARCH_STACKWALK
>  	select ARCH_SUPPORTS_ATOMIC_RMW
> diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
> index 9ece111b0254..4ed1e0ca83a2 100644
> --- a/arch/sh/Kconfig
> +++ b/arch/sh/Kconfig
> @@ -18,6 +18,7 @@ config SUPERH
>  	select ARCH_HAVE_CUSTOM_GPIO_H
>  	select ARCH_HAVE_NMI_SAFE_CMPXCHG if (GUSA_RB || CPU_SH4A)
>  	select ARCH_HAS_GCOV_PROFILE_ALL
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
>  	select PERF_USE_VMALLOC
>  	select HAVE_DEBUG_KMEMLEAK
>  	select HAVE_KERNEL_GZIP
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index e8c3ea01c12f..f31700309621 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -30,6 +30,7 @@ config SPARC
>  	select RTC_SYSTOHC
>  	select HAVE_ARCH_JUMP_LABEL if SPARC64
>  	select GENERIC_IRQ_SHOW
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
>  	select ARCH_WANT_IPC_PARSE_VERSION
>  	select GENERIC_PCI_IOMAP
>  	select HAVE_NMI_WATCHDOG if SPARC64
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index b595ecb21a0f..e8edab974f67 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -85,6 +85,7 @@ config X86
>  	select ARCH_MIGHT_HAVE_ACPI_PDC		if ACPI
>  	select ARCH_MIGHT_HAVE_PC_PARPORT
>  	select ARCH_MIGHT_HAVE_PC_SERIO
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT	if HOTPLUG_CPU
>  	select ARCH_STACKWALK
>  	select ARCH_SUPPORTS_ACPI
>  	select ARCH_SUPPORTS_ATOMIC_RMW
> diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
> index 1c645172b4b5..c862dfa69ed9 100644
> --- a/arch/xtensa/Kconfig
> +++ b/arch/xtensa/Kconfig
> @@ -7,6 +7,7 @@ config XTENSA
>  	select ARCH_HAS_SYNC_DMA_FOR_CPU if MMU
>  	select ARCH_HAS_SYNC_DMA_FOR_DEVICE if MMU
>  	select ARCH_HAS_UNCACHED_SEGMENT if MMU
> +	select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
>  	select ARCH_USE_QUEUED_RWLOCKS
>  	select ARCH_USE_QUEUED_SPINLOCKS
>  	select ARCH_WANT_FRAME_POINTERS
> diff --git a/drivers/power/reset/sc27xx-poweroff.c b/drivers/power/reset/sc27xx-poweroff.c
> index 29fb08b8faa0..d6cdf837235c 100644
> --- a/drivers/power/reset/sc27xx-poweroff.c
> +++ b/drivers/power/reset/sc27xx-poweroff.c
> @@ -30,7 +30,7 @@ static void sc27xx_poweroff_shutdown(void)
>  #ifdef CONFIG_PM_SLEEP_SMP
>  	int cpu = smp_processor_id();
>  
> -	freeze_secondary_cpus(cpu);
> +	freeze_secondary_cpus(cpu, false);
>  #endif
>  }
>  
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 1ca2baf817ed..9c62274a4db9 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -137,11 +137,14 @@ static inline void cpu_hotplug_done(void) { cpus_write_unlock(); }
>  static inline void get_online_cpus(void) { cpus_read_lock(); }
>  static inline void put_online_cpus(void) { cpus_read_unlock(); }
>  
> +#if defined(CONFIG_PM_SLEEP_SMP) || defined(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
> +extern int freeze_secondary_cpus(int primary, bool reboot);
> +#endif
> +
>  #ifdef CONFIG_PM_SLEEP_SMP
> -extern int freeze_secondary_cpus(int primary);
>  static inline int disable_nonboot_cpus(void)
>  {
> -	return freeze_secondary_cpus(0);
> +	return freeze_secondary_cpus(0, false);
>  }
>  extern void enable_nonboot_cpus(void);
>  
> @@ -152,7 +155,7 @@ static inline int suspend_disable_secondary_cpus(void)
>  	if (IS_ENABLED(CONFIG_PM_SLEEP_SMP_NONZERO_CPU))
>  		cpu = -1;
>  
> -	return freeze_secondary_cpus(cpu);
> +	return freeze_secondary_cpus(cpu, false);
>  }
>  static inline void suspend_enable_secondary_cpus(void)
>  {
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 9c706af713fb..87d2c6f5ce4c 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1209,10 +1209,10 @@ int cpu_up(unsigned int cpu)
>  }
>  EXPORT_SYMBOL_GPL(cpu_up);
>  
> -#ifdef CONFIG_PM_SLEEP_SMP
> +#if defined(CONFIG_PM_SLEEP_SMP) || defined(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
>  static cpumask_var_t frozen_cpus;
>  
> -int freeze_secondary_cpus(int primary)
> +int freeze_secondary_cpus(int primary, bool reboot)
>  {
>  	int cpu, error = 0;
>  
> @@ -1237,20 +1237,25 @@ int freeze_secondary_cpus(int primary)
>  		if (cpu == primary)
>  			continue;
>  
> -		if (pm_wakeup_pending()) {
> +#ifdef CONFIG_PM_SLEEP
> +		if (!reboot && pm_wakeup_pending()) {

I would've moveed pm_wakeup_pending() check to callers
(disable_nonboot_cpus()/suspend_disable_secondary_cpus()) now.

>  			pr_info("Wakeup pending. Abort CPU freeze\n");
>  			error = -EBUSY;
>  			break;
>  		}
> +#endif
>  
> -		trace_suspend_resume(TPS("CPU_OFF"), cpu, true);
> +		if (!reboot)
> +			trace_suspend_resume(TPS("CPU_OFF"), cpu, true);
>  		error = _cpu_down(cpu, 1, CPUHP_OFFLINE);
> -		trace_suspend_resume(TPS("CPU_OFF"), cpu, false);
> +		if (!reboot)
> +			trace_suspend_resume(TPS("CPU_OFF"), cpu, false);

Does it actually hurt if we keep this for reboot case?

>  		if (!error)
>  			cpumask_set_cpu(cpu, frozen_cpus);
>  		else {
>  			pr_err("Error taking CPU%d down: %d\n", cpu, error);
> -			break;
> +			if (!reboot)
> +				break;

I would've added a comment like "When rebooting, try to offline as many
CPUs as possible".


>  		}
>  	}
>  
> diff --git a/kernel/reboot.c b/kernel/reboot.c
> index c4d472b7f1b4..e3bf515dc2bc 100644
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -7,6 +7,7 @@
>  
>  #define pr_fmt(fmt)	"reboot: " fmt
>  
> +#include <linux/cpu.h>
>  #include <linux/ctype.h>
>  #include <linux/export.h>
>  #include <linux/kexec.h>
> @@ -220,7 +221,9 @@ void migrate_to_reboot_cpu(void)
>  	/* The boot cpu is always logical cpu 0 */
>  	int cpu = reboot_cpu;
>  
> +#if !IS_ENABLED(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
>  	cpu_hotplug_disable();
> +#endif
>  
>  	/* Make certain the cpu I'm about to reboot on is online */
>  	if (!cpu_online(cpu))
> @@ -231,6 +234,11 @@ void migrate_to_reboot_cpu(void)
>  
>  	/* Make certain I only run on the appropriate processor */
>  	set_cpus_allowed_ptr(current, cpumask_of(cpu));
> +
> +#if IS_ENABLED(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
> +	/* Hotplug other cpus if possible */
> +	freeze_secondary_cpus(cpu, true);
> +#endif
>  }
>  
>  /**

-- 
Vitaly


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] reboot: support hotplug CPUs before reboot
  2020-01-14  9:45 ` Vitaly Kuznetsov
@ 2020-01-14 10:19   ` Hsin-Yi Wang
  0 siblings, 0 replies; 4+ messages in thread
From: Hsin-Yi Wang @ 2020-01-14 10:19 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Josh Poimboeuf, Ingo Molnar, Peter Zijlstra, Jiri Kosina,
	Pavankumar Kondeti, Aaro Koskinen, Greg Kroah-Hartman,
	Guenter Roeck, Stephen Boyd, lkml, Thomas Gleixner

On Tue, Jan 14, 2020 at 5:45 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> Hsin-Yi Wang <hsinyi@chromium.org> writes:
>
> > Currently system reboots uses architecture specific codes (smp_send_stop)
> > to offline non reboot CPUs. Most architecture's implementation is looping
> > through all non reboot online CPUs and call ipi function to each of them. Some
> > architecture like arm64, arm, and x86... would set offline masks to cpu without
> > really offline them. This causes some race condition and kernel warning comes
> > out sometimes when system reboots.
> >
> > This patch adds a config ARCH_OFFLINE_CPUS_ON_REBOOT, which would hotplug cpus in
> > migrate_to_reboot_cpu(). If non reboot cpus are all offlined here, the loop for
> > checking online cpus would be an empty loop. If architecture don't enable this
> > config, or some cpus somehow fails to offline, it would fallback to ipi
> > function.
> >
> > Opt in this config for architectures that support CONFIG_HOTPLUG_CPU.
> >
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > ---
> > Change from v3:
> > * Opt in config for architectures that support CONFIG_HOTPLUG_CPU
> > * Merge function offline_secondary_cpus() and freeze_secondary_cpus()
> >   with an additional flag.
>
> I haven't commented on the idea itself, actually (I'm not sure I'm the
> right person though but as I'm already looking at the code...)
>
> On x86, the custom IPI/NMI solution we have now is more 'lightweight'
> and here you suggest we switch to the full-blown CPUHP machinery
> instead. We seem to be already using it for suspend/hibernation,
> however, I would expect it to be way less tested: on servers, for
> example, less people do suspend/resume than reboot :-)
>
> The existing IPI/NMI code stays in place so nothing (in theory) should
> break -- unles some cpuhp_ hook does something really weird. But your
> change will definitely contribute to improving these hooks' quality in
> the long run :-)
>
> I have nothing to say on other arches and I don't see anyone from
> e.g. ARM on the CC: list, not good.
>
I'll resend to CC ARM as well, thanks.

The patch idea came from there are some warnings during reboot seen on
x86 and arm, arm64.
(x86) https://lore.kernel.org/lkml/20190727164450.GA11726@roeck-us.net/
(arm64) https://lkml.org/lkml/2012/8/22/3, resend version
https://lore.kernel.org/patchwork/patch/1117201/

> >
> > Change from v2:
> > * Add another config instead of configed by CONFIG_HOTPLUG_CPU
> > ---
> >  arch/Kconfig                          |  5 +++++
> >  arch/arm/Kconfig                      |  1 +
> >  arch/arm64/Kconfig                    |  1 +
> >  arch/arm64/kernel/hibernate.c         |  2 +-
> >  arch/csky/Kconfig                     |  1 +
> >  arch/ia64/Kconfig                     |  1 +
> >  arch/mips/Kconfig                     |  1 +
> >  arch/parisc/Kconfig                   |  1 +
> >  arch/powerpc/Kconfig                  |  1 +
> >  arch/s390/Kconfig                     |  1 +
> >  arch/sh/Kconfig                       |  1 +
> >  arch/sparc/Kconfig                    |  1 +
> >  arch/x86/Kconfig                      |  1 +
> >  arch/xtensa/Kconfig                   |  1 +
> >  drivers/power/reset/sc27xx-poweroff.c |  2 +-
> >  include/linux/cpu.h                   |  9 ++++++---
> >  kernel/cpu.c                          | 17 +++++++++++------
> >  kernel/reboot.c                       |  8 ++++++++
> >  18 files changed, 44 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 48b5e103bdb0..2ba3d62c98c5 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -255,6 +255,11 @@ config ARCH_HAS_UNCACHED_SEGMENT
> >       select ARCH_HAS_DMA_PREP_COHERENT
> >       bool
> >
> > +# Select to do a full hotplug on secondary CPUs before reboot.
> > +config ARCH_OFFLINE_CPUS_ON_REBOOT
> > +     bool "Support for hotplug CPUs before reboot"
> > +     depends on HOTPLUG_CPU
> > +
>
> Nit: what we're actually doing is not 'hotplug', it's either 'hotunplug' or
> 'offlining'.
>
> >  # Select if arch init_task must go in the __init_task_data section
> >  config ARCH_TASK_STRUCT_ON_STACK
> >       bool
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 69950fb5be64..d53cc8cb47e3 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -28,6 +28,7 @@ config ARM
> >       select ARCH_KEEP_MEMBLOCK if HAVE_ARCH_PFN_VALID || KEXEC
> >       select ARCH_MIGHT_HAVE_PC_PARPORT
> >       select ARCH_NO_SG_CHAIN if !ARM_HAS_SG_CHAIN
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
> >       select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX
> >       select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7
> >       select ARCH_SUPPORTS_ATOMIC_RMW
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 9af26ac75d19..9f913bc5c1f6 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -61,6 +61,7 @@ config ARM64
> >       select ARCH_INLINE_SPIN_UNLOCK_IRQ if !PREEMPTION
> >       select ARCH_INLINE_SPIN_UNLOCK_IRQRESTORE if !PREEMPTION
> >       select ARCH_KEEP_MEMBLOCK
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
> >       select ARCH_USE_CMPXCHG_LOCKREF
> >       select ARCH_USE_QUEUED_RWLOCKS
> >       select ARCH_USE_QUEUED_SPINLOCKS
> > diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> > index 590963c9c609..f7245dfa09d9 100644
> > --- a/arch/arm64/kernel/hibernate.c
> > +++ b/arch/arm64/kernel/hibernate.c
> > @@ -581,5 +581,5 @@ int hibernate_resume_nonboot_cpu_disable(void)
> >               return -ENODEV;
> >       }
> >
> > -     return freeze_secondary_cpus(sleep_cpu);
> > +     return freeze_secondary_cpus(sleep_cpu, false);
> >  }
> > diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
> > index 4acef4088de7..0f03e5c3f2fc 100644
> > --- a/arch/csky/Kconfig
> > +++ b/arch/csky/Kconfig
> > @@ -5,6 +5,7 @@ config CSKY
> >       select ARCH_HAS_DMA_PREP_COHERENT
> >       select ARCH_HAS_SYNC_DMA_FOR_CPU
> >       select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
> >       select ARCH_USE_BUILTIN_BSWAP
> >       select ARCH_USE_QUEUED_RWLOCKS if NR_CPUS>2
> >       select COMMON_CLK
> > diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> > index bab7cd878464..f12b4b11ee98 100644
> > --- a/arch/ia64/Kconfig
> > +++ b/arch/ia64/Kconfig
> > @@ -10,6 +10,7 @@ config IA64
> >       bool
> >       select ARCH_MIGHT_HAVE_PC_PARPORT
> >       select ARCH_MIGHT_HAVE_PC_SERIO
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
> >       select ACPI
> >       select ACPI_NUMA if NUMA
> >       select ARCH_SUPPORTS_ACPI
> > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > index b6b5f83af169..9bb2556d21fc 100644
> > --- a/arch/mips/Kconfig
> > +++ b/arch/mips/Kconfig
> > @@ -8,6 +8,7 @@ config MIPS
> >       select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> >       select ARCH_HAS_UBSAN_SANITIZE_ALL
> >       select ARCH_HAS_FORTIFY_SOURCE
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
> >       select ARCH_SUPPORTS_UPROBES
> >       select ARCH_USE_BUILTIN_BSWAP
> >       select ARCH_USE_CMPXCHG_LOCKREF if 64BIT
> > diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
> > index 71034b54d74e..41609f00b057 100644
> > --- a/arch/parisc/Kconfig
> > +++ b/arch/parisc/Kconfig
> > @@ -13,6 +13,7 @@ config PARISC
> >       select ARCH_HAS_STRICT_KERNEL_RWX
> >       select ARCH_HAS_UBSAN_SANITIZE_ALL
> >       select ARCH_NO_SG_CHAIN
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
> >       select ARCH_SUPPORTS_MEMORY_FAILURE
> >       select RTC_CLASS
> >       select RTC_DRV_GENERIC
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 658e0324d256..a6b76dd82a2d 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -142,6 +142,7 @@ config PPC
> >       select ARCH_KEEP_MEMBLOCK
> >       select ARCH_MIGHT_HAVE_PC_PARPORT
> >       select ARCH_MIGHT_HAVE_PC_SERIO
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT      if HOTPLUG_CPU
> >       select ARCH_OPTIONAL_KERNEL_RWX         if ARCH_HAS_STRICT_KERNEL_RWX
> >       select ARCH_SUPPORTS_ATOMIC_RMW
> >       select ARCH_USE_BUILTIN_BSWAP
> > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> > index 287714d51b47..19eec37b1682 100644
> > --- a/arch/s390/Kconfig
> > +++ b/arch/s390/Kconfig
> > @@ -102,6 +102,7 @@ config S390
> >       select ARCH_INLINE_WRITE_UNLOCK_IRQ
> >       select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
> >       select ARCH_KEEP_MEMBLOCK
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
> >       select ARCH_SAVE_PAGE_KEYS if HIBERNATION
> >       select ARCH_STACKWALK
> >       select ARCH_SUPPORTS_ATOMIC_RMW
> > diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
> > index 9ece111b0254..4ed1e0ca83a2 100644
> > --- a/arch/sh/Kconfig
> > +++ b/arch/sh/Kconfig
> > @@ -18,6 +18,7 @@ config SUPERH
> >       select ARCH_HAVE_CUSTOM_GPIO_H
> >       select ARCH_HAVE_NMI_SAFE_CMPXCHG if (GUSA_RB || CPU_SH4A)
> >       select ARCH_HAS_GCOV_PROFILE_ALL
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
> >       select PERF_USE_VMALLOC
> >       select HAVE_DEBUG_KMEMLEAK
> >       select HAVE_KERNEL_GZIP
> > diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> > index e8c3ea01c12f..f31700309621 100644
> > --- a/arch/sparc/Kconfig
> > +++ b/arch/sparc/Kconfig
> > @@ -30,6 +30,7 @@ config SPARC
> >       select RTC_SYSTOHC
> >       select HAVE_ARCH_JUMP_LABEL if SPARC64
> >       select GENERIC_IRQ_SHOW
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
> >       select ARCH_WANT_IPC_PARSE_VERSION
> >       select GENERIC_PCI_IOMAP
> >       select HAVE_NMI_WATCHDOG if SPARC64
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index b595ecb21a0f..e8edab974f67 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -85,6 +85,7 @@ config X86
> >       select ARCH_MIGHT_HAVE_ACPI_PDC         if ACPI
> >       select ARCH_MIGHT_HAVE_PC_PARPORT
> >       select ARCH_MIGHT_HAVE_PC_SERIO
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT      if HOTPLUG_CPU
> >       select ARCH_STACKWALK
> >       select ARCH_SUPPORTS_ACPI
> >       select ARCH_SUPPORTS_ATOMIC_RMW
> > diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
> > index 1c645172b4b5..c862dfa69ed9 100644
> > --- a/arch/xtensa/Kconfig
> > +++ b/arch/xtensa/Kconfig
> > @@ -7,6 +7,7 @@ config XTENSA
> >       select ARCH_HAS_SYNC_DMA_FOR_CPU if MMU
> >       select ARCH_HAS_SYNC_DMA_FOR_DEVICE if MMU
> >       select ARCH_HAS_UNCACHED_SEGMENT if MMU
> > +     select ARCH_OFFLINE_CPUS_ON_REBOOT if HOTPLUG_CPU
> >       select ARCH_USE_QUEUED_RWLOCKS
> >       select ARCH_USE_QUEUED_SPINLOCKS
> >       select ARCH_WANT_FRAME_POINTERS
> > diff --git a/drivers/power/reset/sc27xx-poweroff.c b/drivers/power/reset/sc27xx-poweroff.c
> > index 29fb08b8faa0..d6cdf837235c 100644
> > --- a/drivers/power/reset/sc27xx-poweroff.c
> > +++ b/drivers/power/reset/sc27xx-poweroff.c
> > @@ -30,7 +30,7 @@ static void sc27xx_poweroff_shutdown(void)
> >  #ifdef CONFIG_PM_SLEEP_SMP
> >       int cpu = smp_processor_id();
> >
> > -     freeze_secondary_cpus(cpu);
> > +     freeze_secondary_cpus(cpu, false);
> >  #endif
> >  }
> >
> > diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> > index 1ca2baf817ed..9c62274a4db9 100644
> > --- a/include/linux/cpu.h
> > +++ b/include/linux/cpu.h
> > @@ -137,11 +137,14 @@ static inline void cpu_hotplug_done(void) { cpus_write_unlock(); }
> >  static inline void get_online_cpus(void) { cpus_read_lock(); }
> >  static inline void put_online_cpus(void) { cpus_read_unlock(); }
> >
> > +#if defined(CONFIG_PM_SLEEP_SMP) || defined(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
> > +extern int freeze_secondary_cpus(int primary, bool reboot);
> > +#endif
> > +
> >  #ifdef CONFIG_PM_SLEEP_SMP
> > -extern int freeze_secondary_cpus(int primary);
> >  static inline int disable_nonboot_cpus(void)
> >  {
> > -     return freeze_secondary_cpus(0);
> > +     return freeze_secondary_cpus(0, false);
> >  }
> >  extern void enable_nonboot_cpus(void);
> >
> > @@ -152,7 +155,7 @@ static inline int suspend_disable_secondary_cpus(void)
> >       if (IS_ENABLED(CONFIG_PM_SLEEP_SMP_NONZERO_CPU))
> >               cpu = -1;
> >
> > -     return freeze_secondary_cpus(cpu);
> > +     return freeze_secondary_cpus(cpu, false);
> >  }
> >  static inline void suspend_enable_secondary_cpus(void)
> >  {
> > diff --git a/kernel/cpu.c b/kernel/cpu.c
> > index 9c706af713fb..87d2c6f5ce4c 100644
> > --- a/kernel/cpu.c
> > +++ b/kernel/cpu.c
> > @@ -1209,10 +1209,10 @@ int cpu_up(unsigned int cpu)
> >  }
> >  EXPORT_SYMBOL_GPL(cpu_up);
> >
> > -#ifdef CONFIG_PM_SLEEP_SMP
> > +#if defined(CONFIG_PM_SLEEP_SMP) || defined(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
> >  static cpumask_var_t frozen_cpus;
> >
> > -int freeze_secondary_cpus(int primary)
> > +int freeze_secondary_cpus(int primary, bool reboot)
> >  {
> >       int cpu, error = 0;
> >
> > @@ -1237,20 +1237,25 @@ int freeze_secondary_cpus(int primary)
> >               if (cpu == primary)
> >                       continue;
> >
> > -             if (pm_wakeup_pending()) {
> > +#ifdef CONFIG_PM_SLEEP
> > +             if (!reboot && pm_wakeup_pending()) {
>
> I would've moveed pm_wakeup_pending() check to callers
> (disable_nonboot_cpus()/suspend_disable_secondary_cpus()) now.
>
pm_wakeup_pending() is in for_each_online_cpu() loop. Quoting from
commit message of a66d955e910:
"CPU hotplug is a slow operation, so it makes sense to check for wakeup
pending in the freezer loop before bringing down the next CPU. This
improves the system suspend abort latency significantly."

In reboot case don't have to check for wakeup pending condition. But
for suspend it should check between bringing down CPUs.
> >                       pr_info("Wakeup pending. Abort CPU freeze\n");
> >                       error = -EBUSY;
> >                       break;
> >               }
> > +#endif
> >
> > -             trace_suspend_resume(TPS("CPU_OFF"), cpu, true);
> > +             if (!reboot)
> > +                     trace_suspend_resume(TPS("CPU_OFF"), cpu, true);
> >               error = _cpu_down(cpu, 1, CPUHP_OFFLINE);
> > -             trace_suspend_resume(TPS("CPU_OFF"), cpu, false);
> > +             if (!reboot)
> > +                     trace_suspend_resume(TPS("CPU_OFF"), cpu, false);
>
> Does it actually hurt if we keep this for reboot case?
>
Okay I'll keep them with reboot case.

> >               if (!error)
> >                       cpumask_set_cpu(cpu, frozen_cpus);
> >               else {
> >                       pr_err("Error taking CPU%d down: %d\n", cpu, error);
> > -                     break;
> > +                     if (!reboot)
> > +                             break;
>
> I would've added a comment like "When rebooting, try to offline as many
> CPUs as possible".
>
>
> >               }
> >       }
> >
> > diff --git a/kernel/reboot.c b/kernel/reboot.c
> > index c4d472b7f1b4..e3bf515dc2bc 100644
> > --- a/kernel/reboot.c
> > +++ b/kernel/reboot.c
> > @@ -7,6 +7,7 @@
> >
> >  #define pr_fmt(fmt)  "reboot: " fmt
> >
> > +#include <linux/cpu.h>
> >  #include <linux/ctype.h>
> >  #include <linux/export.h>
> >  #include <linux/kexec.h>
> > @@ -220,7 +221,9 @@ void migrate_to_reboot_cpu(void)
> >       /* The boot cpu is always logical cpu 0 */
> >       int cpu = reboot_cpu;
> >
> > +#if !IS_ENABLED(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
> >       cpu_hotplug_disable();
> > +#endif
> >
> >       /* Make certain the cpu I'm about to reboot on is online */
> >       if (!cpu_online(cpu))
> > @@ -231,6 +234,11 @@ void migrate_to_reboot_cpu(void)
> >
> >       /* Make certain I only run on the appropriate processor */
> >       set_cpus_allowed_ptr(current, cpumask_of(cpu));
> > +
> > +#if IS_ENABLED(CONFIG_ARCH_OFFLINE_CPUS_ON_REBOOT)
> > +     /* Hotplug other cpus if possible */
> > +     freeze_secondary_cpus(cpu, true);
> > +#endif
> >  }
> >
> >  /**
>
> --
> Vitaly
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] reboot: support hotplug CPUs before reboot
  2020-01-14  6:30 [PATCH v4] reboot: support hotplug CPUs before reboot Hsin-Yi Wang
  2020-01-14  9:45 ` Vitaly Kuznetsov
@ 2020-01-17  8:23 ` kbuild test robot
  1 sibling, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2020-01-17  8:23 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: kbuild-all, Thomas Gleixner, Josh Poimboeuf, Ingo Molnar,
	Peter Zijlstra, Jiri Kosina, Pavankumar Kondeti,
	Vitaly Kuznetsov, Aaro Koskinen, Greg Kroah-Hartman,
	Guenter Roeck, Stephen Boyd, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4426 bytes --]

Hi Hsin-Yi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20200110]
[also build test ERROR on v5.5-rc6]
[cannot apply to arm64/for-next/core arm/for-next ia64/next hp-parisc/for-next powerpc/next s390/features sparc/master tip/x86/core linus/master sparc-next/master v5.5-rc6 v5.5-rc5 v5.5-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Hsin-Yi-Wang/reboot-support-hotplug-CPUs-before-reboot/20200114-143340
base:    6c09d7dbb7d366122d0218bc7487e0a1e6cca6ed
config: powerpc-rhel-kconfig (attached as .config)
compiler: powerpc64le-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> kernel/cpu.c:1286:6: error: redefinition of 'enable_nonboot_cpus'
    void enable_nonboot_cpus(void)
         ^~~~~~~~~~~~~~~~~~~
   In file included from kernel/cpu.c:16:0:
   include/linux/cpu.h:167:20: note: previous definition of 'enable_nonboot_cpus' was here
    static inline void enable_nonboot_cpus(void) {}
                       ^~~~~~~~~~~~~~~~~~~

vim +/enable_nonboot_cpus +1286 kernel/cpu.c

d0af9eed5aa91b Suresh Siddha     2009-08-19  1285  
71cf5aeeb8e215 Mathias Krause    2015-07-19 @1286  void enable_nonboot_cpus(void)
e3920fb42c8ddf Rafael J. Wysocki 2006-09-25  1287  {
e3920fb42c8ddf Rafael J. Wysocki 2006-09-25  1288  	int cpu, error;
e3920fb42c8ddf Rafael J. Wysocki 2006-09-25  1289  
e3920fb42c8ddf Rafael J. Wysocki 2006-09-25  1290  	/* Allow everyone to use the CPU hotplug again */
d221938c049f48 Gautham R Shenoy  2008-01-25  1291  	cpu_maps_update_begin();
01b41159066531 Lianwei Wang      2016-06-09  1292  	__cpu_hotplug_enable();
e0b582ec56f1a1 Rusty Russell     2009-01-01  1293  	if (cpumask_empty(frozen_cpus))
1d64b9cb1dc2a7 Rafael J. Wysocki 2007-04-01  1294  		goto out;
e3920fb42c8ddf Rafael J. Wysocki 2006-09-25  1295  
84117da5b79ffb Fabian Frederick  2014-06-04  1296  	pr_info("Enabling non-boot CPUs ...\n");
d0af9eed5aa91b Suresh Siddha     2009-08-19  1297  
d0af9eed5aa91b Suresh Siddha     2009-08-19  1298  	arch_enable_nonboot_cpus_begin();
d0af9eed5aa91b Suresh Siddha     2009-08-19  1299  
e0b582ec56f1a1 Rusty Russell     2009-01-01  1300  	for_each_cpu(cpu, frozen_cpus) {
bb3632c6101b2f Todd E Brandt     2014-06-06  1301  		trace_suspend_resume(TPS("CPU_ON"), cpu, true);
af1f40457da6f7 Thomas Gleixner   2016-02-26  1302  		error = _cpu_up(cpu, 1, CPUHP_ONLINE);
bb3632c6101b2f Todd E Brandt     2014-06-06  1303  		trace_suspend_resume(TPS("CPU_ON"), cpu, false);
e3920fb42c8ddf Rafael J. Wysocki 2006-09-25  1304  		if (!error) {
84117da5b79ffb Fabian Frederick  2014-06-04  1305  			pr_info("CPU%d is up\n", cpu);
e3920fb42c8ddf Rafael J. Wysocki 2006-09-25  1306  			continue;
e3920fb42c8ddf Rafael J. Wysocki 2006-09-25  1307  		}
84117da5b79ffb Fabian Frederick  2014-06-04  1308  		pr_warn("Error taking CPU%d up: %d\n", cpu, error);
e3920fb42c8ddf Rafael J. Wysocki 2006-09-25  1309  	}
d0af9eed5aa91b Suresh Siddha     2009-08-19  1310  
d0af9eed5aa91b Suresh Siddha     2009-08-19  1311  	arch_enable_nonboot_cpus_end();
d0af9eed5aa91b Suresh Siddha     2009-08-19  1312  
e0b582ec56f1a1 Rusty Russell     2009-01-01  1313  	cpumask_clear(frozen_cpus);
1d64b9cb1dc2a7 Rafael J. Wysocki 2007-04-01  1314  out:
d221938c049f48 Gautham R Shenoy  2008-01-25  1315  	cpu_maps_update_done();
^1da177e4c3f41 Linus Torvalds    2005-04-16  1316  }
e0b582ec56f1a1 Rusty Russell     2009-01-01  1317  

:::::: The code at line 1286 was first introduced by commit
:::::: 71cf5aeeb8e2154efda5f40be50c925f15057755 kernel, cpu: Remove bogus __ref annotations

:::::: TO: Mathias Krause <minipli@googlemail.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 15306 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-01-17  8:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14  6:30 [PATCH v4] reboot: support hotplug CPUs before reboot Hsin-Yi Wang
2020-01-14  9:45 ` Vitaly Kuznetsov
2020-01-14 10:19   ` Hsin-Yi Wang
2020-01-17  8:23 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).