Re: [PATCH RFC] Priority boosting for preemptible RCU

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Wed, Aug 22, 2007 at 02:41:54PM -0700, Andrew Morton wrote:
> 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.

Sounds good -- PREEMPT_RCU_BOOST_STATS is strictly for kernel developers.

> > > 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.

OK, then I have to stay with for_each_possible_cpu().  If a CPU was
there an hour ago, I need to account for its contribution to the
statistics.  I do heartily agree with the sentiment, however.

> > > 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().

<slap forehead>

> > > > +	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);' ;)

OK, got it.  Another one in the "old habits die hard" list.  ;-)

						Thanx, Paul