Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1431159 > unrolled thread

[PATCH 0/4][RFC v2] x86, hotplug: Use hlt instead of mwait when resuming from hibernation

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4][RFC v2] x86, hotplug: Use hlt instead of mwait when resuming from hibernation Chen Yu <yu.c.chen@intel.com> - 2016-06-25 18:20 +0200
    [PATCH 3/4][RFC v2] PM / hibernate: introduce a flag to indicate resuming from hibernation Chen Yu <yu.c.chen@intel.com> - 2016-06-25 18:20 +0200
      Re: [PATCH 3/4][RFC v2] PM / hibernate: introduce a flag to indicate  resuming from hibernation Pavel Machek <pavel@ucw.cz> - 2016-06-25 19:00 +0200
        RE: [PATCH 3/4][RFC v2] PM / hibernate: introduce a flag to  indicate resuming from hibernation "Chen, Yu C" <yu.c.chen@intel.com> - 2016-06-26 03:40 +0200
    [PATCH 1/4][RFC v2] PM / sleep: Avoid accessing frozen_cpus if it is NULL Chen Yu <yu.c.chen@intel.com> - 2016-06-25 18:20 +0200
      Re: [PATCH 1/4][RFC v2] PM / sleep: Avoid accessing frozen_cpus if  it is NULL Pavel Machek <pavel@ucw.cz> - 2016-06-25 19:00 +0200
        RE: [PATCH 1/4][RFC v2] PM / sleep: Avoid accessing frozen_cpus if  it is NULL "Chen, Yu C" <yu.c.chen@intel.com> - 2016-06-26 03:20 +0200
        RE: [PATCH 1/4][RFC v2] PM / sleep: Avoid accessing frozen_cpus if  it is NULL "Chen, Yu C" <yu.c.chen@intel.com> - 2016-06-26 06:30 +0200
    [PATCH 2/4][RFC v2] PM / sleep: Introduce arch-specific hook for disable/enable nonboot cpus Chen Yu <yu.c.chen@intel.com> - 2016-06-25 18:20 +0200
      Re: [PATCH 2/4][RFC v2] PM / sleep: Introduce arch-specific hook for  disable/enable nonboot cpus Pavel Machek <pavel@ucw.cz> - 2016-06-25 19:00 +0200
        RE: [PATCH 2/4][RFC v2] PM / sleep: Introduce arch-specific hook  for disable/enable nonboot cpus "Chen, Yu C" <yu.c.chen@intel.com> - 2016-06-26 03:20 +0200

#1431159 — [PATCH 0/4][RFC v2] x86, hotplug: Use hlt instead of mwait when resuming from hibernation

Currently it is reported that, when system is trying to resume
from hibernation, the nonboot CPUs might be incorrectly woken up
and hang there. The reason for this is because of inconsistent
page tables across hibernation resume. To avoid this situation,
use hlt instead of mwait to put nonboot CPUs in a more safe state
and just let them watch the boot CPU to do all the things and
wakes them up later.

The first three patches are preparation for the fourth one, please
refer to [4/4] for detail.

Chen Yu (4):
  PM / sleep: Avoid accessing frozen_cpus if it is NULL
  PM / sleep: Introduce arch-specific hook for disable/enable nonboot
    cpus
  PM / hibernate: introduce a flag to indicate resuming from hibernation
  x86, hotplug: Use hlt instead of mwait when resuming from hibernation

 arch/x86/kernel/smpboot.c | 15 +++++++++++++++
 include/linux/cpu.h       |  2 ++
 include/linux/suspend.h   |  7 +++++++
 kernel/cpu.c              | 38 ++++++++++++++++++++++++++++++++++++++
 kernel/power/hibernate.c  |  3 +++
 5 files changed, 65 insertions(+)

-- 
2.7.4

[toc] | [next] | [standalone]

#1431160 — [PATCH 3/4][RFC v2] PM / hibernate: introduce a flag to indicate resuming from hibernation

Sometime we need to do some operations before resuming from
hibernation, so introduce a flag to indicate this stage.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 include/linux/suspend.h  | 7 +++++++
 kernel/power/hibernate.c | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 8b6ec7e..422e87a 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -384,6 +384,12 @@ extern bool system_entering_hibernation(void);
 extern bool hibernation_available(void);
 asmlinkage int swsusp_save(void);
 extern struct pbe *restore_pblist;
