svn commit: r216060 - in head/sys: kern sys
Matthew D Fleming
mdf at FreeBSD.org
Mon Nov 29 18:18:08 UTC 2010
Author: mdf
Date: Mon Nov 29 18:18:07 2010
New Revision: 216060
URL: http://svn.freebsd.org/changeset/base/216060
Log:
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 .
Reviewed by: jhb
MFC after: 1 month
X-MFC-note: split oid_refcnt field for oid_running to preserve KBI
Modified:
head/sys/kern/kern_sysctl.c
head/sys/sys/sysctl.h
Modified: head/sys/kern/kern_sysctl.c
==============================================================================
--- head/sys/kern/kern_sysctl.c Mon Nov 29 18:18:00 2010 (r216059)
+++ head/sys/kern/kern_sysctl.c Mon Nov 29 18:18:07 2010 (r216060)
@@ -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>
@@ -86,13 +87,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);
@@ -106,7 +106,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);
@@ -313,7 +313,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) {
@@ -409,6 +409,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,
@@ -581,7 +591,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++)
@@ -622,7 +632,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);
}
@@ -640,7 +652,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);
@@ -649,7 +661,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;
@@ -665,7 +677,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++;
@@ -681,7 +693,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, "");
@@ -692,7 +707,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;
@@ -756,7 +771,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));
@@ -773,7 +790,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);
@@ -831,8 +848,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 */
@@ -848,7 +863,9 @@ sysctl_sysctl_name2oid(SYSCTL_HANDLER_AR
p [req->newlen] = '\0';
+ SYSCTL_XLOCK();
error = name2oid(p, oid, &len, &op);
+ SYSCTL_XUNLOCK();
free(p, M_SYSCTL);
@@ -868,16 +885,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);
}
@@ -891,13 +913,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);
}
@@ -1177,9 +1204,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);
@@ -1307,7 +1334,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) {
@@ -1326,6 +1353,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);
@@ -1333,6 +1362,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);
@@ -1352,7 +1383,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)
@@ -1416,12 +1447,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);
}
@@ -1521,9 +1561,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: head/sys/sys/sysctl.h
==============================================================================
--- head/sys/sys/sysctl.h Mon Nov 29 18:18:00 2010 (r216059)
+++ head/sys/sys/sysctl.h Mon Nov 29 18:18:07 2010 (r216060)
@@ -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.
@@ -163,6 +164,7 @@ struct sysctl_oid {
int (*oid_handler)(SYSCTL_HANDLER_ARGS);
const char *oid_fmt;
int oid_refcnt;
+ u_int oid_running;
const char *oid_descr;
};
@@ -222,7 +224,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-head
mailing list