Date: Thu, 29 Nov 2012 10:53:46 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Andre Oppermann <andre@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Bruce Evans <brde@optusnet.com.au>, Alan Cox <alc@rice.edu> Subject: Re: svn commit: r243631 - in head/sys: kern sys Message-ID: <20121129103059.W1853@besplex.bde.org> In-Reply-To: <50B69B22.80706@freebsd.org> References: <201211272119.qARLJxXV061083@svn.freebsd.org> <50B64BE8.3040708@rice.edu> <20121129072617.T1123@besplex.bde.org> <50B69B22.80706@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121129103059.W1853>