Re: [PATCH] rcu: eliminate rcu_data.last_qsctr

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

On Wed, Dec 29, 2004 at 04:37:31PM +0100, Manfred Spraul wrote:
> Oleg Nesterov wrote:
> 
> >last_qsctr is used in rcu_check_quiescent_state() exclusively.
> >We can reset qsctr at the start of the grace period, and then
> >just test qsctr against 0.
> >
> > 
> >
> It seems the patch got lost, I've updated it a bit and resent it to Andrew.
> 
> But: I think there is the potential for an even larger cleanup, although 
> this would be more a rewrite:
> Get rid of rcu_check_quiescent_state and instead use something like this 
> in rcu_qsctr_inc:
> 
> static inline void rcu_qsctr_inc(int cpu)
> {
>        struct rcu_data *rdp = &per_cpu(rcu_data, cpu);
>        if (rdp->quiescbatch != rcp->cur) {
>             /* a new grace period is running. And we are at a quiescent
>              * point, so complete it
>              */
>             spin_lock(&rsp->lock);
>             rdp->quiescbatch = rcp->cur;
>             cpu_quiet(rdp->cpu, rcp, rsp);
>            spin_unlock(&rsp->lock);
>     }
> }
> 
> It's just an idea, it needs testing on big systems - does reading from 
> the global rcp from every schedule call cause any problems? The cache 
> line is virtually read-only, so it shouldn't cause trashing, but who knows?

Hello, Manfred,

The main concern I have with this is not cache thrashing of rcp->cur,
but shrinking the grace periods on large systems, which can result in
extra overhead per callback, since the shorter grace periods will tend
to have fewer callbacks.  We saw this problem on some of the early
RCU-infrastructure patches.

Another approach would be to conditionally compile the two versions,
though that might make the code more complex.

						Thanx, Paul