Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 26 Oct 2012 00:25:56 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, Alfred Perlstein <alfred@FreeBSD.org>, src-committers@FreeBSD.org
Subject:   Re: svn commit: r242029 - head/sys/kern
Message-ID:  <20121026001150.Q842@besplex.bde.org>
In-Reply-To: <20121025080551.GG35915@deviant.kiev.zoral.com.ua>
References:  <201210250146.q9P1kLi8043704@svn.freebsd.org> <20121025080551.GG35915@deviant.kiev.zoral.com.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 25 Oct 2012, Konstantin Belousov wrote:

> On Thu, Oct 25, 2012 at 01:46:21AM +0000, Alfred Perlstein wrote:
>> ...
>> Modified: head/sys/kern/subr_param.c
>> ==============================================================================
>> --- head/sys/kern/subr_param.c	Thu Oct 25 01:27:01 2012	(r242028)
>> +++ head/sys/kern/subr_param.c	Thu Oct 25 01:46:20 2012	(r242029)
>> @@ -278,8 +278,16 @@ init_param2(long physpages)
>>  		maxusers = physpages / (2 * 1024 * 1024 / PAGE_SIZE);
>>  		if (maxusers < 32)
>>  			maxusers = 32;
>> -		if (maxusers > 384)
>> -			maxusers = 384;
>> +		/*
>> +		 * Clips maxusers to 384 on machines with <= 4GB RAM or 32bit.
>> +		 * Scales it down 6x for large memory machines.
>> +		 */
>> +		if (maxusers > 384) {
>> +			if (sizeof(void *) <= 4)
>> +			    maxusers = 384;
>> +			else
>> +			    maxusers = 384 + ((maxusers - 384) / 6);
>> +		}
> This is unbelievably weird way to express the '64bit'. The
> #ifdef _LP64 is enough there instead of the runtime check.

Such runtime checks are well optimized by compilers, giving nicer code than
ifdefs.

However, there are lots of other style bugs:
- comments writtens not in English is
- after deciphering the comments, we see that they match the code only in
   the unbelievably weird ways:
   (1) <= 4GB RAM isn't necessarily related to the size of void *.
       In fact, the size of physical memory certainly isn't related.  The
       size of virtual memory is more closely related.  But it is the
       physical memory size that is most relevant here.
   (2) 32 bitness isn't necessaril related to the size of void *.
- after fixing the comments, they would just echo the code, and thus
   shouldn't be made.  No comments are made for the other maxusers
   initializations, although it would be easy to write ones 10 times
   longer than this mail to describe all the historical bogus tuning
   given by maxusers.
- half a level of indentation for the maxusers lines
- excessive parentheses.

Bruce



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