disable_nonboot_cpus() is not safe to use when doing machine_down(),
because it relies on freeze_secondary_cpus() which in turn is
a suspend/resume related freeze and could abort if the logic detects any
pending activities that can prevent finishing the offlining process.
Beside disable_nonboot_cpus() is dependent on CONFIG_PM_SLEEP_SMP which
is an othogonal config to rely on to ensure this function works
correctly.
Use `reboot_cpu` variable instead of hardcoding 0 as the reboot cpu.
Signed-off-by: Qais Yousef <[email protected]>
CC: Russell King <[email protected]>
CC: [email protected]
CC: [email protected]
---
arch/arm/kernel/reboot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/reboot.c b/arch/arm/kernel/reboot.c
index bb18ed0539f4..0ce388f15422 100644
--- a/arch/arm/kernel/reboot.c
+++ b/arch/arm/kernel/reboot.c
@@ -88,11 +88,11 @@ void soft_restart(unsigned long addr)
* to execute e.g. a RAM-based pin loop is not sufficient. This allows the
* kexec'd kernel to use any and all RAM as it sees fit, without having to
* avoid any code or data used by any SW CPU pin loop. The CPU hotplug
- * functionality embodied in disable_nonboot_cpus() to achieve this.
+ * functionality embodied in smp_shutdown_nonboot_cpus() to achieve this.
*/
void machine_shutdown(void)
{
- disable_nonboot_cpus();
+ smp_shutdown_nonboot_cpus(reboot_cpu);
}
/*
--
2.17.1
On 02/23/20 19:29, Qais Yousef wrote:
> disable_nonboot_cpus() is not safe to use when doing machine_down(),
> because it relies on freeze_secondary_cpus() which in turn is
> a suspend/resume related freeze and could abort if the logic detects any
> pending activities that can prevent finishing the offlining process.
>
> Beside disable_nonboot_cpus() is dependent on CONFIG_PM_SLEEP_SMP which
> is an othogonal config to rely on to ensure this function works
> correctly.
>
> Use `reboot_cpu` variable instead of hardcoding 0 as the reboot cpu.
>
> Signed-off-by: Qais Yousef <[email protected]>
> CC: Russell King <[email protected]>
> CC: [email protected]
> CC: [email protected]
> ---
Hi Russel
Does the updated version look good to you now?
Thanks
--
Qais Yousef
> arch/arm/kernel/reboot.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kernel/reboot.c b/arch/arm/kernel/reboot.c
> index bb18ed0539f4..0ce388f15422 100644
> --- a/arch/arm/kernel/reboot.c
> +++ b/arch/arm/kernel/reboot.c
> @@ -88,11 +88,11 @@ void soft_restart(unsigned long addr)
> * to execute e.g. a RAM-based pin loop is not sufficient. This allows the
> * kexec'd kernel to use any and all RAM as it sees fit, without having to
> * avoid any code or data used by any SW CPU pin loop. The CPU hotplug
> - * functionality embodied in disable_nonboot_cpus() to achieve this.
> + * functionality embodied in smp_shutdown_nonboot_cpus() to achieve this.
> */
> void machine_shutdown(void)
> {
> - disable_nonboot_cpus();
> + smp_shutdown_nonboot_cpus(reboot_cpu);
> }
>
> /*
> --
> 2.17.1
>
On Fri, Mar 20, 2020 at 11:04:31AM +0000, Qais Yousef wrote:
> On 02/23/20 19:29, Qais Yousef wrote:
> > disable_nonboot_cpus() is not safe to use when doing machine_down(),
> > because it relies on freeze_secondary_cpus() which in turn is
> > a suspend/resume related freeze and could abort if the logic detects any
> > pending activities that can prevent finishing the offlining process.
> >
> > Beside disable_nonboot_cpus() is dependent on CONFIG_PM_SLEEP_SMP which
> > is an othogonal config to rely on to ensure this function works
> > correctly.
> >
> > Use `reboot_cpu` variable instead of hardcoding 0 as the reboot cpu.
I think that should be a separate change - you have two separate
changes in this patch:
1. to switch to using the new helper.
2. to change the CPU that we potentially use for the final steps of
shutdown
These should be two seperate changes, so if (2) causes a regression
it can be reverted independently of (1).
> >
> > Signed-off-by: Qais Yousef <[email protected]>
> > CC: Russell King <[email protected]>
> > CC: [email protected]
> > CC: [email protected]
> > ---
>
> Hi Russel
>
> Does the updated version look good to you now?
>
> Thanks
>
> --
> Qais Yousef
>
> > arch/arm/kernel/reboot.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/kernel/reboot.c b/arch/arm/kernel/reboot.c
> > index bb18ed0539f4..0ce388f15422 100644
> > --- a/arch/arm/kernel/reboot.c
> > +++ b/arch/arm/kernel/reboot.c
> > @@ -88,11 +88,11 @@ void soft_restart(unsigned long addr)
> > * to execute e.g. a RAM-based pin loop is not sufficient. This allows the
> > * kexec'd kernel to use any and all RAM as it sees fit, without having to
> > * avoid any code or data used by any SW CPU pin loop. The CPU hotplug
> > - * functionality embodied in disable_nonboot_cpus() to achieve this.
> > + * functionality embodied in smp_shutdown_nonboot_cpus() to achieve this.
> > */
> > void machine_shutdown(void)
> > {
> > - disable_nonboot_cpus();
> > + smp_shutdown_nonboot_cpus(reboot_cpu);
> > }
> >
> > /*
> > --
> > 2.17.1
> >
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
On 03/20/20 13:07, Russell King - ARM Linux admin wrote:
> On Fri, Mar 20, 2020 at 11:04:31AM +0000, Qais Yousef wrote:
> > On 02/23/20 19:29, Qais Yousef wrote:
> > > disable_nonboot_cpus() is not safe to use when doing machine_down(),
> > > because it relies on freeze_secondary_cpus() which in turn is
> > > a suspend/resume related freeze and could abort if the logic detects any
> > > pending activities that can prevent finishing the offlining process.
> > >
> > > Beside disable_nonboot_cpus() is dependent on CONFIG_PM_SLEEP_SMP which
> > > is an othogonal config to rely on to ensure this function works
> > > correctly.
> > >
> > > Use `reboot_cpu` variable instead of hardcoding 0 as the reboot cpu.
>
> I think that should be a separate change - you have two separate
> changes in this patch:
>
> 1. to switch to using the new helper.
> 2. to change the CPU that we potentially use for the final steps of
> shutdown
>
> These should be two seperate changes, so if (2) causes a regression
> it can be reverted independently of (1).
Okay will do.
Thanks
--
Qais Yousef
>
> > >
> > > Signed-off-by: Qais Yousef <[email protected]>
> > > CC: Russell King <[email protected]>
> > > CC: [email protected]
> > > CC: [email protected]
> > > ---
> >
> > Hi Russel
> >
> > Does the updated version look good to you now?
> >
> > Thanks
> >
> > --
> > Qais Yousef
> >
> > > arch/arm/kernel/reboot.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm/kernel/reboot.c b/arch/arm/kernel/reboot.c
> > > index bb18ed0539f4..0ce388f15422 100644
> > > --- a/arch/arm/kernel/reboot.c
> > > +++ b/arch/arm/kernel/reboot.c
> > > @@ -88,11 +88,11 @@ void soft_restart(unsigned long addr)
> > > * to execute e.g. a RAM-based pin loop is not sufficient. This allows the
> > > * kexec'd kernel to use any and all RAM as it sees fit, without having to
> > > * avoid any code or data used by any SW CPU pin loop. The CPU hotplug
> > > - * functionality embodied in disable_nonboot_cpus() to achieve this.
> > > + * functionality embodied in smp_shutdown_nonboot_cpus() to achieve this.
> > > */
> > > void machine_shutdown(void)
> > > {
> > > - disable_nonboot_cpus();
> > > + smp_shutdown_nonboot_cpus(reboot_cpu);
> > > }
> > >
> > > /*
> > > --
> > > 2.17.1
> > >
> >
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up