| Message ID | 1464876657-6692-4-git-send-email-james.morse@arm.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thursday, June 02, 2016 03:10:55 PM James Morse wrote: > On arm64 the cpu with logical id 0 is assumed to be the boot CPU. If a > user hotplugs this CPU out, then uses kexec to boot a new kernel, the new > kernel will assign logical id 0 to a different physical CPU. > This breaks hibernate as hibernate and resume will be attempted on different > CPUs. > > Define a weak symbol arch_hibernation_disable_cpus(), which defaults to > calling disable_nonboot_cpus). Architectures that allow CPU 0 to be > hotplugged can use this to control which CPU is used for hibernate/resume. > > Signed-off-by: James Morse <james.morse@arm.com> > Cc: Rafael J. Wysocki <rjw@rjwysocki.net> > Cc: Pavel Machek <pavel@ucw.cz> > --- > I would have preferred a macro, but there is no hibernate-relevant header > file that all arch's have. arm, arm64 and x86 have a suspend.h, but powerpc > doesn't. What about include/linux/suspend.h? There are arch_ things declared in there. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, June 14, 2016 12:05:28 AM Rafael J. Wysocki wrote: > On Thursday, June 02, 2016 03:10:55 PM James Morse wrote: > > On arm64 the cpu with logical id 0 is assumed to be the boot CPU. If a > > user hotplugs this CPU out, then uses kexec to boot a new kernel, the new > > kernel will assign logical id 0 to a different physical CPU. > > This breaks hibernate as hibernate and resume will be attempted on different > > CPUs. > > > > Define a weak symbol arch_hibernation_disable_cpus(), which defaults to > > calling disable_nonboot_cpus). Architectures that allow CPU 0 to be > > hotplugged can use this to control which CPU is used for hibernate/resume. > > > > Signed-off-by: James Morse <james.morse@arm.com> > > Cc: Rafael J. Wysocki <rjw@rjwysocki.net> > > Cc: Pavel Machek <pavel@ucw.cz> > > --- > > I would have preferred a macro, but there is no hibernate-relevant header > > file that all arch's have. arm, arm64 and x86 have a suspend.h, but powerpc > > doesn't. > > What about include/linux/suspend.h? > > There are arch_ things declared in there. I mean, what about using something like #ifndef arch_hibernation_disable_cpus #define arch_hibernation_disable_cpus disable_nonboot_cpus #endif in there or in another header file? Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rafael, On 13/06/16 23:10, Rafael J. Wysocki wrote: > On Tuesday, June 14, 2016 12:05:28 AM Rafael J. Wysocki wrote: >> On Thursday, June 02, 2016 03:10:55 PM James Morse wrote: >>> On arm64 the cpu with logical id 0 is assumed to be the boot CPU. If a >>> user hotplugs this CPU out, then uses kexec to boot a new kernel, the new >>> kernel will assign logical id 0 to a different physical CPU. >>> This breaks hibernate as hibernate and resume will be attempted on different >>> CPUs. >>> >>> Define a weak symbol arch_hibernation_disable_cpus(), which defaults to >>> calling disable_nonboot_cpus). Architectures that allow CPU 0 to be >>> hotplugged can use this to control which CPU is used for hibernate/resume. >>> >>> Signed-off-by: James Morse <james.morse@arm.com> >>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net> >>> Cc: Pavel Machek <pavel@ucw.cz> >>> --- >>> I would have preferred a macro, but there is no hibernate-relevant header >>> file that all arch's have. arm, arm64 and x86 have a suspend.h, but powerpc >>> doesn't. >> >> What about include/linux/suspend.h? >> >> There are arch_ things declared in there. > > I mean, what about using something like > > #ifndef arch_hibernation_disable_cpus > #define arch_hibernation_disable_cpus disable_nonboot_cpus > #endif > > in there or in another header file? This would work fine for an architecture that doesn't need to use arch_hibernation_disable_cpus(), but which header file should we use for an architecture that does? As a macro it needs to be in an arch-specific header included before the #ifndef above. Of those: include/linux/suspend.h only includes <asm/errno.h> kernel/power/power.h doesn't include any 'asm' headers. snapshot.c includes: > #include <asm/uaccess.h> > #include <asm/mmu_context.h> > #include <asm/pgtable.h> > #include <asm/tlbflush.h> > #include <asm/io.h> ... but none of those are appropriate. I initially added an include for <asm/suspend.h> to kernel/power/power.h, but it broke powerpc, as there is no suspend.h there. I took this as a dead-end for the macro approach, unless someone has a clever trick?! Thanks, James -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, June 14, 2016 10:44:47 AM James Morse wrote: > Hi Rafael, > > On 13/06/16 23:10, Rafael J. Wysocki wrote: > > On Tuesday, June 14, 2016 12:05:28 AM Rafael J. Wysocki wrote: > >> On Thursday, June 02, 2016 03:10:55 PM James Morse wrote: > >>> On arm64 the cpu with logical id 0 is assumed to be the boot CPU. If a > >>> user hotplugs this CPU out, then uses kexec to boot a new kernel, the new > >>> kernel will assign logical id 0 to a different physical CPU. > >>> This breaks hibernate as hibernate and resume will be attempted on different > >>> CPUs. > >>> > >>> Define a weak symbol arch_hibernation_disable_cpus(), which defaults to > >>> calling disable_nonboot_cpus). Architectures that allow CPU 0 to be > >>> hotplugged can use this to control which CPU is used for hibernate/resume. > >>> > >>> Signed-off-by: James Morse <james.morse@arm.com> > >>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net> > >>> Cc: Pavel Machek <pavel@ucw.cz> > >>> --- > >>> I would have preferred a macro, but there is no hibernate-relevant header > >>> file that all arch's have. arm, arm64 and x86 have a suspend.h, but powerpc > >>> doesn't. > >> > >> What about include/linux/suspend.h? > >> > >> There are arch_ things declared in there. > > > > I mean, what about using something like > > > > #ifndef arch_hibernation_disable_cpus > > #define arch_hibernation_disable_cpus disable_nonboot_cpus > > #endif > > > > in there or in another header file? > > This would work fine for an architecture that doesn't need to use > arch_hibernation_disable_cpus(), but which header file should we use for an > architecture that does? > > As a macro it needs to be in an arch-specific header included before the #ifndef > above. > > Of those: > include/linux/suspend.h only includes <asm/errno.h> > kernel/power/power.h doesn't include any 'asm' headers. > snapshot.c includes: > > #include <asm/uaccess.h> > > #include <asm/mmu_context.h> > > #include <asm/pgtable.h> > > #include <asm/tlbflush.h> > > #include <asm/io.h> > > ... but none of those are appropriate. > > > I initially added an include for <asm/suspend.h> to kernel/power/power.h, but it > broke powerpc, as there is no suspend.h there. I took this as a dead-end for the > macro approach, unless someone has a clever trick?! OK, let's do the __weak function, then. After all, it can be changed later if someone has a better idea. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index fca9254280ee..0e668a3207ec 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -70,6 +70,16 @@ bool hibernation_available(void) } /** + * arch_hibernation_disable_cpus - Allow architectures to direct which CPU + * is used for suspend or resume. + * @suspend: True during hibernate, false for resume. + */ +int __weak arch_hibernation_disable_cpus(__maybe_unused bool suspend) +{ + return disable_nonboot_cpus(); +} + +/** * hibernation_set_ops - Set the global hibernate operations. * @ops: Hibernation operations to use in subsequent hibernation transitions. */ @@ -279,7 +289,7 @@ static int create_image(int platform_mode) if (error || hibernation_test(TEST_PLATFORM)) goto Platform_finish; - error = disable_nonboot_cpus(); + error = arch_hibernation_disable_cpus(true); if (error || hibernation_test(TEST_CPUS)) goto Enable_cpus; @@ -433,7 +443,7 @@ static int resume_target_kernel(bool platform_mode) if (error) goto Cleanup; - error = disable_nonboot_cpus(); + error = arch_hibernation_disable_cpus(false); if (error) goto Enable_cpus; @@ -551,7 +561,7 @@ int hibernation_platform_enter(void) if (error) goto Platform_finish; - error = disable_nonboot_cpus(); + error = arch_hibernation_disable_cpus(true); if (error) goto Enable_cpus;
On arm64 the cpu with logical id 0 is assumed to be the boot CPU. If a user hotplugs this CPU out, then uses kexec to boot a new kernel, the new kernel will assign logical id 0 to a different physical CPU. This breaks hibernate as hibernate and resume will be attempted on different CPUs. Define a weak symbol arch_hibernation_disable_cpus(), which defaults to calling disable_nonboot_cpus). Architectures that allow CPU 0 to be hotplugged can use this to control which CPU is used for hibernate/resume. Signed-off-by: James Morse <james.morse@arm.com> Cc: Rafael J. Wysocki <rjw@rjwysocki.net> Cc: Pavel Machek <pavel@ucw.cz> --- I would have preferred a macro, but there is no hibernate-relevant header file that all arch's have. arm, arm64 and x86 have a suspend.h, but powerpc doesn't. If this approach is acceptable, this patch should go with 4&5 via the arm64 tree. kernel/power/hibernate.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)