From owner-svn-src-all@FreeBSD.ORG Fri Feb 25 16:21:11 2011 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 9587D106566C; Fri, 25 Feb 2011 16:21:11 +0000 (UTC) (envelope-from alc@rice.edu) Received: from mh2.mail.rice.edu (mh2.mail.rice.edu [128.42.201.21]) by mx1.freebsd.org (Postfix) with ESMTP id 5C7BF8FC12; Fri, 25 Feb 2011 16:21:11 +0000 (UTC) Received: from mh2.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh2.mail.rice.edu (Postfix) with ESMTP id 4FD5C28FAA7; Fri, 25 Feb 2011 10:05:46 -0600 (CST) X-Virus-Scanned: by amavis-2.6.4 at mh2.mail.rice.edu, auth channel Received: from mh2.mail.rice.edu ([127.0.0.1]) by mh2.mail.rice.edu (mh2.mail.rice.edu [127.0.0.1]) (amavis, port 10026) with ESMTP id 9upgLclPKoHe; Fri, 25 Feb 2011 10:05:46 -0600 (CST) Received: from adsl-216-63-78-18.dsl.hstntx.swbell.net (adsl-216-63-78-18.dsl.hstntx.swbell.net [216.63.78.18]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (No client certificate requested) (Authenticated sender: alc) by mh2.mail.rice.edu (Postfix) with ESMTPSA id 8108B28FAA2; Fri, 25 Feb 2011 10:05:45 -0600 (CST) Message-ID: <4D67D358.2070909@rice.edu> Date: Fri, 25 Feb 2011 10:05:44 -0600 From: Alan Cox User-Agent: Thunderbird 2.0.0.24 (X11/20100725) MIME-Version: 1.0 To: Bruce Evans References: <201102231028.p1NASbET045275@svn.freebsd.org> <20110224063233.Y1100@besplex.bde.org> <1298499116.9366.3.camel@core.nessbank> <20110224102112.P1871@besplex.bde.org> In-Reply-To: <20110224102112.P1871@besplex.bde.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Bruce Cran , svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Bruce Cran Subject: Re: svn commit: r218966 - head/sys/vm 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, 25 Feb 2011 16:21:11 -0000 Bruce Evans wrote: > On Wed, 23 Feb 2011, Bruce Cran wrote: > >> On Thu, 2011-02-24 at 08:23 +1100, Bruce Evans wrote: >> >>> The bug seems to have been overflow in this calculation. >>> [swap_bcount * SWAP_META_PAGES * n / ] >> >> I've attached a patch which changes 'n' to be of type vm_ooffset_t. I >> think this should fix the overflow bug? > > I don't like using vm_ooffset_t either. There are no offsets here, and > it's bad technique to depend on having a large type to avoid overflows > in expressions when the result type is different. > > I would cast operand(s) in the expression as necessary to prevent > overflow > of subexpressions. vm_pindex_t would work, but I prefer to use a type > related to the subexpressions. Not sure what that is. Maybe just > uintmax_t for safety (even that is not safe if the subexpressions have > large values). So: > > (uintmax_t)swap_bcount * SWAP_META_PAGES * n / mumble. > > I like to cast only the leftmost term if possible, and depend on the > larger type propagating to all subexpressions via left-to-right > evaluation. This saves a lot of casts. Here this may be sub-optimal > and we could probably delay the cast to the final multiplication, which > reduces to the same safeness as using uintmax_t for n. > > Next, there is the return type to consider. I don't see why it needs > to be changed from int. The patch in the PR actually changed it to > long, while changing n to vm_offset_t. But on 32-bit machines, long > is essentially the same as int, and vm_offset_t is not much larger. > Even 32-bit machine might actually need a type larger than 32 bits to > prevent overflow in expressions like the above. > With one exception, I would agree with what you have suggested above. I would argue for using a long. HP is already shipping amd64 architecture machines that support 2TB of RAM. In fact, we have already made changes to HEAD so that FreeBSD boots on these machines, albeit with a more modest amount of RAM. So, we are not that far away from the number of 4KB pages overflowing an int. In fact, it might plausibly happen before the End of Life for 9-STABLE. As you point out, most if not all of our page counters are still ints, but I see no reason for new or modified code like this not to begin the transition to a larger type. Alan