Re: [PATCH RFC 1/2] flow: virtualize get and entry deletion methods

["Timo Teräs" <timo.teras@iki.fi>]

Herbert Xu wrote:
> On Mon, Mar 29, 2010 at 02:23:02PM +0300, Timo Teräs wrote:
>>> I don't see the point.  As long as the data paths do not take
>>> the lock changing this doesn't buy us much.  You're still making
>>> that cacheline exclusive.
>> To my understanding declaring an atomic_t, or reading it with
>> atomic_read does not make cache line exclusive. Only the atomic_*
>> writing to it take the cache line. And since this is done exactly
>> once for policy (or it's a bug/warn thingy) it does not impose
>> significant performance issue.
> 
> I was talking about the lock vs. atomic_xchg in xfrm_policy_kill.
> There is practically no difference for that case.
> 
> Yes, on the read side the lock is a completely different beast
> compared to atomic_read, but I don't see how you can safely
> replace
> 
> 	lock
> 	if (!dead)
> 		take ref
> 	unlock
> 
> without making other changes.

Because the lock is not needed to take ref.

You can take ref as long as someone else is also holding a
valid reference.

The flow cache keeps a reference to each object it is holding,
thus we can always take a reference if we find an object there.
This is what is being done in the current code, and can be
still done in the new code.

The only reason my patch had the lock, was for the dead check.
Since it can be checked without locks, the locking can be
just removed.

The dead check is still an improvement: we find outdated cache
entries without doing a full flush and we find them faster than
using a full flush.

The old code would use policy entries with dead flag set, and
happily return and use them until the policy gc had an
opportunity to run.