Re: [PATCH RFC] Priority boosting for preemptible RCU

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 22 Aug 2007 14:22:16 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> On Wed, Aug 22, 2007 at 12:43:40PM -0700, Andrew Morton wrote:
> > On Wed, 22 Aug 2007 12:02:54 -0700
> > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > 
> > > Hello!
> > > 
> > > This patch is a forward-port of RCU priority boosting (described in
> > > http://lwn.net/Articles/220677/).  It applies to 2.6.22 on top of
> > > the patches sent in the http://lkml.org/lkml/2007/8/7/276 series and
> > > the hotplug patch (http://lkml.org/lkml/2007/8/17/262).  Passes several
> > > hours of rcutorture on x86_64 and POWER, so OK for experimentation but
> > > not ready for inclusion.
> > 
> > It'd be nice to have a brief summary of why we might want this code in Linux.
> 
> Good point -- will add something like the following in the next rev:
> 
> 	RCU priority boosting is needed when running a workload that might
> 	include CPU-bound user tasks running at realtime priorities with
> 	a CONFIG_PREEMPT build of the kernel.  In this situation, RCU
> 	priority boosting is needed to avoid OOM.
> 
> Does that cover it?

yup

> > > +config PREEMPT_RCU_BOOST_STATS_INTERVAL
> > 
> > Four new config options?  Sob.  Zero would be preferable.
> 
> Hmmm...  I should be able to fold this into PREEMPT_RCU_BOOST_STATS,
> now that you mention it.  Zero to disable, other number to specify
> interval.  And I should move this to the kernel-hacking group as
> well.  Would that help?

The fewer the better.

We want to avoid options which some people might want to enable in normal
production and which other people might want to disable in normal
production.  Because most people don't build custom kernels and the person
who builds their kernels for them needs to make a decision for them.  We
don't want to force the person who configures others' kernels to have to
make nasty compromises.

Config options which are clearly kernel-devleoper-only are fine: people can
just turn them off for production.

> > for_each_possible_cpu() can sometimes do a *lot* more work than
> > for_each_online_cpu().  And even for_each_present_cpu().
> 
> for_each_online_cpu() would not cut it here, but for_each_present_cpu()
> might -- as long as no platforms physically hotplug the CPUs.

Platforms do physically hotplug cpus.  All the hotplug notifier stuff is
there so that code such as yours can synchronise against that.

> 
> > Andy, can we have a checkpatch rule for SPIN_LOCK_UNLOCKED please?  It's
> > basically always wrong.
> 
> Even for static initializers for top-level variables?

Yes.  For those, use DEFINE_SPINLOCK().

> > > +	if (unlikely(idx < 0))
> > > +		return (NULL);
> > 
> > return-is-not-a-function
> 
> You lost me on this one...  You presumably aren't asking it to be converted
> to a macro.  You want it manually inlined where called?

Do `return foo;', not `return (foo);' ;)