From: Michael Ellerman <michael@ellerman.id.au>
To: Linas Vepstas <linas@austin.ibm.com>
Cc: linuxppc-dev@ozlabs.org, Nathan Lynch <ntl@pobox.com>, paulus@samba.org
Subject: Re: [PATCH]: PowerPC: make sure the rtas stop-self token is defined.
Date: Mon, 13 Nov 2006 11:32:24 +1100 [thread overview]
Message-ID: <1163377944.7410.12.camel@localhost.localdomain> (raw)
In-Reply-To: <20061111010108.GX30625@austin.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]
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
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
next prev parent reply other threads:[~2006-11-13 0:32 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-11-10 18:22 [PATCH]: PowerPC: make sure the rtas stop-self token is defined Linas Vepstas
2006-11-10 20:01 ` Nathan Lynch
2006-11-11 1:01 ` Linas Vepstas
2006-11-13 0:32 ` Michael Ellerman [this message]
2006-11-13 6:21 ` jschopp
2006-11-13 18:05 ` Linas Vepstas
2006-11-14 1:17 ` Michael Ellerman
2006-11-15 18:08 ` Linas Vepstas
2006-11-10 23:55 ` Paul Mackerras
2006-11-11 0:52 ` Linas Vepstas
2006-11-13 18:57 ` Nathan Lynch
2006-11-13 19:39 ` Linas Vepstas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1163377944.7410.12.camel@localhost.localdomain \
--to=michael@ellerman.id.au \
--cc=linas@austin.ibm.com \
--cc=linuxppc-dev@ozlabs.org \
--cc=ntl@pobox.com \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).