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

[Michael Ellerman <michael@ellerman.id.au>]

[-- 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 --]