All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Schmidt <will_schmidt@vnet.ibm.com>
To: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: linuxppc-dev@ozlabs.org, Paul Mackerras <paulus@samba.org>
Subject: Re: [PATCH] [POWERPC] fix up log_plpar_hcall_return
Date: Wed, 07 Mar 2007 09:29:05 -0600	[thread overview]
Message-ID: <1173281345.13165.76.camel@localhost> (raw)
In-Reply-To: <20070307111246.79afc104.sfr@canb.auug.org.au>

On Wed, 2007-07-03 at 11:12 +1100, Stephen Rothwell wrote: 
> [sfr done Blue Hair]

Hi Stephen, 

/me looks for blue spraypaint..

> Hi Will,
> 
> On Tue, 06 Mar 2007 17:11:41 -0600 Will Schmidt <will_schmidt@vnet.ibm.com> wrote:
> >
> > This is mostly cosmetic.  This updates log_plpar_hcall_return() to use a
> > case statement rather than an if-then-else jumble, and moves it to
> > rtas.c where it can be near the other rtas related functions.
> 
> You are moving a static function from the only file that calls it.  You
> are exporting it when the only callers are in a file that cannot be part
> of a module and you are (maybe) optimizing something that is only used in
> error paths.

> Sorry, but unless you are doing to introduce others uses, that I don't
> see that this change helps.

Yes.  This moves the function to somewhere a bit more accessible so
others *can* call it.   The comment at the top of the function being
replaced even says something like "find a better place for me...".  

Lparcfg *did* begin it's life as a module, and I'm sure I've turned it
back into a module at least once.   I dont know when it was last
reverted, but suspect it was done by someone who didnt know about
EXPORT_PER_CPU_SYMBOL_GPL, but I digress.. 
I've got a patch to turn lparcfg back into a module, but wanted to start
with this simpler cleanup patch first.

All that, and I've wanted to get rid of that particular if-else mess for
a while.  You are right that it's an error path optimization, though
this change isn't meant as much for optimization as fixing up the horrid
coding style that was initially used.

> [sfr redons normal hair]
How about Purple?  :-)

> 
> I don't mean to sound harsh  :-)
Not a problem.    depending on what other comments I get back, I'll
reorganize a bit and submit a larger patchset with other changes too. 

-Will

> 
> --
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> http://www.canb.auug.org.au/~sfr/

  reply	other threads:[~2007-03-07 15:29 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-06 23:11 [PATCH] [POWERPC] fix up log_plpar_hcall_return Will Schmidt
2007-03-07  0:12 ` Stephen Rothwell
2007-03-07 15:29   ` Will Schmidt [this message]
2007-03-07 14:36 ` Benjamin Herrenschmidt
2007-03-07 15:26   ` Will Schmidt

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=1173281345.13165.76.camel@localhost \
    --to=will_schmidt@vnet.ibm.com \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=paulus@samba.org \
    --cc=sfr@canb.auug.org.au \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.