svn commit: r238828 - head/sys/sys

[Konstantin Belousov]

On Fri, Jul 27, 2012 at 10:32:55PM +1000, Bruce Evans wrote:
> On Fri, 27 Jul 2012, Gleb Smirnoff wrote:
> 
> >On Fri, Jul 27, 2012 at 02:12:37PM +0300, Konstantin Belousov wrote:
> >K> On Fri, Jul 27, 2012 at 09:16:48AM +0000, Gleb Smirnoff wrote:
> >K> > ...
> >K> > Log:
> >K> >   Add assertion for refcount overflow.
> >K> >
> >K> >   Submitted by:	Andrey Zonov <andrey zonov.org>
> >K> >   Reviewed by:	kib
> >K> It was discussed rather then reviewed.
> >K>
> >K> I suggest that the assert may be expressed as a check after the 
> >increment,
> >K> which verifies that counter is != 0. This allows to avoid namespace
> >K> pollution due to limits.h.
> >
> >Hmm, overflowing unsigned is a defined behavior in C. If Bruce agrees,
> >then I'm happy with KASSERT after increment.
> 
> Comparing with (uint_t)-1 before is equivalent.  You can even omit the
> cast (depend on the default promotion).
> 
> I just noticed that there is a technical problem -- the count is read
> unlocked in the KASSERT.  And since the comparision is for equality,
> if you lose the race reading the count when it reaches the overflow
> threshold, then you won't see it overflow unless it wraps again and
> you win the race next time (or later).  atomic_cmpset could be used
> to clamp the value at the max, but that is too much for an assertion.
Yes, we discussed this with Gleb, and I do not see this as a problem.
To make assert bullet-proof, either fetchadd() shall be used, as
Gleb proposed, or some even more drastic measures applied.

I did not liked fetchadd() proposal because it causes INVARIANTS code
to use different function (and processor instruction in the end) comparing
with !INVARIANTS case.

> 
> Simple locked reads of the count also don't prevent it wrapping and
> going a bit higher than 0 with increments by other CPUs before the
> CPU that notices the overflow can panic.  So the patch in the PR may
> have been better than the one committed (IIRC, it paniced some
> time before wrapping, and people didn't like this).
> 
> I prefer to use signed types, even for, or especially for counters.
> Then if the counter overflows you have a long time to notice this,
> and may notice without explicit testing because negative counts are
> printed somewhere.  Integer overflow gives undefined behaviour
> immediately, and there is a compiler flag to generate tests for it.
> No one ever uses this, and it wouldn't work for variables that need
> atomic accesses anyway.
And there, people complain about loosing half of the counter capacity.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-all/attachments/20120727/ca28ecae/attachment.pgp