Re: [PATCH 2/6] powerpc: Add enable_ppr kernel parameter to enable PPR save/restore

[Ryan Arnold <rsa@us.ibm.com>]

On Tue, 2012-09-11 at 15:55 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2012-09-10 at 22:42 -0700, Haren Myneni wrote:
> > 
> > Thanks Michael. Yes, we noticed 6% overhead with null syscall test.
> > Hence added cmdline option as suggested. I will add this comment in
> > the
> > changelog.
> > 
> > Regarding the option name, I thought about various ones such as
> > retain_process_ppr, retain_smt_priority, save_ppr and etc. Finally
> > added
> > 'enable_ppr' since it enables CPU_FTR (CPU_FTR_HAS_PPR) which allows
> > to
> > save/restore PPR value. Sure, I will change this option.
> 
> No, that isn't a problem with the name. It's a problem with the polarity
> of the option.
> 
> If you need a command line argument to enable the option, then nobody
> will enable it, it's pointless.

In GLIBC (ppc.h) we'll be providing a user space API to change the
thread priority in user state.  We're also interested in using this in
some of the locking constructs if performance tests indicate it's
beneficial.

I have concerns with being able to enable/disable this option at boot
time.  Usually, in GLIBC we'll just do a kernel version check and enable
certain facilities if we're building against a particular kernel that
supports them.

In this case, with a configurable option, GLIBC is going to need the
kernel to export a hwcap bit that tells us whether we need to do the
save/restore ourselves.  Having to check the hwcap, and do the
save/restore in user space will, of course, increase the overhead on our
side.

If no hwcap bit is provided and this is disabled at kernel boot time, no
check is done and the user process assumes it's running under a certain
priority when it is, in-fact, not.  I don't care for this option.  We'll
be hitting code paths that are ineffective and unnecessary.

Ryan S. Arnold
Linux Technology Center