Re: [PATCH] PM / Hibernate: remove duplicate disable_nonboot_cpus()

From: Rafael J. Wysocki
Date: Wed Jan 30 2013 - 18:37:23 EST


On Tuesday, January 29, 2013 04:20:15 PM Stephen Warren wrote:
> From: Stephen Warren <swarren@xxxxxxxxxx>
>
> Within hibernate.c, hibernation_platform_enter() is only called from
> power_down(), which in turn is only called by hibernate() after having
> called hibernation_snapshot(), which in turn calls create_image() which
> in turn calls disable_nonboot_cpus().
>
> Elsewhere, snapshot_ioctl() allows hibernation_snapshot() and
> hibernation_platform_enter() to be invoked via separate IOCTLs, but I
> assume the rule is that SNAPSHOT_CREATE_IMAGE must be executed before
> IOCTL SNAPSHOT_POWER_OFF, and hence the same ordering applies.
>
> This means that disable_nonboot_cpus() must have been called already
> before hibernation_platform_enter() is entered, and hence the call there
> is redundant, so remove it.

No, it isn't. This is a different stage of hibernation and
enable_nonboot_cpus() is called before.

It works like this: First, before creating the image, the complete "freeze"
sequence is carried out which includes disable_nonboot_cpus(). Then, the
image is created and the complete "thaw" sequence is carried out which includes
enable_nonboot_cpus(). Next, the image is saved and finally the memory taken
up by it is freed and hibernation_platform_enter() is called. Roughly.

Thanks,
Rafael


> Signed-off-by: Stephen Warren <swarren@xxxxxxxxxx>
> ---
> Note: I have compile-tested this on x86-64, but have not attempted to test
> it in any way; I'm proposing this patch following code inspection.
> ---
> kernel/power/hibernate.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index b26f5f1..4ddeacc 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -537,10 +537,6 @@ int hibernation_platform_enter(void)
> if (error)
> goto Platform_finish;
>
> - error = disable_nonboot_cpus();
> - if (error)
> - goto Platform_finish;
> -
> local_irq_disable();
> syscore_suspend();
> if (pm_wakeup_pending()) {
> @@ -555,7 +551,6 @@ int hibernation_platform_enter(void)
> Power_up:
> syscore_resume();
> local_irq_enable();
> - enable_nonboot_cpus();
>
> Platform_finish:
> hibernation_ops->finish();
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/