From owner-svn-src-head@FreeBSD.ORG  Wed Nov 28 03:04:44 2012
Return-Path: <owner-svn-src-head@FreeBSD.ORG>
Delivered-To: svn-src-head@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52])
 by hub.freebsd.org (Postfix) with ESMTP id 8E52AEDA;
 Wed, 28 Nov 2012 03:04:44 +0000 (UTC) (envelope-from bright@mu.org)
Received: from elvis.mu.org (elvis.mu.org [192.203.228.196])
 by mx1.freebsd.org (Postfix) with ESMTP id 432AA8FC18;
 Wed, 28 Nov 2012 03:04:44 +0000 (UTC)
Received: from Alfreds-MacBook-Pro-5.local
 (c-67-180-208-218.hsd1.ca.comcast.net [67.180.208.218])
 by elvis.mu.org (Postfix) with ESMTPSA id 637E81A3D8A;
 Tue, 27 Nov 2012 19:04:37 -0800 (PST)
Message-ID: <50B57F46.1060207@mu.org>
Date: Tue, 27 Nov 2012 19:04:38 -0800
From: Alfred Perlstein <bright@mu.org>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7;
 rv:16.0) Gecko/20121026 Thunderbird/16.0.2
MIME-Version: 1.0
To: Andre Oppermann <andre@freebsd.org>
Subject: Re: svn commit: r243594 - head/sys/netinet
References: <201211270304.qAR34PfD056691@svn.freebsd.org>
 <50B53ABC.1010304@freebsd.org>
In-Reply-To: <50B53ABC.1010304@freebsd.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org,
 Alfred Perlstein <alfred@FreeBSD.org>, src-committers@freebsd.org
X-BeenThere: svn-src-head@freebsd.org
X-Mailman-Version: 2.1.14
Precedence: list
List-Id: SVN commit messages for the src tree for head/-current
 <svn-src-head.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/svn-src-head>,
 <mailto:svn-src-head-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-head>
List-Post: <mailto:svn-src-head@freebsd.org>
List-Help: <mailto:svn-src-head-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-head>,
 <mailto:svn-src-head-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 28 Nov 2012 03:04:44 -0000

On 11/27/12 2:12 PM, Andre Oppermann wrote:
> On 27.11.2012 04:04, Alfred Perlstein wrote:
>> Author: alfred
>> Date: Tue Nov 27 03:04:24 2012
>> New Revision: 243594
>> URL: http://svnweb.freebsd.org/changeset/base/243594
>>
>> Log:
>>    Auto size the tcbhashsize structure based on max sockets.
>>
>>    While here, also make the code that enforces power-of-two more
>>    forgiving, instead of just resetting to 512, graciously round-down
>>    to the next lower power of two.
>
> The porthash (also a parameter to in_pcbinfo_init()) should not be
> scaled the same as tcbhashsize.  It's unlikely that anyone is going
> to have more than a thousand listen ports.
>
> The default tcbhashsize scaling factor of maxsockets / 4 is IMHO
> excessive.  If someone is running at the limit he's going to up
> maxsockets to double the target load.  Dividing by 8 is a better
> balance for normal and well used large memory machines.
> The hashsize calculation logic is done a bit in a roundabout way.
>
> What do think of the attached patch to fix the porthash and to simplify
> the hashsize logic?
>
I like the porthash fix.

However your change:
1) removes explanation of what the code is doing. (comments/clarity)
2) removes the more forgiving code that rounds-down the tcp_tcbhashsize.

Can you please maintain the comments (probably keeping the logic of 
maketcp_hashsize() in a separate function).

Can you please maintain the more forgiving code for dealing with 
non-power of 2 tunable?  I like this, it makes it easier for admins and 
less error prone.

-       if (!powerof2(hashsize)) {
-               int oldhashsize = hashsize;

-               hashsize = maketcp_hashsize(hashsize);
-               /* prevent absurdly low value */
-               if (hashsize < 16)
-                       hashsize = 16;
-               printf("%s: WARNING: TCB hash size not a power of 2, "
-                   "clipped from %d to %d.\n", __func__, oldhashsize,
-                   hashsize);