From owner-p4-projects@FreeBSD.ORG  Sat Feb 11 23:33:51 2006
Return-Path: <owner-p4-projects@FreeBSD.ORG>
X-Original-To: p4-projects@freebsd.org
Delivered-To: p4-projects@freebsd.org
Received: by hub.freebsd.org (Postfix, from userid 32767)
	id BAF7116A423; Sat, 11 Feb 2006 23:33:50 +0000 (GMT)
X-Original-To: perforce@freebsd.org
Delivered-To: perforce@freebsd.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP id 6508116A426;
	Sat, 11 Feb 2006 23:33:50 +0000 (GMT)
	(envelope-from rwatson@FreeBSD.org)
Received: from cyrus.watson.org (cyrus.watson.org [209.31.154.42])
	by mx1.FreeBSD.org (Postfix) with ESMTP id 8DB7C43D45;
	Sat, 11 Feb 2006 23:33:49 +0000 (GMT)
	(envelope-from rwatson@FreeBSD.org)
Received: from fledge.watson.org (fledge.watson.org [209.31.154.41])
	by cyrus.watson.org (Postfix) with ESMTP id E7E2F46BB6;
	Sat, 11 Feb 2006 18:33:35 -0500 (EST)
Date: Sat, 11 Feb 2006 23:36:39 +0000 (GMT)
From: Robert Watson <rwatson@FreeBSD.org>
X-X-Sender: robert@fledge.watson.org
To: Peter Wemm <peter@FreeBSD.org>
In-Reply-To: <200602111826.k1BIQLrI082592@repoman.freebsd.org>
Message-ID: <20060211233448.D71792@fledge.watson.org>
References: <200602111826.k1BIQLrI082592@repoman.freebsd.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
Cc: Perforce Change Reviews <perforce@freebsd.org>
Subject: Re: PERFORCE change 91580 for review
X-BeenThere: p4-projects@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: p4 projects tree changes <p4-projects.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/p4-projects>,
	<mailto:p4-projects-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/p4-projects>
List-Post: <mailto:p4-projects@freebsd.org>
List-Help: <mailto:p4-projects-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/p4-projects>,
	<mailto:p4-projects-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Sat, 11 Feb 2006 23:33:51 -0000


On Sat, 11 Feb 2006, Peter Wemm wrote:

> http://perforce.freebsd.org/chv.cgi?CH=91580
>
> Change 91580 by peter@peter_daintree on 2006/02/11 18:25:50
>
> 	Try making the nmbclusters sysctl actually do something.  Out of paranoia,
> 	do not allow it to reduce the current value - ie: increase only.  It would
> 	probably work, but wouldn't be healthy to the network stack if we allowed it
> 	to be run into the ground.

...

> -/* XXX: These should be tuneables. Can't change UMA limits on the fly. */
> -SYSCTL_INT(_kern_ipc, OID_AUTO, nmbclusters, CTLFLAG_RW, &nmbclusters, 0,
> -    "Maximum number of mbuf clusters allowed");
> +static int
> +nmbcheck(SYSCTL_HANDLER_ARGS)
> +{
> +	int error, oldnmbclusters;
> +
> +	oldnmbclusters = nmbclusters;
> +	error = sysctl_handle_int(oidp, oidp->oid_arg1, oidp->oid_arg2, req);

Should this be:

 	error = sysctl_handle_int(oidp, &nmbclusters, NULL, req);

> +	if (error == 0 && req->newptr) {
> +		if (nmbclusters < oldnmbclusters) {
> +			nmbclusters = oldnmbclusters;
> +			return (EINVAL);
> +		}

Would it be better to use a newnmbclusters variable so as to avoid frobbing 
the global back and forth in the event that you abort the change?

If the value does change, we also don't need to call into UMA, we can just 
return.

Robert N M Watson