From owner-svn-src-head@FreeBSD.ORG Tue Jul 9 12:36:30 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 93A4F2E3; Tue, 9 Jul 2013 12:36:30 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail106.syd.optusnet.com.au (mail106.syd.optusnet.com.au [211.29.132.42]) by mx1.freebsd.org (Postfix) with ESMTP id 432F51D45; Tue, 9 Jul 2013 12:36:29 +0000 (UTC) Received: from c122-106-156-23.carlnfd1.nsw.optusnet.com.au (c122-106-156-23.carlnfd1.nsw.optusnet.com.au [122.106.156.23]) by mail106.syd.optusnet.com.au (Postfix) with ESMTPS id BDDFE3C062E; Tue, 9 Jul 2013 22:36:22 +1000 (EST) Date: Tue, 9 Jul 2013 22:36:20 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Andriy Gapon Subject: Re: svn commit: r253077 - head/sys/kern In-Reply-To: <201307090901.r6991j64023941@svn.freebsd.org> Message-ID: <20130709222704.L16411@besplex.bde.org> References: <201307090901.r6991j64023941@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=Q6eKePKa c=1 sm=1 a=-fB-b2Shr38A:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=M4roAWbnUW4A:10 a=SoqTtPcDvfIZEx2kRbQA:9 a=CjuIK1q_8ugA:10 a=GckuAJ8mm-_RZzYM:21 a=UdTGbyzZN9Vzyiy0:21 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 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: Tue, 09 Jul 2013 12:36:30 -0000 On Tue, 9 Jul 2013, Andriy Gapon wrote: > Log: > should_yield: protect from td_swvoltick being uninitialized or too stale > > The distance between ticks and td_swvoltick should be calculated as > an unsigned number. Previously we could end up comparing a negative > number with hogticks in which case should_yield() would give incorrect > answer. > > We should probably ensure that td_swvoltick is properly initialized. > > Sponsored by: HybridCluster > MFC after: 5 days > > Modified: > head/sys/kern/kern_synch.c > > Modified: head/sys/kern/kern_synch.c > ============================================================================== > --- head/sys/kern/kern_synch.c Tue Jul 9 08:59:39 2013 (r253076) > +++ head/sys/kern/kern_synch.c Tue Jul 9 09:01:44 2013 (r253077) > @@ -581,7 +581,7 @@ int > should_yield(void) > { > > - return (ticks - curthread->td_swvoltick >= hogticks); > + return ((unsigned int)(ticks - curthread->td_swvoltick) >= hogticks); > } Hrmph. Perhaps it should be calculated as an unsigned number, but this calculates it as a signed number, with undefined behaviour if overflow occurs, and then bogusly casts the signed number to unsigned. It also has a style bug in the cast -- "unsigned int" is verbose, and even "unsigned" is too long, so it is spelled "u_int" in the kernel. Next, after the cast the operand types are mismatched, and compilers could reasonably warn about the possible sign extension bugs from this. Compilers do warn about the mismatch for "i < size_t(foo)" in loops. This warning is so annoying and it is not normally produced when both the operands are variables. Many "fixes" for this warning make sign extension bugs worse by casting changing the type of the variable instead of casting the size_t. Bruce