Fix hibernate on SMP spin-table systems
cover Fix hibernate on SMP spin-table systems
Hi all,
These two patches prevent hibernate on systems that use spin-tables and have multiple CPUs. It also wires up 'cpus_stuck_in_kernel' which was added for v4.7.
Prior to 44dbcc93ab67 ("arm64: Fix behavior of maxcpus=N"), we would bring all the CPUs we would ever have up during boot. On a system with spin tables and multiple CPUS the core hibernate code would prevent hibernation because it can't disable secondary CPUs.
After 44dbcc93ab67, when we boot with 'maxcpus=1', we no longer bring all the CPUs up, but we do move them into the secondary_holding_pen. Resuming from hibernate will overwrite the secondary_holding_pen, potentially releasing the secondary CPUs. If the kernel has been loaded at a different physical address over hibernate and resume the secondary_holding_pen may be at a different location after resume. The core code can't help us with this, because these CPUs don't show up in 'num_online_cpus()'
These two patches fix the problem by detecting multiple 'possible cpus' that we have no mechanism to take offline and preventing hibernate [2]. This only happens for spin-table systems with multiple CPUs.
Kexec needs the same checks, so the 'or spin-tables' logic[0] got added to the helper function.
(This problem was spotted on another thread [1])
[0] http://www.spinics.net/lists/arm-kernel/msg510097.html
[1] http://www.spinics.net/lists/arm-kernel/msg511880.html
[2] Failing to hibernate an SMP spin-tables system booted with maxcpus=1 ---------------------%<---------------------
root@localhost:~# echo disk > /sys/power/state [12248.197718] PM: Syncing filesystems ... done. [12248.197727] Freezing user space processes ... (elapsed 0.001 seconds) done. [12248.203197] PM: Preallocating image memory... done (allocated 50769 pages) [12261.838699] PM: Allocated 203076 kbytes in 13.63 seconds (14.89 MB/s) [12261.838760] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [12261.840732] Suspending console(s) (use no_console_suspend to debug) [12261.842540] PM: freeze of devices complete after 1.732 msecs [12261.843897] PM: late freeze of devices complete after 1.333 msecs [12261.845191] PM: noirq freeze of devices complete after 1.272 msecs [12261.845201] Disabling non-boot CPUs ... [12261.845206] hibernate: Can't hibernate: no mechanism to offline secondary CPUs. [12261.845206] PM: Error -16 creating hibernation image [12261.846140] PM: noirq recover of devices complete after 0.908 msecs [12261.847160] PM: early recover of devices complete after 0.940 msecs [12262.765886] PM: recover of devices complete after 1.452 msecs [12262.769191] Restarting tasks ... done. -bash: echo: write error: Device or resource busy root at localhost:~# ---------------------%<---------------------
James Morse (2):
arm64: smp: Add function to determine if cpus are stuck in the kernel
arm64: hibernate: Don't hibernate on systems with stuck CPUs
arch/arm64/include/asm/smp.h | 20 ++++++++++++++++++++ arch/arm64/kernel/hibernate.c | 6 ++++++ arch/arm64/kernel/smp.c | 13 +++++++++++++ 3 files changed, 39 insertions(+)
Signature · 1 line
2.8.0.rc3
PATCH 1/2 arm64: smp: Add function to determine if cpus are stuck in the kernel
kernel/smp.c has a fancy counter that keeps track of the number of CPUs it marked as not-present and left in cpu_park_loop(). If there are any CPUs spinning in here, features like kexec or hibernate may release them by overwriting this memory.
This problem also occurs on machines using spin-tables to release secondary cores.
After commit 44dbcc93ab67 ("arm64: Fix behavior of maxcpus=N") we bring all known cpus into the secondary holding pen, but may not bring them up depending on 'maxcpus'. This memory can't be re-used by kexec or hibernate.
Add a function cpus_are_stuck_in_kernel() to determine if either of these cases have occurred.
--- arch/arm64/include/asm/smp.h | 20 ++++++++++++++++++++ arch/arm64/kernel/smp.c | 13 +++++++++++++ 2 files changed, 33 insertions(+)
arch/arm64/include/asm/smp.h +20 -0
@@ -124,6 +124,26 @@ static inline void cpu_panic_kernel(void) cpu_park_loop(); } +/* +* Kernel features such as hibernate and kexec depend on cpu hotplug to know +* they can replace any kernel memory they are not using themselves. +* +* There are two corner cases: +* If a secondary CPU fails to come online, (e.g. due to mismatched features), +* it will try to call cpu_die(). If this fails, it increases the counter +* cpus_stuck_in_kernel and sits in cpu_park_loop(). The memory containing +* this function must not be re-used for anything else as the 'stuck' core +* is executing it. +* +* CPUs are also considered stuck in the kernel if we have multiple CPUs +* and no way to offline secondary CPUs. This happens when secondaries +* are released via spin-table, these CPUs are moved into the kernel's +* secondary_holding_pen, which must not be overwritten. +* +* This function is used to inhibit features like kexec and hibernate. +*/ +bool cpus_are_stuck_in_kernel (void ); +#endif /* ifndef __ASSEMBLY__ */ #endif /* ifndef __ASM_SMP_H */
arch/arm64/kernel/smp.c +13 -0
@@ -909,3 +909,16 @@ int setup_profiling_timer(unsigned int multiplier) {return -EINVAL ; } + +bool cpus_are_stuck_in_kernel (void ) +{ +bool ret = !!cpus_stuck_in_kernel; +#ifdef CONFIG_HOTPLUG_CPU +int any_cpu =raw_smp_processor_id (); + +if (num_possible_cpus () >1 && !cpu_ops[any_cpu]->cpu_die) + ret = true; +#endif + +return ret; +}
Signature · 1 line
2.8.0.rc3
Mark Rutland
reviewerPATCH 1/2 arm64: smp: Add function to determine if cpus are stuck in the kernel
Quoted 30 lines · James Morse
kernel/smp.chas a fancy counter that keeps track of the number of CPUs it marked as not-present and left incpu_park_loop(). If there are any CPUs spinning in here, features like kexec or hibernate may release them by overwriting this memory.This problem also occurs on machines using spin-tables to release secondary cores.
After commit44dbcc93ab67("arm64: Fix behavior of maxcpus=N") we bring all known cpus into the secondary holding pen, but may not bring them up depending on 'maxcpus'. This memory can't be re-used by kexec or hibernate.Add a function
cpus_are_stuck_in_kernel()to determine if either of these cases have occurred.Signed-off-by James Morse <[email protected]>Cc Suzuki K Poulose <[email protected]>--- arch/arm64/include/asm/smp.h | 20 ++++++++++++++++++++ arch/arm64/kernel/smp.c | 13 +++++++++++++ 2 files changed, 33 insertions(+)arch/arm64/include/asm/smp.h +0
-0@@ -124,6 +124,26 @@ static inline void cpu_panic_kernel(void)cpu_park_loop (); }
arch/arm64/include/asm/smp.h static inline void cpu_panic_kernel(void) · line 136
Quoted 10 lines
arch/arm64/include/asm/smp.h +10
-0+/* +* Kernel features such as hibernate and kexec depend on cpu hotplug to know +* they can replace any kernel memory they are not using themselves. +* +* There are two corner cases: +* If a secondary CPU fails to come online, (e.g. due to mismatched features), +* it will try to call cpu_die(). If this fails, it increases the counter +* cpus_stuck_in_kernel and sits in cpu_park_loop(). The memory containing +* this function must not be re-used for anything else as the 'stuck' core +* is executing it.
It might also be stuck in __no_granule_support, if it never made it to C code. In that case, the CPU in charge of bringing up that new CPU will increment the counter in __cpu_up.
There might be other reasons we do something like that in future, so it might be better to be a little less specific and say something like:
If a secondary CPU enters the kernel but fails to come online, (e.g. due to mismatched features), and cannot exit the kernel, we increment cpus_stuck_in_kernel and leave the CPU in a quiesecent loop within the kernel text. The memory containing this loop must not be re-used for anything else as the 'stuck' core is executing it.
Otherwise, this looks good. FWIW, either way:
Thanks,
Mark.
Quoted 37 lines
arch/arm64/include/asm/smp.h +10
-0+* +* CPUs are also considered stuck in the kernel if we have multiple CPUs +* and no way to offline secondary CPUs. This happens when secondaries +* are released via spin-table, these CPUs are moved into the kernel's +* secondary_holding_pen, which must not be overwritten. +* +* This function is used to inhibit features like kexec and hibernate. +*/ +bool cpus_are_stuck_in_kernel (void ); +#endif /* ifndef __ASSEMBLY__ */ #endif /* ifndef __ASM_SMP_H */ arch/arm64/kernel/smp.c +13
-0@@ -909,3 +909,16 @@ int setup_profiling_timer(unsigned int multiplier) {return -EINVAL ; } + +bool cpus_are_stuck_in_kernel (void ) +{ +bool ret = !!cpus_stuck_in_kernel; +#ifdef CONFIG_HOTPLUG_CPU +int any_cpu =raw_smp_processor_id (); + +if (num_possible_cpus () >1 && !cpu_ops[any_cpu]->cpu_die) + ret = true; +#endif + +return ret; +}Signature · 1 line
2.8.0.rc3
PATCH 1/2 arm64: smp: Add function to determine if cpus are stuck in the kernel
Quoted 31 lines · Mark Rutland
Quoted 30 lines · James Morse
kernel/smp.chas a fancy counter that keeps track of the number of CPUs it marked as not-present and left incpu_park_loop(). If there are any CPUs spinning in here, features like kexec or hibernate may release them by overwriting this memory.This problem also occurs on machines using spin-tables to release secondary cores.
After commit44dbcc93ab67("arm64: Fix behavior of maxcpus=N") we bring all known cpus into the secondary holding pen, but may not bring them up depending on 'maxcpus'. This memory can't be re-used by kexec or hibernate.Add a function
cpus_are_stuck_in_kernel()to determine if either of these cases have occurred.Signed-off-by James Morse <[email protected]>Cc Suzuki K Poulose <[email protected]>--- arch/arm64/include/asm/smp.h | 20 ++++++++++++++++++++ arch/arm64/kernel/smp.c | 13 +++++++++++++ 2 files changed, 33 insertions(+)arch/arm64/include/asm/smp.h +0
-0@@ -124,6 +124,26 @@ static inline void cpu_panic_kernel(void)cpu_park_loop (); }
Quoted 14 lines
arch/arm64/include/asm/smp.h static inline void cpu_panic_kernel(void) · line 136
Quoted 10 lines
arch/arm64/include/asm/smp.h +10
-0+/* +* Kernel features such as hibernate and kexec depend on cpu hotplug to know +* they can replace any kernel memory they are not using themselves. +* +* There are two corner cases: +* If a secondary CPU fails to come online, (e.g. due to mismatched features), +* it will try to call cpu_die(). If this fails, it increases the counter +* cpus_stuck_in_kernel and sits in cpu_park_loop(). The memory containing +* this function must not be re-used for anything else as the 'stuck' core +* is executing it. It might also be stuck in
__no_granule_support, if it never made it to C code. In that case, the CPU in charge of bringing up that new CPU will increment the counter in__cpu_up.
Just to clarify, *in all the cases*, the CPU in charge of bringing up updates the cpus_stuck_in_kernel.
Quoted 10 lines
There might be other reasons we do something like that in future, so it might be better to be a little less specific and say something like:
If a secondary CPU enters the kernel but fails to come online, (e.g. due to mismatched features), and cannot exit the kernel, we increment
cpus_stuck_in_kerneland leave the CPU in a quiesecent loop within the kernel text. The memory containing this loop must not be re-used for anything else as the 'stuck' core is executing it.
Agree.
Quoted 15 lines
Quoted 15 lines
arch/arm64/include/asm/smp.h +2
-0+bool cpus_are_stuck_in_kernel (void ); +#endif /* ifndef __ASSEMBLY__ */ #endif /* ifndef __ASM_SMP_H */ arch/arm64/kernel/smp.c +2
-0@@ -909,3 +909,16 @@ int setup_profiling_timer(unsigned int multiplier) {return -EINVAL ; } + +bool cpus_are_stuck_in_kernel (void )
Quoted 8 lines
arch/arm64/kernel/smp.c int setup_profiling_timer(unsigned int multiplier) · line 921
Quoted 8 lines
arch/arm64/kernel/smp.c +8
-0+{ +bool ret = !!cpus_stuck_in_kernel; +#ifdef CONFIG_HOTPLUG_CPU +int any_cpu =raw_smp_processor_id (); + +if (num_possible_cpus () >1 && !cpu_ops[any_cpu]->cpu_die) + ret = true; +#endif
Minor nit: Moving the cpu_die check to a static inline function with an obvious name might make the code look better.
return !!cpus_stuck_in_kernel || !have_cpu_die() ?
Eitherway,
Cheers
Suzuki
Quoted 7 lines
Quoted 6 lines
arch/arm64/kernel/smp.c +3
-1+ +return ret; +} --2.8.0.rc3
PATCH 1/2 arm64: smp: Add function to determine if cpus are stuck in the kernel
Hi Suzuki,
Quoted 17 lines · Suzuki K Poulose
Quoted 15 lines · Mark Rutland
Quoted 14 lines · James Morse
kernel/smp.chas a fancy counter that keeps track of the number of CPUs it marked as not-present and left incpu_park_loop(). If there are any CPUs spinning in here, features like kexec or hibernate may release them by overwriting this memory.This problem also occurs on machines using spin-tables to release secondary cores.
After commit44dbcc93ab67("arm64: Fix behavior of maxcpus=N") we bring all known cpus into the secondary holding pen, but may not bring them up depending on 'maxcpus'. This memory can't be re-used by kexec or hibernate.Add a function
cpus_are_stuck_in_kernel()to determine if either of these cases have occurred.
Quoted 6 lines
Quoted 3 lines
It might also be stuck in
__no_granule_support, if it never made it to C code. In that case, the CPU in charge of bringing up that new CPU will increment the counter in__cpu_up.Just to clarify, *in all the cases*, the CPU in charge of bringing up updates the
cpus_stuck_in_kernel.
Ah, my mistake. I will switch it for Mark's suggestion.
Quoted 22 lines
Quoted 10 lines
Quoted 10 lines
arch/arm64/kernel/smp.c +2
-0@@ -909,3 +909,16 @@ int setup_profiling_timer(unsigned int multiplier) {return -EINVAL ; } + +bool cpus_are_stuck_in_kernel (void )Quoted 8 lines
arch/arm64/kernel/smp.c int setup_profiling_timer(unsigned int multiplier) · line 921
Quoted 8 lines
arch/arm64/kernel/smp.c +8
-0+{ +bool ret = !!cpus_stuck_in_kernel; +#ifdef CONFIG_HOTPLUG_CPU +int any_cpu =raw_smp_processor_id (); + +if (num_possible_cpus () >1 && !cpu_ops[any_cpu]->cpu_die) + ret = true; +#endif Minor nit: Moving the
cpu_diecheck to a static inline function with an obvious name might make the code look better.
Quoted 2 lines
return !!cpus_stuck_in_kernel || !have_cpu_die() ?
That would be better!
Quoted 3 lines
Eitherway,
Reviewed-by Suzuki K Poulose <[email protected]>
Thanks,
James
PATCH 1/2 arm64: smp: Add function to determine if cpus are stuck in the kernel
HI James,
Quoted 49 lines · James Morse
Quoted 17 lines · Suzuki K Poulose
Quoted 15 lines · Mark Rutland
Quoted 14 lines · James Morse
kernel/smp.chas a fancy counter that keeps track of the number of CPUs it marked as not-present and left incpu_park_loop(). If there are any CPUs spinning in here, features like kexec or hibernate may release them by overwriting this memory.This problem also occurs on machines using spin-tables to release secondary cores.
After commit44dbcc93ab67("arm64: Fix behavior of maxcpus=N") we bring all known cpus into the secondary holding pen, but may not bring them up depending on 'maxcpus'. This memory can't be re-used by kexec or hibernate.Add a function
cpus_are_stuck_in_kernel()to determine if either of these cases have occurred.Quoted 6 lines
Quoted 3 lines
It might also be stuck in
__no_granule_support, if it never made it to C code. In that case, the CPU in charge of bringing up that new CPU will increment the counter in__cpu_up.Just to clarify, *in all the cases*, the CPU in charge of bringing up updates the
cpus_stuck_in_kernel.Ah, my mistake. I will switch it for Mark's suggestion.
Quoted 22 lines
Quoted 10 lines
Quoted 10 lines
arch/arm64/kernel/smp.c +2
-0@@ -909,3 +909,16 @@ int setup_profiling_timer(unsigned int multiplier) {return -EINVAL ; } + +bool cpus_are_stuck_in_kernel (void )Quoted 8 lines
arch/arm64/kernel/smp.c int setup_profiling_timer(unsigned int multiplier) · line 921
Quoted 8 lines
arch/arm64/kernel/smp.c +8
-0+{ +bool ret = !!cpus_stuck_in_kernel; +#ifdef CONFIG_HOTPLUG_CPU +int any_cpu =raw_smp_processor_id (); + +if (num_possible_cpus () >1 && !cpu_ops[any_cpu]->cpu_die) + ret = true; +#endif Minor nit: Moving the
cpu_diecheck to a static inline function with an obvious name might make the code look better.
Quoted 4 lines
Quoted 2 lines
return !!cpus_stuck_in_kernel || !have_cpu_die() ?That would be better!
Can you post a new version of this, please?
Will
PATCH 1/2 arm64: smp: Add function to determine if cpus are stuck in the kernel
In reply to James Morsearch/arm64/kernel/smp.c int setup_profiling_timer(unsigned int multiplier) · line 916
Quoted 4 lines · James Morse
arch/arm64/kernel/smp.c +4
-0+bool cpus_are_stuck_in_kernel (void ) +{ +> > bool ret = !!cpus_stuck_in_kernel; +#ifdef CONFIG_HOTPLUG_CPU
How about using 'if (IS_ENABLED(CONFIG_HOTPLUG_CPU))' here?
arch/arm64/kernel/smp.c int setup_profiling_timer(unsigned int multiplier) · line 921
Quoted 5 lines
arch/arm64/kernel/smp.c +5
-0+int any_cpu =raw_smp_processor_id (); + +> > if (num_possible_cpus() > 1 && !cpu_ops[any_cpu]->cpu_die) +> > > ret = true; +#endif
-Geoff
PATCH 1/2 arm64: smp: Add function to determine if cpus are stuck in the kernel
Quoted 8 lines · Geoff Levand
arch/arm64/kernel/smp.c int setup_profiling_timer(unsigned int multiplier) · line 916
Quoted 4 lines · James Morse
arch/arm64/kernel/smp.c +4
-0+bool cpus_are_stuck_in_kernel (void ) +{ +> > bool ret = !!cpus_stuck_in_kernel; +#ifdef CONFIG_HOTPLUG_CPU How about using 'if (
IS_ENABLED(CONFIG_HOTPLUG_CPU))' here?
Quoted 6 lines
arch/arm64/kernel/smp.c int setup_profiling_timer(unsigned int multiplier) · line 921
Quoted 5 lines
arch/arm64/kernel/smp.c +5
-0+int any_cpu =raw_smp_processor_id (); + +> > if (num_possible_cpus() > 1 && !cpu_ops[any_cpu]->cpu_die) +> > > ret = true; +#endif
That won't work, as the cpu_ops->cpu_die is defined only if CONFIG_HOTPLUG_CPU.
May be we should fix that.
Suzuki
Mark Rutland
reviewerPATCH 1/2 arm64: smp: Add function to determine if cpus are stuck in the kernel
In reply to Geoff LevandQuoted 7 lines · Geoff Levand
arch/arm64/kernel/smp.c int setup_profiling_timer(unsigned int multiplier) · line 916
Quoted 4 lines · James Morse
arch/arm64/kernel/smp.c +4
-0+bool cpus_are_stuck_in_kernel (void ) +{ +> > bool ret = !!cpus_stuck_in_kernel; +#ifdef CONFIG_HOTPLUG_CPU How about using 'if (
IS_ENABLED(CONFIG_HOTPLUG_CPU))' here?
I wanted to suggest that too, but ...
Quoted 4 lines
arch/arm64/kernel/smp.c int setup_profiling_timer(unsigned int multiplier)
Quoted 4 lines
arch/arm64/kernel/smp.c +4
-0+int any_cpu =raw_smp_processor_id (); + +> > if (num_possible_cpus() > 1 && !cpu_ops[any_cpu]->cpu_die) +> > > ret = true;
... here we'd blow up as cpu_ops::cpu_die is only defined when CONFIG_HOTPLUG_CPU is selected.
So we'll have to use an ifdef, but we could instead ifdef a whole stub function (e.g. have_cpu_die()), as Suzuki suggested.
Thanks,
Mark.
PATCH 2/2 arm64: hibernate: Don't hibernate on systems with stuck CPUs
In reply to James MorseHibernate relies on cpu hotplug to prevent secondary cores executing the kernel text while it is being restored.
Add a call to cpus_are_stuck_in_kernel() to determine if there are CPUs not counted by 'num_online_cpus()', and prevent hibernate in this case.
--- arch/arm64/kernel/hibernate.c | 6 ++++++ 1 file changed, 6 insertions(+)
arch/arm64/kernel/hibernate.c +6 -0
@@ -33,6 +33,7 @@#include <asm/pgtable.h> #include <asm/pgtable-hwdef.h> #include <asm/sections.h> +#include <asm/smp.h> #include <asm/suspend.h> #include <asm/virt.h>
@@ -236,6 +237,11 @@ int swsusp_arch_suspend(void)unsigned long flags;struct sleep_stack_data state; +if (cpus_are_stuck_in_kernel ()) { +pr_err ("Can't hibernate: no mechanism to offline secondary CPUs.\n" ); +return -EBUSY ; + } +local_dbg_save (flags);if (__cpu_suspend_enter (&state)) {
Signature · 1 line
2.8.0.rc3
Mark Rutland
reviewerPATCH 2/2 arm64: hibernate: Don't hibernate on systems with stuck CPUs
Quoted 9 lines · James Morse
Hibernate relies on cpu hotplug to prevent secondary cores executing the kernel text while it is being restored.
Add a call to
cpus_are_stuck_in_kernel()to determine if there are CPUs not counted by 'num_online_cpus()', and prevent hibernate in this case.Fixes 82869ac57b5 ("arm64: kernel: Add support for hibernate/suspend-to-disk")Signed-off-by James Morse <[email protected]>
Mark.
Quoted 31 lines
--- arch/arm64/kernel/hibernate.c | 6 ++++++ 1 file changed, 6 insertions(+)arch/arm64/kernel/hibernate.c +6
-0@@ -33,6 +33,7 @@#include <asm/pgtable.h> #include <asm/pgtable-hwdef.h> #include <asm/sections.h> +#include <asm/smp.h> #include <asm/suspend.h> #include <asm/virt.h> @@ -236,6 +237,11 @@ int swsusp_arch_suspend(void)unsigned long flags;struct sleep_stack_data state; +if (cpus_are_stuck_in_kernel ()) { +pr_err ("Can't hibernate: no mechanism to offline secondary CPUs.\n" ); +return -EBUSY ; + } +local_dbg_save (flags);if (__cpu_suspend_enter (&state)) {Signature · 1 line
2.8.0.rc3