From owner-svn-src-all@FreeBSD.ORG Sun Jan 23 15:39:56 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1C7AE106564A; Sun, 23 Jan 2011 15:39:56 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-iw0-f182.google.com (mail-iw0-f182.google.com [209.85.214.182]) by mx1.freebsd.org (Postfix) with ESMTP id BB5E68FC08; Sun, 23 Jan 2011 15:39:55 +0000 (UTC) Received: by iwn39 with SMTP id 39so3315911iwn.13 for ; Sun, 23 Jan 2011 07:39:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=aX9RyIpukijFNLWSMt3OqGMKxrGUOwuoZING8K3TAoo=; b=hKtO7AOX0bUJtztr/EXqFb4sCrdwkZdNjnzmk3v5aTEAyjAj8agEiLhyTINHge6mqG WcJXvJSEsYmzlKGGo/autp4RDLEBlH1v0oUFemO3bXKi3wTzv14Cm3WaUZ54tRShiffp kYHxvTpULSniePnYSvst+OPE+PuJ2b9NVNivQ= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=thK3d5D9FuL6Jcb/rOzsB12h9VwkdLJ9XXJyl+sgIK/rDmG5bmHS4DcuBzJFoGe9RW 1zfj9Z6iVAPjpVo6tXA43+L25c2LyUKQcJ46M1d7D/ROgy+qUe2SlJex1IqSMVmsyV/2 D0uZqGPO7oWcl/vu0H+nOKBVqUBybfXMhUHo8= MIME-Version: 1.0 Received: by 10.231.161.15 with SMTP id p15mr3478731ibx.104.1295797194510; Sun, 23 Jan 2011 07:39:54 -0800 (PST) Sender: mdf356@gmail.com Received: by 10.231.160.147 with HTTP; Sun, 23 Jan 2011 07:39:54 -0800 (PST) In-Reply-To: <201101231300.p0ND0PZi055936@svn.freebsd.org> References: <201101231300.p0ND0PZi055936@svn.freebsd.org> Date: Sun, 23 Jan 2011 07:39:54 -0800 X-Google-Sender-Auth: jyf0BRFmdKLKO0QRMe2CRrhfyd0 Message-ID: From: mdf@FreeBSD.org To: Lawrence Stewart Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r217748 - head/sys/netinet/cc X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 23 Jan 2011 15:39:56 -0000 For sbuf use for a sysctl you can use sbuf_init_for_sysctl() which will, instead of growing, push the current data out using SYSCTL_OUT to a wired user buffer. There's a few examples in the vm/ code. This can sometimes significantly simplify the code since there's no need to worry about held mutex/rwlock anymore. Thanks, matthew On Sun, Jan 23, 2011 at 5:00 AM, Lawrence Stewart wr= ote: > Author: lstewart > Date: Sun Jan 23 13:00:25 2011 > New Revision: 217748 > URL: http://svn.freebsd.org/changeset/base/217748 > > Log: > =A0An sbuf configured with SBUF_AUTOEXTEND will call malloc with M_WAITOK= when a > =A0write to the buffer causes it to overflow. We therefore can't hold the= CC list > =A0rwlock over a call to sbuf_printf() for an sbuf configured with SBUF_A= UTOEXTEND. > > =A0Switch to a fixed length sbuf which should be of sufficient size excep= t in the > =A0very unlikely event that the sysctl is being processed as one or more = new > =A0algorithms are loaded. If that happens, we accept the race and may fai= l the > =A0sysctl gracefully if there is insufficient room to print the names of = all the > =A0algorithms. > > =A0This should address a WITNESS warning and the potential panic that wou= ld occur > =A0if the sbuf call to malloc did sleep whilst holding the CC list rwlock= . > > =A0Sponsored by: FreeBSD Foundation > =A0Reported by: =A0Nick Hibma > =A0Reviewed by: =A0bz > =A0MFC after: =A0 =A03 weeks > =A0X-MFC with: =A0 r215166 > > Modified: > =A0head/sys/netinet/cc/cc.c > > Modified: head/sys/netinet/cc/cc.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/sys/netinet/cc/cc.c =A0 =A0Sun Jan 23 12:44:17 2011 =A0 =A0 =A0 = =A0(r217747) > +++ head/sys/netinet/cc/cc.c =A0 =A0Sun Jan 23 13:00:25 2011 =A0 =A0 =A0 = =A0(r217748) > @@ -128,20 +128,37 @@ cc_list_available(SYSCTL_HANDLER_ARGS) > =A0{ > =A0 =A0 =A0 =A0struct cc_algo *algo; > =A0 =A0 =A0 =A0struct sbuf *s; > - =A0 =A0 =A0 int err, first; > + =A0 =A0 =A0 int err, first, nalgos; > > - =A0 =A0 =A0 err =3D 0; > + =A0 =A0 =A0 err =3D nalgos =3D 0; > =A0 =A0 =A0 =A0first =3D 1; > - =A0 =A0 =A0 s =3D sbuf_new(NULL, NULL, TCP_CA_NAME_MAX, SBUF_AUTOEXTEND= ); > + > + =A0 =A0 =A0 CC_LIST_RLOCK(); > + =A0 =A0 =A0 STAILQ_FOREACH(algo, &cc_list, entries) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 nalgos++; > + =A0 =A0 =A0 } > + =A0 =A0 =A0 CC_LIST_RUNLOCK(); > + > + =A0 =A0 =A0 s =3D sbuf_new(NULL, NULL, nalgos * TCP_CA_NAME_MAX, SBUF_F= IXEDLEN); > > =A0 =A0 =A0 =A0if (s =3D=3D NULL) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (ENOMEM); > > + =A0 =A0 =A0 /* > + =A0 =A0 =A0 =A0* It is theoretically possible for the CC list to have g= rown in size > + =A0 =A0 =A0 =A0* since the call to sbuf_new() and therefore for the sbu= f to be too > + =A0 =A0 =A0 =A0* small. If this were to happen (incredibly unlikely), t= he sbuf will > + =A0 =A0 =A0 =A0* reach an overflow condition, sbuf_printf() will return= an error and > + =A0 =A0 =A0 =A0* the sysctl will fail gracefully. > + =A0 =A0 =A0 =A0*/ > =A0 =A0 =A0 =A0CC_LIST_RLOCK(); > =A0 =A0 =A0 =A0STAILQ_FOREACH(algo, &cc_list, entries) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err =3D sbuf_printf(s, first ? "%s" : ", %= s", algo->name); > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (err) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (err) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Sbuf overflow condition.= */ > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D EOVERFLOW; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0first =3D 0; > =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0CC_LIST_RUNLOCK(); >