Date: Fri, 27 Jul 2012 23:30:58 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Gleb Smirnoff <glebius@freebsd.org> Cc: Konstantin Belousov <kostikbel@gmail.com>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r238828 - head/sys/sys Message-ID: <20120727232757.X7759@besplex.bde.org> In-Reply-To: <20120727124534.GT14135@FreeBSD.org> References: <201207270916.q6R9Gm23086648@svn.freebsd.org> <20120727111237.GC2676@deviant.kiev.zoral.com.ua> <20120727111904.GQ14135@FreeBSD.org> <20120727221529.K7360@besplex.bde.org> <20120727124534.GT14135@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120727232757.X7759>