[PATCH]: wrap cpu_die() with CONFIG_HOTPLUG_CPU

[PATCH]: wrap cpu_die() with CONFIG_HOTPLUG_CPU

[Linas Vepstas]

Paul,

Please apply the janitorial patch below. Its not urgent.
It replaces/obsoletes the previous Subject: [PATCH]: 
PowerPC: make sure the rtas stop-self token is defined.
patch.

--linas

Per email discussion, it appears that rtas_stop_self()
and pSeries_mach_cpu_die() should not be compiled if 
CONFIG_HOTPLUG_CPU is not defined. This patch adds
#ifdefs around these bits of code.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 arch/powerpc/kernel/rtas.c             |    5 +++--
 arch/powerpc/platforms/pseries/setup.c |    4 ++++
 include/asm-powerpc/rtas.h             |    4 ++--
 3 files changed, 9 insertions(+), 4 deletions(-)

Index: linux-2.6.19-rc4-git3/arch/powerpc/kernel/rtas.c
===================================================================
--- linux-2.6.19-rc4-git3.orig/arch/powerpc/kernel/rtas.c	2006-11-15 11:40:14.000000000 -0600
+++ linux-2.6.19-rc4-git3/arch/powerpc/kernel/rtas.c	2006-11-15 12:46:51.000000000 -0600
@@ -810,9 +810,9 @@ asmlinkage int ppc_rtas(struct rtas_args
 	return 0;
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
 /* This version can't take the spinlock, because it never returns */
-
-struct rtas_args rtas_stop_self_args = {
+static struct rtas_args rtas_stop_self_args = {
 	/* The token is initialized for real in setup_system() */
 	.token = RTAS_UNKNOWN_SERVICE,
 	.nargs = 0,
@@ -834,6 +834,7 @@ void rtas_stop_self(void)
 
 	panic("Alas, I survived.\n");
 }
+#endif
 
 /*
  * Call early during boot, before mem init or bootmem, to retrieve the RTAS
Index: linux-2.6.19-rc4-git3/include/asm-powerpc/rtas.h
===================================================================
--- linux-2.6.19-rc4-git3.orig/include/asm-powerpc/rtas.h	2006-11-01 16:15:46.000000000 -0600
+++ linux-2.6.19-rc4-git3/include/asm-powerpc/rtas.h	2006-11-15 11:44:24.000000000 -0600
@@ -54,8 +54,6 @@ struct rtas_args {
 	rtas_arg_t *rets;     /* Pointer to return values in args[]. */
 };  
 
-extern struct rtas_args rtas_stop_self_args;
-
 struct rtas_t {
 	unsigned long entry;		/* physical address pointer */
 	unsigned long base;		/* physical address pointer */
@@ -223,7 +221,9 @@ extern int rtas_get_error_log_max(void);
 extern spinlock_t rtas_data_buf_lock;
 extern char rtas_data_buf[RTAS_DATA_BUF_SIZE];
 
+#ifdef CONFIG_HOTPLUG_CPU
 extern void rtas_stop_self(void);
+#endif
 
 /* RMO buffer reserved for user-space RTAS use */
 extern unsigned long rtas_rmo_buf;
Index: linux-2.6.19-rc4-git3/arch/powerpc/platforms/pseries/setup.c
===================================================================
--- linux-2.6.19-rc4-git3.orig/arch/powerpc/platforms/pseries/setup.c	2006-11-01 16:15:09.000000000 -0600
+++ linux-2.6.19-rc4-git3/arch/powerpc/platforms/pseries/setup.c	2006-11-15 11:46:10.000000000 -0600
@@ -347,6 +347,7 @@ static int __init pSeries_init_panel(voi
 }
 arch_initcall(pSeries_init_panel);
 
+#ifdef CONFIG_HOTPLUG_CPU
 static void pSeries_mach_cpu_die(void)
 {
 	local_irq_disable();
@@ -357,6 +358,7 @@ static void pSeries_mach_cpu_die(void)
 	BUG();
 	for(;;);
 }
+#endif
 
 static int pseries_set_dabr(unsigned long dabr)
 {
@@ -558,7 +560,9 @@ define_machine(pseries) {
 	.power_off		= rtas_power_off,
 	.halt			= rtas_halt,
 	.panic			= rtas_os_term,
+#ifdef CONFIG_HOTPLUG_CPU
 	.cpu_die		= pSeries_mach_cpu_die,
+#endif
 	.get_boot_time		= rtas_get_boot_time,
 	.get_rtc_time		= rtas_get_rtc_time,
 	.set_rtc_time		= rtas_set_rtc_time,

Re: [PATCH]: wrap cpu_die() with CONFIG_HOTPLUG_CPU

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 3220 bytes --]

On Wed, 2006-11-15 at 12:57 -0600, Linas Vepstas wrote:
> 
> Paul,
> 
> Please apply the janitorial patch below. Its not urgent.
> It replaces/obsoletes the previous Subject: [PATCH]: 
> PowerPC: make sure the rtas stop-self token is defined.
> patch.
> 
> --linas
> 
> Per email discussion, it appears that rtas_stop_self()
> and pSeries_mach_cpu_die() should not be compiled if 
> CONFIG_HOTPLUG_CPU is not defined. This patch adds
> #ifdefs around these bits of code.
> 
> Signed-off-by: Linas Vepstas <linas@austin.ibm.com>


Thanks for writing it up as a patch. Couple of nit-picky comments below:

> Index: linux-2.6.19-rc4-git3/include/asm-powerpc/rtas.h
> ===================================================================
> --- linux-2.6.19-rc4-git3.orig/include/asm-powerpc/rtas.h	2006-11-01 16:15:46.000000000 -0600
> +++ linux-2.6.19-rc4-git3/include/asm-powerpc/rtas.h	2006-11-15 11:44:24.000000000 -0600
> @@ -54,8 +54,6 @@ struct rtas_args {
>  	rtas_arg_t *rets;     /* Pointer to return values in args[]. */
>  };  
>  
> -extern struct rtas_args rtas_stop_self_args;
> -
>  struct rtas_t {
>  	unsigned long entry;		/* physical address pointer */
>  	unsigned long base;		/* physical address pointer */
> @@ -223,7 +221,9 @@ extern int rtas_get_error_log_max(void);
>  extern spinlock_t rtas_data_buf_lock;
>  extern char rtas_data_buf[RTAS_DATA_BUF_SIZE];
>  
> +#ifdef CONFIG_HOTPLUG_CPU
>  extern void rtas_stop_self(void);
> +#endif

I think we usually don't bother with #ifdef'ing externs, unless you're
providing a no-op implementation for the !CONFIG_FOO case.
 
>  /* RMO buffer reserved for user-space RTAS use */
>  extern unsigned long rtas_rmo_buf;
> Index: linux-2.6.19-rc4-git3/arch/powerpc/platforms/pseries/setup.c
> ===================================================================
> --- linux-2.6.19-rc4-git3.orig/arch/powerpc/platforms/pseries/setup.c	2006-11-01 16:15:09.000000000 -0600
> +++ linux-2.6.19-rc4-git3/arch/powerpc/platforms/pseries/setup.c	2006-11-15 11:46:10.000000000 -0600
> @@ -347,6 +347,7 @@ static int __init pSeries_init_panel(voi
>  }
>  arch_initcall(pSeries_init_panel);
>  
> +#ifdef CONFIG_HOTPLUG_CPU
>  static void pSeries_mach_cpu_die(void)
>  {
>  	local_irq_disable();
> @@ -357,6 +358,7 @@ static void pSeries_mach_cpu_die(void)
>  	BUG();
>  	for(;;);
>  }
  +#else
   #define pSeries_mach_cpu_die NULL
> +#endif

Not sure if that's a good idea, it would save the #ifdef around the
structure assignment below.

>  static int pseries_set_dabr(unsigned long dabr)
>  {
> @@ -558,7 +560,9 @@ define_machine(pseries) {
>  	.power_off		= rtas_power_off,
>  	.halt			= rtas_halt,
>  	.panic			= rtas_os_term,
> +#ifdef CONFIG_HOTPLUG_CPU
>  	.cpu_die		= pSeries_mach_cpu_die,
> +#endif
>  	.get_boot_time		= rtas_get_boot_time,
>  	.get_rtc_time		= rtas_get_rtc_time,
>  	.set_rtc_time		= rtas_set_rtc_time,

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH]: wrap cpu_die() with CONFIG_HOTPLUG_CPU

[Linas Vepstas]

Paul,

Please apply; this is an updated patch per Michael Ellerman,
below.

--linas

On Thu, Nov 16, 2006 at 10:58:45AM +1100, Michael Ellerman wrote:
> 
> Thanks for writing it up as a patch. Couple of nit-picky comments below:

Nitpicking's OK.

>   +#else
>    #define pSeries_mach_cpu_die NULL
> > +#endif
> 
> Not sure if that's a good idea, it would save the #ifdef around the
> structure assignment below.

Hmm. Personally, I find the #ifdef in the structure to be more 
readable, in the sense that one is less likely to mis-read what
the code is actually doing, when on is skimming quickly. But,
not worth arguing over. The patch below incorporates your 
suggestions.

--linas

Subject: [PATCH]: wrap cpu_die() with CONFIG_HOTPLUG_CPU

Per email discussion, it appears that rtas_stop_self()
and pSeries_mach_cpu_die() should not be compiled if 
CONFIG_HOTPLUG_CPU is not defined. This patch adds
#ifdefs around these bits of code.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
 arch/powerpc/kernel/rtas.c             |    5 +++--
 arch/powerpc/platforms/pseries/setup.c |    4 ++++
 include/asm-powerpc/rtas.h             |    2 --
 3 files changed, 7 insertions(+), 4 deletions(-)

Index: linux-2.6.19-rc4-git3/arch/powerpc/kernel/rtas.c
===================================================================
--- linux-2.6.19-rc4-git3.orig/arch/powerpc/kernel/rtas.c	2006-11-15 11:40:14.000000000 -0600
+++ linux-2.6.19-rc4-git3/arch/powerpc/kernel/rtas.c	2006-11-15 12:46:51.000000000 -0600
@@ -810,9 +810,9 @@ asmlinkage int ppc_rtas(struct rtas_args
 	return 0;
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
 /* This version can't take the spinlock, because it never returns */
-
-struct rtas_args rtas_stop_self_args = {
+static struct rtas_args rtas_stop_self_args = {
 	/* The token is initialized for real in setup_system() */
 	.token = RTAS_UNKNOWN_SERVICE,
 	.nargs = 0,
@@ -834,6 +834,7 @@ void rtas_stop_self(void)
 
 	panic("Alas, I survived.\n");
 }
+#endif
 
 /*
  * Call early during boot, before mem init or bootmem, to retrieve the RTAS
Index: linux-2.6.19-rc4-git3/include/asm-powerpc/rtas.h
===================================================================
--- linux-2.6.19-rc4-git3.orig/include/asm-powerpc/rtas.h	2006-11-01 16:15:46.000000000 -0600
+++ linux-2.6.19-rc4-git3/include/asm-powerpc/rtas.h	2006-11-16 15:23:24.000000000 -0600
@@ -54,8 +54,6 @@ struct rtas_args {
 	rtas_arg_t *rets;     /* Pointer to return values in args[]. */
 };  
 
-extern struct rtas_args rtas_stop_self_args;
-
 struct rtas_t {
 	unsigned long entry;		/* physical address pointer */
 	unsigned long base;		/* physical address pointer */
Index: linux-2.6.19-rc4-git3/arch/powerpc/platforms/pseries/setup.c
===================================================================
--- linux-2.6.19-rc4-git3.orig/arch/powerpc/platforms/pseries/setup.c	2006-11-01 16:15:09.000000000 -0600
+++ linux-2.6.19-rc4-git3/arch/powerpc/platforms/pseries/setup.c	2006-11-16 15:25:02.000000000 -0600
@@ -347,6 +347,7 @@ static int __init pSeries_init_panel(voi
 }
 arch_initcall(pSeries_init_panel);
 
+#ifdef CONFIG_HOTPLUG_CPU
 static void pSeries_mach_cpu_die(void)
 {
 	local_irq_disable();
@@ -357,6 +358,9 @@ static void pSeries_mach_cpu_die(void)
 	BUG();
 	for(;;);
 }
+#else
+#define pSeries_mach_cpu_die NULL
+#endif
 
 static int pseries_set_dabr(unsigned long dabr)
 {

Re: [PATCH]: wrap cpu_die() with CONFIG_HOTPLUG_CPU

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1169 bytes --]

On Thu, 2006-11-16 at 15:41 -0600, Linas Vepstas wrote:
> Paul,
> 
> Please apply; this is an updated patch per Michael Ellerman,
> below.
> 
> --linas
> 
> On Thu, Nov 16, 2006 at 10:58:45AM +1100, Michael Ellerman wrote:
> > 
> > Thanks for writing it up as a patch. Couple of nit-picky comments below:
> 
> Nitpicking's OK.
> 
> >   +#else
> >    #define pSeries_mach_cpu_die NULL
> > > +#endif
> > 
> > Not sure if that's a good idea, it would save the #ifdef around the
> > structure assignment below.
> 
> Hmm. Personally, I find the #ifdef in the structure to be more 
> readable, in the sense that one is less likely to mis-read what
> the code is actually doing, when on is skimming quickly. But,
> not worth arguing over. The patch below incorporates your 
> suggestions.

Yeah, it's a trade off, I'm not 100% sure it's a good idea - but we'll
see what others think :)

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]