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

Groups > linux.kernel > #1497483 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/4][RFC v2] PM / sleep: Introduce arch-specific hook for  disable/enable nonboot cpus Andy Lutomirski <luto@kernel.org> - 2016-10-07 21:40 +0200
    Re: [PATCH 2/4][RFC v2] PM / sleep: Introduce arch-specific hook for  disable/enable nonboot cpus Chen Yu <yu.c.chen@intel.com> - 2016-10-08 19:00 +0200
    Re: [PATCH 2/4][RFC v2] PM / sleep: Introduce arch-specific hook for  disable/enable nonboot cpus Pavel Machek <pavel@ucw.cz> - 2016-10-11 17:50 +0200

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

On 06/25/2016 09:18 AM, 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.
>
> 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)

I don't like using __weak.  It penalizes code size on architectures that 
don't hook these functions.  My preferred pattern is:

include/linux/something.h:

#include <asm/something.h>

#ifndef arch_do_xyz
static inline void arch_do_xyz() {}
#endif

arch/whatever/asm/something.h:

extern void arch_do_xyz();  /* or static inline... */
#define arch_do_xyz


This is totally free for architectures that don't have the hooks and it 
can potentially be inlined on architectures that do have the hooks. 
Everyone wins except that it's about five additional lines of code.

--Andy

[toc] | [next] | [standalone]

#1497779

Hi Andy,
On Fri, Oct 07, 2016 at 12:31:41PM -0700, Andy Lutomirski wrote:
> On 06/25/2016 09:18 AM, 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.
> >
> >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)
> 
> I don't like using __weak.  It penalizes code size on architectures that
> don't hook these functions.  My preferred pattern is:
> 
> include/linux/something.h:
> 
> #include <asm/something.h>
> 
> #ifndef arch_do_xyz
> static inline void arch_do_xyz() {}
> #endif
> 
> arch/whatever/asm/something.h:
> 
> extern void arch_do_xyz();  /* or static inline... */
> #define arch_do_xyz
> 
> 
> This is totally free for architectures that don't have the hooks and it can
> potentially be inlined on architectures that do have the hooks. Everyone
> wins except that it's about five additional lines of code.
> 
Thanks for your guidance, this patch was deprecated previously, but I'll
take your advice the next time when writing similar patch.  
> --Andy
Thanks,
Yu

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

#1498986

[Multipart message — attachments visible in raw view] — view raw

On Fri 2016-10-07 12:31:41, Andy Lutomirski wrote:
> On 06/25/2016 09:18 AM, 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.
> >
> >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)
> 
> I don't like using __weak.  It penalizes code size on architectures that
> don't hook these functions.  My preferred pattern is:
> 
> include/linux/something.h:
> 
> #include <asm/something.h>
> 
> #ifndef arch_do_xyz
> static inline void arch_do_xyz() {}
> #endif
> 
> arch/whatever/asm/something.h:
> 
> extern void arch_do_xyz();  /* or static inline... */
> #define arch_do_xyz
> 
> 
> This is totally free for architectures that don't have the hooks and it can
> potentially be inlined on architectures that do have the hooks. Everyone
> wins except that it's about five additional lines of code.

Well... 5 additional lines may be worse than few bytes in the object
file...

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web