Date: Mon, 29 Nov 2010 08:11:50 -0500 From: John Baldwin <jhb@freebsd.org> To: freebsd-arch@freebsd.org Cc: mdf@freebsd.org Subject: Re: Fixing sysctl LOR Message-ID: <201011290811.50262.jhb@freebsd.org> In-Reply-To: <AANLkTi=yTwfMh8tLtfEFcjQmPYggxo6Nk1T=TeOieYvA@mail.gmail.com> References: <AANLkTi=yTwfMh8tLtfEFcjQmPYggxo6Nk1T=TeOieYvA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201011290811.50262.jhb>