From owner-freebsd-arch@FreeBSD.ORG Mon Nov 29 15:32:20 2010 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 07C701065673; Mon, 29 Nov 2010 15:32:20 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id CEB6B8FC1B; Mon, 29 Nov 2010 15:32:19 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 87F2146B23; Mon, 29 Nov 2010 10:32:19 -0500 (EST) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 740B78A009; Mon, 29 Nov 2010 10:32:18 -0500 (EST) From: John Baldwin To: freebsd-arch@freebsd.org Date: Mon, 29 Nov 2010 08:11:50 -0500 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20101102; KDE/4.4.5; amd64; ; ) References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201011290811.50262.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Mon, 29 Nov 2010 10:32:18 -0500 (EST) X-Virus-Scanned: clamav-milter 0.96.3 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-1.9 required=4.2 tests=BAYES_00 autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on bigwig.baldwin.cx Cc: mdf@freebsd.org Subject: Re: Fixing sysctl LOR X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Nov 2010 15:32:20 -0000 On Thursday, November 25, 2010 8:44:07 pm mdf@freebsd.org wrote: > My previous thought on the matter was incorrect. This patch (and two > small cleanups before it) mean the sysctl lock is no longer held > across the call to oid_handler, which means that WITNESS will no > longer make entries where sysctl lock is taken before any random lock > in the handler. > > The idea is simple; keep track of the number of threads running the > handler so that the oid is not deleted (and sysctl_ctx_free(9) doesn't > return) before all threads are drained. It does unfortunately mean > that the sysctl lock is only taken in exclusive mode, but since it's > held for less time I don't anticipate a significant loss of > concurrency. If there is a simple benchmark someone can recommend I'd > be happy to check the difference. > > I would like at some point to also reduce the number of calls to > sysctl(2) made by sysctl(8); this would also help performance. Among > other things I wonder if eliminating the numerical array to describe > an oid would be acceptable; in a few circumstances it would mean > longer comparisons (strcmp versus integer comparison) but for many > uses it eliminates the name2oid step, and it would also mean there's > no longer a need for fixed numbered entries. > > Cleanup: > http://people.freebsd.org/~mdf/0001-Use-the-SYSCTL_CHILDREN-macro-in- kern_sysctl.c-to-he.patch > http://people.freebsd.org/~mdf/0002-Slightly-modify-the-logic-in- sysctl_find_oid-to-redu.patch These look fine to me. > The patch: > http://people.freebsd.org/~mdf/0003-Do-not-hold-the-sysctl-lock-across-a- call-to-the-han.patch Concurrent sysctl's aren't that big of a gain anyway, especially since it only worked for in-kernel invocations (very rare) and not for userland invocations. Minor nit: --- a/sys/sys/sysctl.h +++ b/sys/sys/sysctl.h @@ -87,7 +87,7 @@ struct ctlname { #define CTLFLAG_MPSAFE 0x00040000 /* Handler is MP safe */ #define CTLFLAG_VNET 0x00020000 /* Prisons with vnet can fiddle */ #define CTLFLAG_RDTUN (CTLFLAG_RD|CTLFLAG_TUN) - +#define CTLFLAG_DYING 0x00010000 /* oid is being removed */ /* * Secure level. Note that CTLFLAG_SECURE == CTLFLAG_SECURE1. * The newline before the block comment should stay. Also, this isn't MFC'able since it breaks the KBI. If you wanted to MFC I suppose you could break the oid_refcnt into two shorts for the MFC patch (but keep it as full int's in HEAD). -- John Baldwin