Subject: Re: [PATCH]: PowerPC: make sure the rtas stop-self token is defined.

From: Michael Ellerman <[email-masked]>
Date: Mon, 13 Nov 2006 11:32:24 +1100

On Fri, 2006-11-10 at 19:01 -0600, Linas Vepstas wrote:
> On Fri, Nov 10, 2006 at 02:01:52PM -0600, Nathan Lynch wrote:
> > Linas Vepstas wrote:
> > > 
> > > There are a variety of code paths that lead to rtas_stop_self()
> > > being called, primarily through cpu_die(). However, rtas_stop_self() 
> > > has a BUG_ON(rtas_args->token == RTAS_UNKNOWN_SERVICE); in it, and
> > > this rtas token is only set up if CONFIG_HOTPLUG_CPU is defined.
> > > 
> > > Rather than wrapping all of the callers of rtas_stop_self()
> > > with CONFIG_HOTPLUG_CPU, it seems wiser to just unwrap the token
> > > definition.
> > 
> > Is there actually a code path that calls rtas_stop_self with
> > CONFIG_HOTPLUG_CPU=n?  That would be a bug, I think.
> 
> Yeah, that was my first reaction too, right next to "user error". 
> But then I started tracing the code paths, and after they multiplied
> a bit, it was clear that the answer wouldn't be clear. 
> 
> So then I philosophized a bit: if the code cannot be called
> unless CONFIG_HOTPLUG_CPU=y, then I should add #ifdef CONFIG_HOTPLUG_CPU
> to make sure that the code doesn't even get compiled in.
> 
> Well, there used to be such ifdefs (according to Joel) but someone 
> removed the ifdefs. I'm guessing that these got removed during the
> powerpc tree migration.
> 
> So... Rather than wrapping all of the callers of rtas_stop_self() ...

I can only find one caller? Is it called via a #define or something?

Wouldn't it be preferable to just change rtas.h like so:

#ifdef CONFIG_HOTPLUG_CPU
extern void rtas_stop_self(void);
#else
static inline void rtas_stop_self(void) { }
#endif

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