On Fri, Jul 27, 2012 at 10:32:55PM +1000, Bruce Evans wrote: B> > On Fri, Jul 27, 2012 at 02:12:37PM +0300, Konstantin Belousov wrote: B> > K> On Fri, Jul 27, 2012 at 09:16:48AM +0000, Gleb Smirnoff wrote: B> > K> > ... B> > K> > Log: B> > K> > Add assertion for refcount overflow. B> > K> > B> > K> > Submitted by: Andrey Zonov <andrey zonov.org> B> > K> > Reviewed by: kib B> > K> It was discussed rather then reviewed. B> > K> B> > K> I suggest that the assert may be expressed as a check after the increment, B> > K> which verifies that counter is != 0. This allows to avoid namespace B> > K> pollution due to limits.h. B> > B> > Hmm, overflowing unsigned is a defined behavior in C. If Bruce agrees, B> > then I'm happy with KASSERT after increment. B> B> Comparing with (uint_t)-1 before is equivalent. You can even omit the B> cast (depend on the default promotion). B> B> I just noticed that there is a technical problem -- the count is read B> unlocked in the KASSERT. And since the comparision is for equality, B> if you lose the race reading the count when it reaches the overflow B> threshold, then you won't see it overflow unless it wraps again and B> you win the race next time (or later). atomic_cmpset could be used B> to clamp the value at the max, but that is too much for an assertion.
We have discussed that. As alternative I proposed: @@ -50,8 +51,14 @@ static __inline void refcount_acquire(volatile u_int *count) { +#ifdef INVARIANTS + u_int old; + old = atomic_fetchadd_int(count, 1); + KASSERT(old < UINT_MAX, ("refcount %p overflowed", count)); +#else atomic_add_acq_int(count, 1); +#endif } Konstantin didn't like that production code differs from INVARIANTS. So we ended with what I committed, advocating to the fact that although assertion is racy and bad panics still can occur, the "good panics" would occur much more often, and a single "good panic" is enough to show what's going on. -- Totus tuus, Glebius. _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"