Re: [PATCH 03/10] Use percpu stats

Christoph Lameter
2010-09-02T00:34:58Z

On Wed, 1 Sep 2010, Eric Dumazet wrote:

> > The additional advantage would be that for the 64bit case you would have
> > much faster and more compact code.
>
> My implementation is portable and use existing infrastructure, at the
> time it was coded. BTW, its fast on 64bit too. As fast as previous
> implementation. No extra code added. Please double check.

Well if you use the this_cpu_add() on 64 bit you have a single instruction
without the preempt off/on dance.

> If you believe you can do better, please do so.

Its right there in the x86_64 implementation of the this_cpu_ ops.

> Of course, we added 64bit network stats to all 32bit arches only because
> cost was acceptable. (I say all 32bit arches, because you seem to think
> only x86 was the target)

Ok then add your 64 bit on 32 bit implementation as a default for all 32
bit configurations. The point is to not have special implementations for
particular counters. It would be great if the logic you developed for the
network counters could be used in general for all who have issues with 64
bit counter increments.

> Using this_cpu_{add|res}() fallback using atomic ops or spinlocks would
> be slower than actual implemenation (smp_wmb() (nops on x86) and
> increments).

That would be bad. But then I do not see anyone around that wants
to implement such fallback behavior.

The current generic fallback for 64 ops (if the arch does not provide it
which 32 bit arches generally do not) is to switch off preempt and then
perform the 64 bit op. There is also a irqsafe version as well. Have a
look at inclue/linux/percpu.h.