From owner-svn-src-head@FreeBSD.ORG Fri Jul 27 12:33:04 2012 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 691E8106566C; Fri, 27 Jul 2012 12:33:04 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail27.syd.optusnet.com.au (mail27.syd.optusnet.com.au [211.29.133.168]) by mx1.freebsd.org (Postfix) with ESMTP id E6DBC8FC15; Fri, 27 Jul 2012 12:33:03 +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 mail27.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q6RCWtFP017590 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 27 Jul 2012 22:32:56 +1000 Date: Fri, 27 Jul 2012 22:32:55 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Gleb Smirnoff In-Reply-To: <20120727111904.GQ14135@FreeBSD.org> Message-ID: <20120727221529.K7360@besplex.bde.org> References: <201207270916.q6R9Gm23086648@svn.freebsd.org> <20120727111237.GC2676@deviant.kiev.zoral.com.ua> <20120727111904.GQ14135@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 Subject: Re: svn commit: r238828 - head/sys/sys X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Jul 2012 12:33:04 -0000 On Fri, 27 Jul 2012, Gleb Smirnoff wrote: > On Fri, Jul 27, 2012 at 02:12:37PM +0300, Konstantin Belousov wrote: > K> On Fri, Jul 27, 2012 at 09:16:48AM +0000, Gleb Smirnoff wrote: > K> > ... > K> > Log: > K> > Add assertion for refcount overflow. > K> > > K> > Submitted by: Andrey Zonov > K> > Reviewed by: kib > K> It was discussed rather then reviewed. > K> > K> I suggest that the assert may be expressed as a check after the increment, > K> which verifies that counter is != 0. This allows to avoid namespace > K> pollution due to limits.h. > > Hmm, overflowing unsigned is a defined behavior in C. If Bruce agrees, > then I'm happy with KASSERT after increment. Comparing with (uint_t)-1 before is equivalent. You can even omit the cast (depend on the default promotion). I just noticed that there is a technical problem -- the count is read unlocked in the KASSERT. And since the comparision is for equality, if you lose the race reading the count when it reaches the overflow threshold, then you won't see it overflow unless it wraps again and you win the race next time (or later). atomic_cmpset could be used to clamp the value at the max, but that is too much for an assertion. Simple locked reads of the count also don't prevent it wrapping and going a bit higher than 0 with increments by other CPUs before the CPU that notices the overflow can panic. So the patch in the PR may have been better than the one committed (IIRC, it paniced some time before wrapping, and people didn't like this). I prefer to use signed types, even for, or especially for counters. Then if the counter overflows you have a long time to notice this, and may notice without explicit testing because negative counts are printed somewhere. Integer overflow gives undefined behaviour immediately, and there is a compiler flag to generate tests for it. No one ever uses this, and it wouldn't work for variables that need atomic accesses anyway. Bruce