Date: Sun, 09 Dec 2012 14:35:03 -0600 From: Alan Cox <alc@rice.edu> To: Andre Oppermann <andre@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r243668 - in head/sys: kern sys Message-ID: <50C4F5F7.7080101@rice.edu> In-Reply-To: <201211290730.qAT7Uhkv016745@svn.freebsd.org> References: <201211290730.qAT7Uhkv016745@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Andre, I believe that this change did not actually correct the overflow problem. See below for an explanation. On 11/29/2012 01:30, Andre Oppermann wrote: > Author: andre > Date: Thu Nov 29 07:30:42 2012 > New Revision: 243668 > URL: http://svnweb.freebsd.org/changeset/base/243668 > > Log: > Using a long is the wrong type to represent the realmem and maxmbufmem > variable as they may overflow on i386/PAE and i386 with > 2GB RAM. > > Use 64bit quad_t instead. It has broader kernel infrastructure support > with TUNABLE_QUAD_FETCH() and qmin/qmax() than other available types. > > Pointed out by: alc, bde > > Modified: > head/sys/kern/subr_param.c > head/sys/sys/mbuf.h > > Modified: head/sys/kern/subr_param.c > ============================================================================== > --- head/sys/kern/subr_param.c Thu Nov 29 06:26:42 2012 (r243667) > +++ head/sys/kern/subr_param.c Thu Nov 29 07:30:42 2012 (r243668) > @@ -93,7 +93,7 @@ int ncallout; /* maximum # of timer ev > int nbuf; > int ngroups_max; /* max # groups per process */ > int nswbuf; > -long maxmbufmem; /* max mbuf memory */ > +quad_t maxmbufmem; /* max mbuf memory */ > pid_t pid_max = PID_MAX; > long maxswzone; /* max swmeta KVA storage */ > long maxbcache; /* max buffer cache KVA storage */ > @@ -271,7 +271,7 @@ init_param1(void) > void > init_param2(long physpages) > { > - long realmem; > + quad_t realmem; > > /* Base parameters */ > maxusers = MAXUSERS; > @@ -332,10 +332,10 @@ init_param2(long physpages) > * 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); "physpages" is a signed long. Suppose it is 1,000,000. On i386/PAE, the product of 1,000,000 and PAGE_SIZE will be a negative number. Likewise, quad_t is a signed type. So, the negative product of 1,000,000 and PAGE_SIZE will be sign extended to a 64-bit signed value when it is passed to qmin(), and qmin() will return a negative number. > maxmbufmem = realmem / 2; > - TUNABLE_LONG_FETCH("kern.maxmbufmem", &maxmbufmem); > + TUNABLE_QUAD_FETCH("kern.maxmbufmem", &maxmbufmem); > if (maxmbufmem > (realmem / 4) * 3) > maxmbufmem = (realmem / 4) * 3; > > > Modified: head/sys/sys/mbuf.h > ============================================================================== > --- head/sys/sys/mbuf.h Thu Nov 29 06:26:42 2012 (r243667) > +++ head/sys/sys/mbuf.h Thu Nov 29 07:30:42 2012 (r243668) > @@ -395,7 +395,7 @@ struct mbstat { > * > * 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; >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50C4F5F7.7080101>