From owner-svn-src-all@FreeBSD.ORG Wed Nov 28 23:53:57 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id A3E242FD; Wed, 28 Nov 2012 23:53:57 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail05.syd.optusnet.com.au (mail05.syd.optusnet.com.au [211.29.132.186]) by mx1.freebsd.org (Postfix) with ESMTP id 3449E8FC12; Wed, 28 Nov 2012 23:53:56 +0000 (UTC) Received: from c122-106-175-26.carlnfd1.nsw.optusnet.com.au (c122-106-175-26.carlnfd1.nsw.optusnet.com.au [122.106.175.26]) by mail05.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id qASNrk4Q016806 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 29 Nov 2012 10:53:48 +1100 Date: Thu, 29 Nov 2012 10:53:46 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Andre Oppermann Subject: Re: svn commit: r243631 - in head/sys: kern sys In-Reply-To: <50B69B22.80706@freebsd.org> Message-ID: <20121129103059.W1853@besplex.bde.org> References: <201211272119.qARLJxXV061083@svn.freebsd.org> <50B64BE8.3040708@rice.edu> <20121129072617.T1123@besplex.bde.org> <50B69B22.80706@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=FoRdZBXq c=1 sm=1 a=PcN1GEuqyV8A:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=Y5AXMyR2jB0A:10 a=9rSyG_wS7LSbufcbvGEA:9 a=CjuIK1q_8ugA:10 a=9p-u1cbOahvkULHJ:21 a=rgtl55nj80HkSBee:21 a=bxQHXO5Py4tHmhUgaywp5w==:117 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Bruce Evans , Alan Cox X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 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: Wed, 28 Nov 2012 23:53:57 -0000 On Thu, 29 Nov 2012, Andre Oppermann wrote: > On 28.11.2012 22:45, Bruce Evans wrote: >> On Wed, 28 Nov 2012, Alan Cox wrote: >> >>> I'm pretty sure that the "realmem" calculation is going to overflow on >>> i386/PAE, where the number of bytes of physical memory is greater than >>> the type long can represent. >> >> It overflows on i386 even without PAE, where the number of bytes of >> physical memory is greater than the type long can represent (2GB). This is >> the usual case for new >> hardware. > > Please have a look at the attached patch. Is quad_t the appropriate > type to use? If not, which is the right one? quad_t is an old BSD type and shouldn't be used in any code newer than C99. Using a signed type risks sign extension bugs (but using an unsigned type may risk more). >> Reserving half of kva for mbufs is network-centric. I reserve more >> than half of kva for the buffer cache in some configurations >> (VM_BCACHE_SIZE_MAX defaults too 200 MB, but I use 512 MB), and in the >> old scheme where the default for mbufs was under-sized, this may even >> have fitted without separate tuning for mbufs. > > Please note that NO *reservations* are made for mbufs. It's only *limits* > that are enforced. For example stand-alone mbufs were not limited at all > previously and could completely exhaust all available kernel memory. I think there is no real difference. If the limits are reached then virtual memory may still be overcommitted. > Index: kern/subr_param.c > =================================================================== > --- kern/subr_param.c (revision 243631) > +++ kern/subr_param.c (working copy) > @@ -271,7 +271,7 @@ > void > init_param2(long physpages) > { > - long realmem; > + quad_t realmem; > > /* Base parameters */ > maxusers = MAXUSERS; > @@ -332,10 +332,10 @@ > * available kernel memory (physical or kmem). > * At most it can be 3/4 of available kernel memory. > */ > - realmem = lmin(physpages * PAGE_SIZE, > + realmem = qmin(physpages * PAGE_SIZE, > VM_MAX_KERNEL_ADDRESS - VM_MIN_KERNEL_ADDRESS); Oops, libkern hasn't caught up with C99 yet, so it doesn't have uimmin() or uimmax() for uintmax_t. Its only new min/max functions since 1993 are for off_t. These are mistakes, since off_t is not a basic type like all the others and it is much less important than intmax_t. Using these functions is too hard, but 4.4BSD intentionally left out the unsafe macros MIN() and MAX() in the kernel. Someone broke this, so we now have MIN() and MAX() and random choices of which is used. Older subr_param code mostly doesn't use min/max functions, and you could copy this and just use compare-and-assign for uintmax_t here. > maxmbufmem = realmem / 2; > - TUNABLE_LONG_FETCH("kern.maxmbufmem", &maxmbufmem); > + TUNABLE_QUAD_FETCH("kern.maxmbufmem", &maxmbufmem); It is a more intractable problem that tunables also haven't caught up with C99 despite them being born in 1999 too. Nothing larger than quad_t is supported. Values larger than QUAD_MAX are blindly clamped to QUAD_MAX. This is better than for 32-bit ints and longs -- values larger than the respective MAX are blindly truncated mod 2**32. sysctls also haven't caught up with C99. So there is nothing better than TUNABLE_QUAD_FETCH() here. Then you can use any of quad_t, uquad_t, intmax_t or uintmax_t on the value. But quad_t shouldn't be used after C99... > if (maxmbufmem > (realmem / 4) * 3) > maxmbufmem = (realmem / 4) * 3; > > Index: sys/mbuf.h > =================================================================== > --- sys/mbuf.h (revision 243631) > +++ sys/mbuf.h (working copy) > @@ -395,7 +395,7 @@ > * > * The rest of it is defined in kern/kern_mbuf.c > */ > -extern long maxmbufmem; > +extern quad_t maxmbufmem; > extern uma_zone_t zone_mbuf; > extern uma_zone_t zone_clust; > extern uma_zone_t zone_pack; > Bruce