Re: [patch] latency tracer, 2.6.15-rc7

[Paul E. McKenney]

On Fri, Dec 30, 2005 at 11:00:15PM -0500, Lee Revell wrote:
> On Fri, 2005-12-30 at 17:39 -0800, Linus Torvalds wrote:
> > diff --git a/kernel/rcupdate.c b/kernel/rcupdate.c
> > index 48d3bce..b107562 100644
> > --- a/kernel/rcupdate.c
> > +++ b/kernel/rcupdate.c
> > @@ -149,11 +149,10 @@ void fastcall call_rcu_bh(struct rcu_hea
> >         *rdp->nxttail = head;
> >         rdp->nxttail = &head->next;
> >         rdp->count++;
> > -/*
> > - *  Should we directly call rcu_do_batch() here ?
> > - *  if (unlikely(rdp->count > 10000))
> > - *      rcu_do_batch(rdp);
> > - */
> > +
> > +       if (unlikely(++rdp->count > 100))
> > +               set_need_resched();

This should help in UP configurations, or in SMP configurations where
all CPUs are doing call_rcu_bh() very frequently.  I would not expect
it to help in cases where one of several CPUs is frequently executing
call_rcu_bh(), but where the other CPUs are either CPU-bound in user
space or are in a tickful idle state.

My guess is that Lee would be running a UP kernel.

> >         local_irq_restore(flags);
> >  } 
> 
> This increments rdp->count twice - is that intentional?

If it was intentional, I would argue instead for

	if (unlikely(rdp->count > 50))

> Also what was the story deal with the commented out code?

Any number of people, myself included, have been seduced by the thought
of invoking callbacks from call_rcu() or call_rcu_bh() context.  When I
first un-seduced myself from this approach, I documented the reasons
why it is a bad idea in Documentation/RCU/UP.txt -- the gist of it is
that there are a number of self-deadlock traps that are easy to fall into.

						Thanx, Paul
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/