Fixing sysctl LOR
John Baldwin
jhb at freebsd.org
Mon Nov 29 15:32:20 UTC 2010
On Thursday, November 25, 2010 8:44:07 pm mdf at 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
More information about the freebsd-arch
mailing list