On 2012/7/27 21:30, Bruce Evans wrote:
On Fri, 27 Jul 2012, Gleb Smirnoff wrote:
On Fri, Jul 27, 2012 at 10:32:55PM +1000, Bruce Evans wrote:
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.
Yes, it is excessive.
So why do people even care about this particular overflow? There are
many integers that can overflow in the kernel. Some binary wraparounds
are even intentional.
Bruce
The overflow might not be important. if it is important, all code which
use 32-bit
generation number could also be a problem, how about if it is wrapped around
to same value another thread has just read and waiting for a CPU to run. :D
_______________________________________________
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"
