svn commit: r220011 - in stable/8/sys: kern sys
Matthew D Fleming
mdf at FreeBSD.org
Fri Mar 25 22:11:39 UTC 2011
Author: mdf
Date: Fri Mar 25 22:11:39 2011
New Revision: 220011
URL: http://svn.freebsd.org/changeset/base/220011
Log:
MFC r216060. This differs from the original commit in that it
preserves the KBI size of struct sysctl_oid. Also, on stable/8 the
compiler thinks that 'len' in sysctl_sysctl_name2oid() is used
uninitialized.
Do not hold the sysctl lock across a call to the handler. This fixes a
general LOR issue where the sysctl lock had no good place in the
hierarchy. One specific instance is #284 on
http://sources.zabbadoz.net/freebsd/lor.html .
Modified:
stable/8/sys/kern/kern_sysctl.c
stable/8/sys/sys/sysctl.h
Directory Properties:
stable/8/sys/ (props changed)
stable/8/sys/amd64/include/xen/ (props changed)
stable/8/sys/cddl/contrib/opensolaris/ (props changed)
stable/8/sys/contrib/dev/acpica/ (props changed)
stable/8/sys/contrib/pf/ (props changed)
Modified: stable/8/sys/kern/kern_sysctl.c
==============================================================================
--- stable/8/sys/kern/kern_sysctl.c Fri Mar 25 22:00:43 2011 (r220010)
+++ stable/8/sys/kern/kern_sysctl.c Fri Mar 25 22:11:39 2011 (r220011)
@@ -42,6 +42,7 @@ __FBSDID("$FreeBSD$");
#include "opt_ktrace.h"
#include <sys/param.h>
+#include <sys/fail.h>
#include <sys/systm.h>
#include <sys/kernel.h>
#include <sys/sysctl.h>
@@ -85,13 +86,12 @@ static MALLOC_DEFINE(M_SYSCTLTMP, "sysct
static struct sx sysctllock;
static struct sx sysctlmemlock;
-#define SYSCTL_SLOCK() sx_slock(&sysctllock)
-#define SYSCTL_SUNLOCK() sx_sunlock(&sysctllock)
#define SYSCTL_XLOCK() sx_xlock(&sysctllock)
#define SYSCTL_XUNLOCK() sx_xunlock(&sysctllock)
#define SYSCTL_ASSERT_XLOCKED() sx_assert(&sysctllock, SA_XLOCKED)
-#define SYSCTL_ASSERT_LOCKED() sx_assert(&sysctllock, SA_LOCKED)
#define SYSCTL_INIT() sx_init(&sysctllock, "sysctl lock")
+#define SYSCTL_SLEEP(ch, wmesg, timo) \
+ sx_sleep(ch, &sysctllock, 0, wmesg, timo)
static int sysctl_root(SYSCTL_HANDLER_ARGS);
@@ -105,7 +105,7 @@ sysctl_find_oidname(const char *name, st
{
struct sysctl_oid *oidp;
- SYSCTL_ASSERT_LOCKED();
+ SYSCTL_ASSERT_XLOCKED();
SLIST_FOREACH(oidp, list, oid_link) {
if (strcmp(oidp->oid_name, name) == 0) {
return (oidp);
@@ -312,7 +312,7 @@ sysctl_ctx_entry_find(struct sysctl_ctx_
{
struct sysctl_ctx_entry *e;
- SYSCTL_ASSERT_LOCKED();
+ SYSCTL_ASSERT_XLOCKED();
if (clist == NULL || oidp == NULL)
return(NULL);
TAILQ_FOREACH(e, clist, link) {
@@ -408,6 +408,16 @@ sysctl_remove_oid_locked(struct sysctl_o
}
sysctl_unregister_oid(oidp);
if (del) {
+ /*
+ * Wait for all threads running the handler to drain.
+ * This preserves the previous behavior when the
+ * sysctl lock was held across a handler invocation,
+ * and is necessary for module unload correctness.
+ */
+ while (oidp->oid_running > 0) {
+ oidp->oid_kind |= CTLFLAG_DYING;
+ SYSCTL_SLEEP(&oidp->oid_running, "oidrm", 0);
+ }
if (oidp->oid_descr)
free((void *)(uintptr_t)(const void *)oidp->oid_descr, M_SYSCTLOID);
free((void *)(uintptr_t)(const void *)oidp->oid_name,
@@ -580,7 +590,7 @@ sysctl_sysctl_debug_dump_node(struct sys
int k;
struct sysctl_oid *oidp;
- SYSCTL_ASSERT_LOCKED();
+ SYSCTL_ASSERT_XLOCKED();
SLIST_FOREACH(oidp, l, oid_link) {
for (k=0; k<i; k++)
@@ -621,7 +631,9 @@ sysctl_sysctl_debug(SYSCTL_HANDLER_ARGS)
error = priv_check(req->td, PRIV_SYSCTL_DEBUG);
if (error)
return (error);
+ SYSCTL_XLOCK();
sysctl_sysctl_debug_dump_node(&sysctl__children, 0);
+ SYSCTL_XUNLOCK();
return (ENOENT);
}
@@ -639,7 +651,7 @@ sysctl_sysctl_name(SYSCTL_HANDLER_ARGS)
struct sysctl_oid_list *lsp = &sysctl__children, *lsp2;
char buf[10];
- SYSCTL_ASSERT_LOCKED();
+ SYSCTL_XLOCK();
while (namelen) {
if (!lsp) {
snprintf(buf,sizeof(buf),"%d",*name);
@@ -648,7 +660,7 @@ sysctl_sysctl_name(SYSCTL_HANDLER_ARGS)
if (!error)
error = SYSCTL_OUT(req, buf, strlen(buf));
if (error)
- return (error);
+ goto out;
namelen--;
name++;
continue;
@@ -664,7 +676,7 @@ sysctl_sysctl_name(SYSCTL_HANDLER_ARGS)
error = SYSCTL_OUT(req, oid->oid_name,
strlen(oid->oid_name));
if (error)
- return (error);
+ goto out;
namelen--;
name++;
@@ -680,7 +692,10 @@ sysctl_sysctl_name(SYSCTL_HANDLER_ARGS)
}
lsp = lsp2;
}
- return (SYSCTL_OUT(req, "", 1));
+ error = SYSCTL_OUT(req, "", 1);
+ out:
+ SYSCTL_XUNLOCK();
+ return (error);
}
static SYSCTL_NODE(_sysctl, 1, name, CTLFLAG_RD, sysctl_sysctl_name, "");
@@ -691,7 +706,7 @@ sysctl_sysctl_next_ls(struct sysctl_oid_
{
struct sysctl_oid *oidp;
- SYSCTL_ASSERT_LOCKED();
+ SYSCTL_ASSERT_XLOCKED();
*len = level;
SLIST_FOREACH(oidp, lsp, oid_link) {
*next = oidp->oid_number;
@@ -755,7 +770,9 @@ sysctl_sysctl_next(SYSCTL_HANDLER_ARGS)
struct sysctl_oid_list *lsp = &sysctl__children;
int newoid[CTL_MAXNAME];
+ SYSCTL_XLOCK();
i = sysctl_sysctl_next_ls(lsp, name, namelen, newoid, &j, 1, &oid);
+ SYSCTL_XUNLOCK();
if (i)
return (ENOENT);
error = SYSCTL_OUT(req, newoid, j * sizeof (int));
@@ -772,7 +789,7 @@ name2oid(char *name, int *oid, int *len,
struct sysctl_oid_list *lsp = &sysctl__children;
char *p;
- SYSCTL_ASSERT_LOCKED();
+ SYSCTL_ASSERT_XLOCKED();
if (!*name)
return (ENOENT);
@@ -830,8 +847,6 @@ sysctl_sysctl_name2oid(SYSCTL_HANDLER_AR
int error, oid[CTL_MAXNAME], len;
struct sysctl_oid *op = 0;
- SYSCTL_ASSERT_LOCKED();
-
if (!req->newlen)
return (ENOENT);
if (req->newlen >= MAXPATHLEN) /* XXX arbitrary, undocumented */
@@ -846,8 +861,10 @@ sysctl_sysctl_name2oid(SYSCTL_HANDLER_AR
}
p [req->newlen] = '\0';
-
+ len = 0;
+ SYSCTL_XLOCK();
error = name2oid(p, oid, &len, &op);
+ SYSCTL_XUNLOCK();
free(p, M_SYSCTL);
@@ -867,16 +884,21 @@ sysctl_sysctl_oidfmt(SYSCTL_HANDLER_ARGS
struct sysctl_oid *oid;
int error;
+ SYSCTL_XLOCK();
error = sysctl_find_oid(arg1, arg2, &oid, NULL, req);
if (error)
- return (error);
+ goto out;
- if (!oid->oid_fmt)
- return (ENOENT);
+ if (oid->oid_fmt == NULL) {
+ error = ENOENT;
+ goto out;
+ }
error = SYSCTL_OUT(req, &oid->oid_kind, sizeof(oid->oid_kind));
if (error)
- return (error);
+ goto out;
error = SYSCTL_OUT(req, oid->oid_fmt, strlen(oid->oid_fmt) + 1);
+ out:
+ SYSCTL_XUNLOCK();
return (error);
}
@@ -890,13 +912,18 @@ sysctl_sysctl_oiddescr(SYSCTL_HANDLER_AR
struct sysctl_oid *oid;
int error;
+ SYSCTL_XLOCK();
error = sysctl_find_oid(arg1, arg2, &oid, NULL, req);
if (error)
- return (error);
+ goto out;
- if (!oid->oid_descr)
- return (ENOENT);
+ if (oid->oid_descr == NULL) {
+ error = ENOENT;
+ goto out;
+ }
error = SYSCTL_OUT(req, oid->oid_descr, strlen(oid->oid_descr) + 1);
+ out:
+ SYSCTL_XUNLOCK();
return (error);
}
@@ -1176,9 +1203,9 @@ kernel_sysctl(struct thread *td, int *na
req.newfunc = sysctl_new_kernel;
req.lock = REQ_LOCKED;
- SYSCTL_SLOCK();
+ SYSCTL_XLOCK();
error = sysctl_root(0, name, namelen, &req);
- SYSCTL_SUNLOCK();
+ SYSCTL_XUNLOCK();
if (req.lock == REQ_WIRED && req.validlen > 0)
vsunlock(req.oldptr, req.validlen);
@@ -1306,7 +1333,7 @@ sysctl_find_oid(int *name, u_int namelen
struct sysctl_oid *oid;
int indx;
- SYSCTL_ASSERT_LOCKED();
+ SYSCTL_ASSERT_XLOCKED();
lsp = &sysctl__children;
indx = 0;
while (indx < CTL_MAXNAME) {
@@ -1325,6 +1352,8 @@ sysctl_find_oid(int *name, u_int namelen
*noid = oid;
if (nindx != NULL)
*nindx = indx;
+ KASSERT((oid->oid_kind & CTLFLAG_DYING) == 0,
+ ("%s found DYING node %p", __func__, oid));
return (0);
}
lsp = SYSCTL_CHILDREN(oid);
@@ -1332,6 +1361,8 @@ sysctl_find_oid(int *name, u_int namelen
*noid = oid;
if (nindx != NULL)
*nindx = indx;
+ KASSERT((oid->oid_kind & CTLFLAG_DYING) == 0,
+ ("%s found DYING node %p", __func__, oid));
return (0);
} else {
return (ENOTDIR);
@@ -1351,7 +1382,7 @@ sysctl_root(SYSCTL_HANDLER_ARGS)
struct sysctl_oid *oid;
int error, indx, lvl;
- SYSCTL_ASSERT_LOCKED();
+ SYSCTL_ASSERT_XLOCKED();
error = sysctl_find_oid(arg1, arg2, &oid, &indx, req);
if (error)
@@ -1415,12 +1446,21 @@ sysctl_root(SYSCTL_HANDLER_ARGS)
if (error != 0)
return (error);
#endif
+ oid->oid_running++;
+ SYSCTL_XUNLOCK();
+
if (!(oid->oid_kind & CTLFLAG_MPSAFE))
mtx_lock(&Giant);
error = oid->oid_handler(oid, arg1, arg2, req);
if (!(oid->oid_kind & CTLFLAG_MPSAFE))
mtx_unlock(&Giant);
+ KFAIL_POINT_ERROR(_debug_fail_point, sysctl_running, error);
+
+ SYSCTL_XLOCK();
+ oid->oid_running--;
+ if (oid->oid_running == 0 && (oid->oid_kind & CTLFLAG_DYING) != 0)
+ wakeup(&oid->oid_running);
return (error);
}
@@ -1520,9 +1560,9 @@ userland_sysctl(struct thread *td, int *
for (;;) {
req.oldidx = 0;
req.newidx = 0;
- SYSCTL_SLOCK();
+ SYSCTL_XLOCK();
error = sysctl_root(0, name, namelen, &req);
- SYSCTL_SUNLOCK();
+ SYSCTL_XUNLOCK();
if (error != EAGAIN)
break;
uio_yield();
Modified: stable/8/sys/sys/sysctl.h
==============================================================================
--- stable/8/sys/sys/sysctl.h Fri Mar 25 22:00:43 2011 (r220010)
+++ stable/8/sys/sys/sysctl.h Fri Mar 25 22:11:39 2011 (r220011)
@@ -87,6 +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.
@@ -164,7 +165,8 @@ struct sysctl_oid {
const char *oid_name;
int (*oid_handler)(SYSCTL_HANDLER_ARGS);
const char *oid_fmt;
- int oid_refcnt;
+ int16_t oid_refcnt;
+ uint16_t oid_running;
const char *oid_descr;
};
@@ -224,7 +226,7 @@ TAILQ_HEAD(sysctl_ctx_list, sysctl_ctx_e
#define SYSCTL_OID(parent, nbr, name, kind, a1, a2, handler, fmt, descr) \
static struct sysctl_oid sysctl__##parent##_##name = { \
&sysctl_##parent##_children, { NULL }, nbr, kind, \
- a1, a2, #name, handler, fmt, 0, __DESCR(descr) }; \
+ a1, a2, #name, handler, fmt, 0, 0, __DESCR(descr) }; \
DATA_SET(sysctl_set, sysctl__##parent##_##name)
#define SYSCTL_ADD_OID(ctx, parent, nbr, name, kind, a1, a2, handler, fmt, descr) \
More information about the svn-src-stable-8
mailing list