+extern bool in_resume_hibernate;
+
+static inline bool hibernation_in_resume(void)
+{
+	return in_resume_hibernate;
+}
 #else /* CONFIG_HIBERNATION */
 static inline void register_nosave_region(unsigned long b, unsigned long e) {}
 static inline void register_nosave_region_late(unsigned long b, unsigned long e) {}
@@ -395,6 +401,7 @@ static inline void hibernation_set_ops(const struct platform_hibernation_ops *op
 static inline int hibernate(void) { return -ENOSYS; }
 static inline bool system_entering_hibernation(void) { return false; }
 static inline bool hibernation_available(void) { return false; }
+static inline bool hibernation_in_resume(void) { return false; }
 #endif /* CONFIG_HIBERNATION */
 
 /* Hibernation and suspend events */
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index fca9254..41be909 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -43,6 +43,7 @@ static char resume_file[256] = CONFIG_PM_STD_PARTITION;
 dev_t swsusp_resume_device;
 sector_t swsusp_resume_block;
 __visible int in_suspend __nosavedata;
+bool in_resume_hibernate;
 
 enum {
 	HIBERNATION_INVALID,
@@ -433,6 +434,7 @@ static int resume_target_kernel(bool platform_mode)
 	if (error)
 		goto Cleanup;
 
+	in_resume_hibernate = true;
 	error = disable_nonboot_cpus();
 	if (error)
 		goto Enable_cpus;
@@ -474,6 +476,7 @@ static int resume_target_kernel(bool platform_mode)
 	local_irq_enable();
 
  Enable_cpus:
+	in_resume_hibernate = false;
 	enable_nonboot_cpus();
 
  Cleanup:
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1431170 — Re: [PATCH 3/4][RFC v2] PM / hibernate: introduce a flag to indicate resuming from hibernation

On Sun 2016-06-26 00:18:52, Chen Yu wrote:
> Sometime we need to do some operations before resuming from
> hibernation, so introduce a flag to indicate this stage.
> 
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  include/linux/suspend.h  | 7 +++++++
>  kernel/power/hibernate.c | 3 +++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 8b6ec7e..422e87a 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -384,6 +384,12 @@ extern bool system_entering_hibernation(void);
>  extern bool hibernation_available(void);
>  asmlinkage int swsusp_save(void);
>  extern struct pbe *restore_pblist;
> +extern bool in_resume_hibernate;

in_resume_hibernation? But it is pretty sad if we need another such
state function...
								Pavel

> +static inline bool hibernation_in_resume(void)
> +{
> +	return in_resume_hibernate;
> +}
>  #else /* CONFIG_HIBERNATION */
>  static inline void register_nosave_region(unsigned long b, unsigned long e) {}
>  static inline void register_nosave_region_late(unsigned long b, unsigned long e) {}
> @@ -395,6 +401,7 @@ static inline void hibernation_set_ops(const struct platform_hibernation_ops *op
>  static inline int hibernate(void) { return -ENOSYS; }
>  static inline bool system_entering_hibernation(void) { return false; }
>  static inline bool hibernation_available(void) { return false; }
> +static inline bool hibernation_in_resume(void) { return false; }
>  #endif /* CONFIG_HIBERNATION */
>  
>  /* Hibernation and suspend events */
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index fca9254..41be909 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -43,6 +43,7 @@ static char resume_file[256] = CONFIG_PM_STD_PARTITION;
>  dev_t swsusp_resume_device;
>  sector_t swsusp_resume_block;
>  __visible int in_suspend __nosavedata;
> +bool in_resume_hibernate;
>  
>  enum {
>  	HIBERNATION_INVALID,
> @@ -433,6 +434,7 @@ static int resume_target_kernel(bool platform_mode)
>  	if (error)
>  		goto Cleanup;
>  
> +	in_resume_hibernate = true;
>  	error = disable_nonboot_cpus();
>  	if (error)
>  		goto Enable_cpus;
> @@ -474,6 +476,7 @@ static int resume_target_kernel(bool platform_mode)
>  	local_irq_enable();
>  
>   Enable_cpus:
> +	in_resume_hibernate = false;
>  	enable_nonboot_cpus();
>  
>   Cleanup:

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1431283 — RE: [PATCH 3/4][RFC v2] PM / hibernate: introduce a flag to indicate resuming from hibernation

Hi,

> -----Original Message-----
> From: Pavel Machek [mailto:pavel@ucw.cz]
> Sent: Sunday, June 26, 2016 12:53 AM
> To: Chen, Yu C
> Cc: linux-pm@vger.kernel.org; x86@kernel.org; Rafael J. Wysocki; Len Brown;
> Peter Zijlstra; H. Peter Anvin; Borislav Petkov; Brian Gerst; Thomas Gleixner;
> Ingo Molnar; Varun Koyyalagunta; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 3/4][RFC v2] PM / hibernate: introduce a flag to indicate
> resuming from hibernation
> 
> On Sun 2016-06-26 00:18:52, Chen Yu wrote:
> > Sometime we need to do some operations before resuming from
> > hibernation, so introduce a flag to indicate this stage.
> >
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > ---
> >  include/linux/suspend.h  | 7 +++++++
> >  kernel/power/hibernate.c | 3 +++
> >  2 files changed, 10 insertions(+)
> >
> > diff --git a/include/linux/suspend.h b/include/linux/suspend.h index
> > 8b6ec7e..422e87a 100644
> > --- a/include/linux/suspend.h
> > +++ b/include/linux/suspend.h
> > @@ -384,6 +384,12 @@ extern bool system_entering_hibernation(void);
> >  extern bool hibernation_available(void);  asmlinkage int
> > swsusp_save(void);  extern struct pbe *restore_pblist;
> > +extern bool in_resume_hibernate;
> 
> in_resume_hibernation? But it is pretty sad if we need another such
> state function...
At first I tried not to add any new flags, and just let all the CPUs fall into hlt if it is
triggered by disable_nonboot_cpus, but as depicted in [PATCH 4/4], 
there is a scenario that,   what about the CPUs already  offline before
the hibernation? They might be in deep cstate by mwait, and mwait is not
safe during resume.
We have to kick all the offline CPUs online again, and then
put them offline to hlt state, thus to avoid the unsafe memory access during resume.
(The memory mapping for text code is not safe during resume, so it's hard to fix it
in mwait_play_dead, and the kick mechanism seems to be a suitable choice).
In order to restrict the impact of the kick mechanism to a minimal scope, I used this
flag to make this  mechanism only work for hibernation resume phase.

thanks,
Yu

[toc] | [prev] | [next] | [standalone]

#1431161 — [PATCH 1/4][RFC v2] PM / sleep: Avoid accessing frozen_cpus if it is NULL

frozen_cpus might be NULL if the allocation in previous
alloc_frozen_cpus failed, when CONFIG_CPUMASK_OFFSTACK
is set.

This patch avoid accessing this cpumask if it is NULL.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 kernel/cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index d948e44..d25266e 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1021,6 +1021,8 @@ int disable_nonboot_cpus(void)
 {
 	int cpu, first_cpu, error = 0;
 
+	if (frozen_cpus == NULL)
+		return -ENOMEM;
 	cpu_maps_update_begin();
 	first_cpu = cpumask_first(cpu_online_mask);
 	/*
@@ -1072,6 +1074,8 @@ void enable_nonboot_cpus(void)
 {
 	int cpu, error;
 
+	if (frozen_cpus == NULL)
+		return;
 	/* Allow everyone to use the CPU hotplug again */
 	cpu_maps_update_begin();
 	WARN_ON(--cpu_hotplug_disabled < 0);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1431169 — Re: [PATCH 1/4][RFC v2] PM / sleep: Avoid accessing frozen_cpus if it is NULL

On Sun 2016-06-26 00:18:30, Chen Yu wrote:
> frozen_cpus might be NULL if the allocation in previous
> alloc_frozen_cpus failed, when CONFIG_CPUMASK_OFFSTACK
> is set.
> 
> This patch avoid accessing this cpumask if it is NULL.
> 
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  kernel/cpu.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index d948e44..d25266e 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1021,6 +1021,8 @@ int disable_nonboot_cpus(void)
>  {
>  	int cpu, first_cpu, error = 0;
>  
> +	if (frozen_cpus == NULL)
> +		return -ENOMEM;
>  	cpu_maps_update_begin();
>  	first_cpu = cpumask_first(cpu_online_mask);
>  	/*

I'd say that whoever allocates frozen_cpus should just abort the
hibernation if there's not enough memory for the operation...? This
seems like checking for the problem too late.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1431275 — RE: [PATCH 1/4][RFC v2] PM / sleep: Avoid accessing frozen_cpus if it is NULL

Hi,

> -----Original Message-----
> From: Pavel Machek [mailto:pavel@ucw.cz]
> Sent: Sunday, June 26, 2016 12:51 AM
> To: Chen, Yu C
> Cc: linux-pm@vger.kernel.org; x86@kernel.org; Rafael J. Wysocki; Len Brown;
> Peter Zijlstra; H. Peter Anvin; Borislav Petkov; Brian Gerst; Thomas Gleixner;
> Ingo Molnar; Varun Koyyalagunta; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/4][RFC v2] PM / sleep: Avoid accessing frozen_cpus if it
> is NULL
> 
> On Sun 2016-06-26 00:18:30, Chen Yu wrote:
> > frozen_cpus might be NULL if the allocation in previous
> > alloc_frozen_cpus failed, when CONFIG_CPUMASK_OFFSTACK is set.
> >
> > This patch avoid accessing this cpumask if it is NULL.
> >
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > ---
> >  kernel/cpu.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/kernel/cpu.c b/kernel/cpu.c index d948e44..d25266e 100644
> > --- a/kernel/cpu.c
> > +++ b/kernel/cpu.c
> > @@ -1021,6 +1021,8 @@ int disable_nonboot_cpus(void)  {
> >  	int cpu, first_cpu, error = 0;
> >
> > +	if (frozen_cpus == NULL)
> > +		return -ENOMEM;
> >  	cpu_maps_update_begin();
> >  	first_cpu = cpumask_first(cpu_online_mask);
> >  	/*
> 
> I'd say that whoever allocates frozen_cpus should just abort the hibernation if
> there's not enough memory for the operation...? This seems like checking for
> the problem too late.
> 
The allocation of frozen_cpus is alloc_frozen_cpus, which is in core_initcall,
and do_initcall during boot up  seems not to care about the return value from 
these functions.  So I think either we add the check in disable_nonboot_cpus, or
we can set noresume = 1 and nohibernate = 1 if alloc_frozen_cpus fails.

thanks,
Yu

[toc] | [prev] | [next] | [standalone]

#1431330 — RE: [PATCH 1/4][RFC v2] PM / sleep: Avoid accessing frozen_cpus if it is NULL

> -----Original Message-----
> From: Chen, Yu C
> Sent: Sunday, June 26, 2016 9:17 AM
> To: 'Pavel Machek'
> Cc: linux-pm@vger.kernel.org; x86@kernel.org; Rafael J. Wysocki; Len Brown;
> Peter Zijlstra; H. Peter Anvin; Borislav Petkov; Brian Gerst; Thomas Gleixner;
> Ingo Molnar; Varun Koyyalagunta; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH 1/4][RFC v2] PM / sleep: Avoid accessing frozen_cpus if it
> is NULL
> 
> Hi,
> 
> > -----Original Message-----
> > From: Pavel Machek [mailto:pavel@ucw.cz]
> > Sent: Sunday, June 26, 2016 12:51 AM
> > To: Chen, Yu C
> > Cc: linux-pm@vger.kernel.org; x86@kernel.org; Rafael J. Wysocki; Len
> > Brown; Peter Zijlstra; H. Peter Anvin; Borislav Petkov; Brian Gerst;
> > Thomas Gleixner; Ingo Molnar; Varun Koyyalagunta;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH 1/4][RFC v2] PM / sleep: Avoid accessing
> > frozen_cpus if it is NULL
> >
[cut]
> >
> > I'd say that whoever allocates frozen_cpus should just abort the
> > hibernation if there's not enough memory for the operation...? This
> > seems like checking for the problem too late.
> >
> The allocation of frozen_cpus is alloc_frozen_cpus, which is in core_initcall, and
> do_initcall during boot up  seems not to care about the return value from these
> functions.  So I think either we add the check in disable_nonboot_cpus, or we
> can set noresume = 1 and nohibernate = 1 if alloc_frozen_cpus fails.
> 
Besides, disable_nonboot_cpus is not only used for hibernation, but also for
suspend to ram, kexec, etc, so it might be better to check before we use
it, thus to avoid introducing new flags IMO.

thanks,
Yu

[toc] | [prev] | [next] | [standalone]

#1431163 — [PATCH 2/4][RFC v2] PM / sleep: Introduce arch-specific hook for disable/enable nonboot cpus

There is requirement that we need to do some arch-specific
operations before putting the nonboot CPUs offline/online.
One of the requirements comes from the hibernation resume
process on x86_64, we need to kick all the offlin-CPUs
online and offline again, in order to put them in a safe
state, thus to avoid possible unwilling wake up during
hibernation resume.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 kernel/cpu.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index d25266e..ce6e5e4 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1017,6 +1017,16 @@ EXPORT_SYMBOL_GPL(cpu_up);
 #ifdef CONFIG_PM_SLEEP_SMP
 static cpumask_var_t frozen_cpus;
 
+void __weak arch_disable_nonboot_cpus_pre(void)
+{
+
+}
+
+void __weak arch_enable_nonboot_cpus_pre(void)
+{
+
+}
+
 int disable_nonboot_cpus(void)
 {
 	int cpu, first_cpu, error = 0;
@@ -1024,6 +1034,7 @@ int disable_nonboot_cpus(void)
 	if (frozen_cpus == NULL)
 		return -ENOMEM;
 	cpu_maps_update_begin();
+	arch_disable_nonboot_cpus_pre();
 	first_cpu = cpumask_first(cpu_online_mask);
 	/*
 	 * We take down all of the non-boot CPUs in one shot to avoid races
@@ -1078,6 +1089,7 @@ void enable_nonboot_cpus(void)
 		return;
 	/* Allow everyone to use the CPU hotplug again */
 	cpu_maps_update_begin();
+	arch_enable_nonboot_cpus_pre();
 	WARN_ON(--cpu_hotplug_disabled < 0);
 	if (cpumask_empty(frozen_cpus))
 		goto out;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1431171 — Re: [PATCH 2/4][RFC v2] PM / sleep: Introduce arch-specific hook for disable/enable nonboot cpus

On Sun 2016-06-26 00:18:41, Chen Yu wrote:
> There is requirement that we need to do some arch-specific
> operations before putting the nonboot CPUs offline/online.
> One of the requirements comes from the hibernation resume
> process on x86_64, we need to kick all the offlin-CPUs
> online and offline again, in order to put them in a safe
> state, thus to avoid possible unwilling wake up during
> hibernation resume.

You may want to check english here.


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1431270 — RE: [PATCH 2/4][RFC v2] PM / sleep: Introduce arch-specific hook for disable/enable nonboot cpus

Hi,


> -----Original Message-----
> From: Pavel Machek [mailto:pavel@ucw.cz]
> Sent: Sunday, June 26, 2016 12:52 AM
> To: Chen, Yu C
> Cc: linux-pm@vger.kernel.org; x86@kernel.org; Rafael J. Wysocki; Len Brown;
> Peter Zijlstra; H. Peter Anvin; Borislav Petkov; Brian Gerst; Thomas Gleixner;
> Ingo Molnar; Varun Koyyalagunta; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/4][RFC v2] PM / sleep: Introduce arch-specific hook for
> disable/enable nonboot cpus
> 
> On Sun 2016-06-26 00:18:41, Chen Yu wrote:
> > There is requirement that we need to do some arch-specific operations
> > before putting the nonboot CPUs offline/online.
> > One of the requirements comes from the hibernation resume process on
> > x86_64, we need to kick all the offlin-CPUs online and offline again,
> > in order to put them in a safe state, thus to avoid possible unwilling
> > wake up during hibernation resume.
> 
> You may want to check english here.
Ah, sorry, I'll rewrite the log, and [PATCH 4/4] might contain the detail of this issue.

thanks,
Yu

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web