[PATCH v3 5/7] PM / Hibernate: Allow arch code to influence CPU hotplug during hibernate

[Rafael J. Wysocki]

On Monday, July 04, 2016 10:04:26 AM James Morse wrote:
> Hi Rafael,
> 
> On 29/06/16 01:10, Rafael J. Wysocki wrote:
> > On Tuesday, June 28, 2016 03:51:48 PM James Morse wrote:
> >> Architecture code may need to do extra work when secondary CPUs are
> >> disabled during hibernate and resume. This may include pushing sleeping
> >> CPUs into a deeper power-saving state, or influencing which CPU resume
> >> occurs on.
> >>
> >> Define a macro arch_hibernation_disable_cpus(), which defaults to calling
> >> disable_nonboot_cpus() if undefined. Architectures that need to do extra
> >> work around these calls can use this to influence disable_nonboot_cpus()
> >> behaviour. The macro should be defined in asm/suspend.h, and
> >> ARCH_HIBERNATION_CPUHP should be added to Kconfig.
> 
> > As you noted, this could be used to address the x86 issue that Yu is working on,
> > so I'd like it to go in as the first patch in the series and through the PM tree.
> 
> Sure, I will split the series here, and group the later patches so there are no
> dependencies.
> 
> 
> > 
> >> ---
> >> Changes since v2:
> >>  * Added CONFIG_ARCH_HIBERNATION_CPUHP include guard allowing
> > 
> > What about calling it CONFIG_ARCH_HIBERNATION_CPU_OFFLINE?  It's not
> > hotplug really.
> 
> Even with the enable calls added? CONFIG_ARCH_HIBERNATION_CPU_HOOKS?

Fine by me. :-)

> >> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> >> index fca9254280ee..338745e78f7e 100644
> >> --- a/kernel/power/hibernate.c
> >> +++ b/kernel/power/hibernate.c
> >> @@ -31,8 +31,16 @@
> >>  #include <linux/ktime.h>
> >>  #include <trace/events/power.h>
> >>  
> >> +#ifdef CONFIG_ARCH_HIBERNATION_CPUHP
> >> +/* Arch definition of the arch_hibernation_disable_cpus() macro? */
> >> +#include <asm/suspend.h>
> >> +#endif
> >> +
> >>  #include "power.h"
> >>  
> >> +#ifndef arch_hibernation_disable_cpus
> >> +#define arch_hibernation_disable_cpus(x) disable_nonboot_cpus()
> > 
> > For the x86 case we'll also need the complementary "enable", so why don't
> > you add it here and then use it instead of the enable_nonboot_cpus()?
> 
> Done. I've added the 'in_suspend' argument so that it's symmetrical, this may be
> slightly different to the behaviour of your 'in_resume_hibernate' flag.

Well, it turns out that I was wrong, we don't need the "enable" thing, so
I guess it can stay the way it was.

Thanks,
Rafael