Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 11 Nov 2012 06:32:12 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Peter Wemm <peter@wemm.org>
Cc:        Ian Lepore <freebsd@damnhippie.dyndns.org>, Alfred Perlstein <alfred@FreeBSD.org>, Eitan Adler <eadler@FreeBSD.org>, svn-src-all@FreeBSD.org, Alfred Perlstein <bright@mu.org>, src-committers@FreeBSD.org, svn-src-head@FreeBSD.org
Subject:   Re: svn commit: r242847 - in head/sys: i386/include kern
Message-ID:  <20121111061517.H1208@besplex.bde.org>
In-Reply-To: <CAGE5yCp4N7fML05-Tomm0TM-ROBSka5%2Bb9EKJTFR%2ByUpFuGj5Q@mail.gmail.com>
References:  <201211100208.qAA28e0v004842@svn.freebsd.org> <CAF6rxg=HPmQS1T-LFsZ=DuKEqH30iJFpkz%2BJGhLr4OBL8nohjg@mail.gmail.com> <509DC25E.5030306@mu.org> <509E3162.5020702@FreeBSD.org> <509E7E7C.9000104@mu.org> <CAF6rxgmV8dx-gsQceQKuMQEsJ%2BGkExcKYxEvQ3kY%2B5_nSjvA3w@mail.gmail.com> <509E830D.5080006@mu.org> <1352568275.17290.85.camel@revolution.hippie.lan> <CAGE5yCp4N7fML05-Tomm0TM-ROBSka5%2Bb9EKJTFR%2ByUpFuGj5Q@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 10 Nov 2012, Peter Wemm wrote:

> On Sat, Nov 10, 2012 at 9:24 AM, Ian Lepore
> <freebsd@damnhippie.dyndns.org> wrote:
>> On Sat, 2012-11-10 at 08:38 -0800, Alfred Perlstein wrote:
>>> It probably could be added, but then a bunch of other people would
>>> complain about the comment being too wordy or "not in English".
>>
>> The fact that such a thing could happen explains much about the current
>> state of the code.  An outsider could easily come to the conclusion that
>> the FreeBSD motto is something along the lines of "It should be as hard
>> to read as it was to write."
>
> Don't forget to explain that you get 1 maxusers per 2MB of physical
> memory which turns into 64 x 2k clusters and a whole series of side
> effects.
>
> Wouldn't it be nice if we could write "By default, mbuf clusters are
> capped at 6% of physical ram or 6% of kernel address space, whichever
> is smaller" ?
>
> That's a little easier than trying to explain
> maxusers = physpages / (2 * 1024 * 1024 / PAGE_SIZE)
> if (maxusers > 384)
> maxusers = 384 + ((maxusers - 384) / 8);
> nmbclusters = 1024 + maxusers * 64;
>
> I'd sure prefer to explain:
>
> /* pick smaller of kva pages or physical pages */
> if ((physpages / 16) < (kvapages / 16))
> nmbclusters = physpages / 16;
> else
> nmbclusters = kvapages / 16;
>
> Leave maxusers for calculating maxproc.

I prefer to write clear code and not echo it in comments:

 	nmbclusters = min(physpages, kvapages) / 16;

The most interesting point (the reason why 6% was chosen) is not
documented by either.   Unfortunately, this is too simple to work
- probably min() has a wrong type.
- probably physical == virtual limit is too simple
- varies scaling errors for the magic 16.  It assumes that the
   cluster size is the page size.  Your comment avoids this problem
   by saying "mbuf clusters" and not giving their units, so that is
   correct if the units are the same as for the memory sizes.

But don't ask alfred to fix all the old mistakes.  To change to the
above, you at least have to subtract the contribution of nmbclusters
to maxusers and change related magic and check that everything fits
again.

Lots of separate limits risks unduly restricting each while not
actually preventing over-commit unless all the limits are unduly
restrictive.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121111061517.H1208>