Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 27 Jul 2012 18:49:18 +0400
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        Konstantin Belousov <kostikbel@gmail.com>, svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r238828 - head/sys/sys
Message-ID:  <20120727144918.GV14135@FreeBSD.org>
In-Reply-To: <20120727232757.X7759@besplex.bde.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> <20120727232757.X7759@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jul 27, 2012 at 11:30:58PM +1000, Bruce Evans wrote:
B> > On Fri, Jul 27, 2012 at 10:32:55PM +1000, Bruce Evans wrote:
B> > B> I just noticed that there is a technical problem -- the count is read
B> > B> unlocked in the KASSERT.  And since the comparision is for equality,
B> > B> if you lose the race reading the count when it reaches the overflow
B> > B> threshold, then you won't see it overflow unless it wraps again and
B> > B> you win the race next time (or later).  atomic_cmpset could be used
B> > B> to clamp the value at the max, but that is too much for an assertion.
B> >
B> > We have discussed that. As alternative I proposed:
B> >
B> > @@ -50,8 +51,14 @@
B> > static __inline void
B> > refcount_acquire(volatile u_int *count)
B> > {
B> > +#ifdef INVARIANTS
B> > +       u_int old;
B> > +       old = atomic_fetchadd_int(count, 1);
B> > +       KASSERT(old < UINT_MAX, ("refcount %p overflowed", count));
B> > +#else
B> >        atomic_add_acq_int(count, 1);
B> > +#endif
B> > }
B> >
B> > Konstantin didn't like that production code differs from INVARIANTS.
B> >
B> > So we ended with what I committed, advocating to the fact that although
B> > assertion is racy and bad panics still can occur, the "good panics"
B> > would occur much more often, and a single "good panic" is enough to
B> > show what's going on.
B> 
B> Yes, it is excessive.
B> 
B> So why do people even care about this particular overflow?  There are
B> many integers that can overflow in the kernel.  Some binary wraparounds
B> are even intentional.

Because "negative refcount" panic is very confusing in the case when one
got overflow.

http://lists.freebsd.org/pipermail/freebsd-net/2012-July/032822.html

-- 
Totus tuus, Glebius.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120727144918.GV14135>