From owner-svn-src-all@FreeBSD.ORG Fri Jul 27 13:31:09 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 76554106566B; Fri, 27 Jul 2012 13:31:09 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail14.syd.optusnet.com.au (mail14.syd.optusnet.com.au [211.29.132.195]) by mx1.freebsd.org (Postfix) with ESMTP id 0C0C98FC18; Fri, 27 Jul 2012 13:31:08 +0000 (UTC) Received: from c122-106-171-246.carlnfd1.nsw.optusnet.com.au (c122-106-171-246.carlnfd1.nsw.optusnet.com.au [122.106.171.246]) by mail14.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q6RDUwbF026719 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 27 Jul 2012 23:31:01 +1000 Date: Fri, 27 Jul 2012 23:30:58 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Gleb Smirnoff In-Reply-To: <20120727124534.GT14135@FreeBSD.org> Message-ID: <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> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Konstantin Belousov , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Bruce Evans Subject: Re: svn commit: r238828 - head/sys/sys X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Jul 2012 13:31:09 -0000 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