Linux Ethernet Bridging - Mar 2017 - [Bridge] [PATCH 07/17] net: convert sock.sk_refcnt from atomic_t to refcount_t

From: Herbert Xu
> Sent: 20 March 2017 13:16
> On Mon, Mar 20, 2017 at 11:39:37AM +0100, Peter Zijlstra wrote:
> >
> > Can we at least give a benchmark and have someone run numbers? We
should
> > be able to quantify these things.
> 
> Do you realise how many times this thing gets hit at 10Gb/s or
> higher? Anyway, since you're proposing this change you should
> demonstrate that it does not cause a performance regression.
What checks does refcnt_t actually do?

An extra decrement is hard to detect since the item gets freed early.
I guess making the main 'allocate/free' code hold (say) 64k references
would give some leeway for extra decrements.

An extra increment will be detected when the count eventually wraps.
Unless the error is in a very common path that won't happen for a long time.

On x86 the cpu flags from the 'lock inc/dec' could be used to reasonably
cheaply detect errors - provided you actually generate a forwards branch.

Otherwise having a common, but not every packet, code path verify that the
reference count is 'sane' would give reasonable coverage.

	David

On Mon, Mar 20, 2017 at 02:10:24PM +0000, David Laight
wrote:
> On x86 the cpu flags from the 'lock inc/dec' could be used to
reasonably
> cheaply detect errors - provided you actually generate a forwards branch.
Note that currently there is no arch specific implementation. We could
of course cure this.

But note that the thing you propose; using the overflow flag, can only
reasonably be done on PREEMPT=n kernels, otherwise we have an incredible
number of contexts that can nest.

Sure; getting all starts aligned to double overflow is incredibly rare,
but I don't want to be the one to have to debug that.

From: Peter Zijlstra
> Sent: 20 March 2017 14:28
> On Mon, Mar 20, 2017 at 02:10:24PM +0000, David Laight wrote:
> > On x86 the cpu flags from the 'lock inc/dec' could be used to
reasonably
> > cheaply detect errors - provided you actually generate a forwards
branch.
> 
> Note that currently there is no arch specific implementation. We could
> of course cure this.
> 
> But note that the thing you propose; using the overflow flag, can only
> reasonably be done on PREEMPT=n kernels, otherwise we have an incredible
> number of contexts that can nest.
> 
> Sure; getting all starts aligned to double overflow is incredibly rare,
> but I don't want to be the one to have to debug that.
One overflow would set the overflow flag, you don't need both to fail.

In any case you can use the sign flag.
Say valid count values are -64k to -256 and 0 to MAXINT.
The count will normally be +ve unless the 'main free path'
has released the 64k references it holds.
If the sign bit is set after inc/dec the value is checked; 
might valid, an error, or require the item be freed.

Ok assuming the items have reasonable lifetimes and have a nominal
'delete' function.

	David