Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 10 Mar 2009 18:57:11 +0000 (UTC)
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org
Subject:   svn commit: r189640 - in stable/7/sys: . cam/scsi contrib/pf dev/ath/ath_hal dev/cxgb ia64/ia64 ia64/include kern sys
Message-ID:  <200903101857.n2AIvBWr070786@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: jhb
Date: Tue Mar 10 18:57:10 2009
New Revision: 189640
URL: http://svn.freebsd.org/changeset/base/189640

Log:
  MFC: Expand the scope of the sysctllock sx lock to protect the sysctl tree
  itself.  This also includes changes to the ia64 machine check code to
  defer adding machine check records to the sysctl tree, removing Giant
  from the CAM code that created dynamic sysctls, and tweaking the teardown
  of da(4) and cd(4) peripheral devices to not hold locks when freeing the
  sysctl tree.

Modified:
  stable/7/sys/   (props changed)
  stable/7/sys/cam/scsi/scsi_cd.c
  stable/7/sys/cam/scsi/scsi_da.c
  stable/7/sys/contrib/pf/   (props changed)
  stable/7/sys/dev/ath/ath_hal/   (props changed)
  stable/7/sys/dev/cxgb/   (props changed)
  stable/7/sys/ia64/ia64/mca.c
  stable/7/sys/ia64/include/mca.h
  stable/7/sys/kern/kern_linker.c
  stable/7/sys/kern/kern_sysctl.c
  stable/7/sys/kern/vfs_init.c
  stable/7/sys/sys/sysctl.h

Modified: stable/7/sys/cam/scsi/scsi_cd.c
==============================================================================
--- stable/7/sys/cam/scsi/scsi_cd.c	Tue Mar 10 18:41:06 2009	(r189639)
+++ stable/7/sys/cam/scsi/scsi_cd.c	Tue Mar 10 18:57:10 2009	(r189640)
@@ -401,11 +401,6 @@ cdcleanup(struct cam_periph *periph)
 
 	xpt_print(periph->path, "removing device entry\n");
 
-	if ((softc->flags & CD_FLAG_SCTX_INIT) != 0
-	    && sysctl_ctx_free(&softc->sysctl_ctx) != 0) {
-		xpt_print(periph->path, "can't remove sysctl context\n");
-	}
-
 	/*
 	 * In the queued, non-active case, the device in question
 	 * has already been removed from the changer run queue.  Since this
@@ -474,9 +469,14 @@ cdcleanup(struct cam_periph *periph)
 		free(softc->changer, M_DEVBUF);
 	}
 	cam_periph_unlock(periph);
+	if ((softc->flags & CD_FLAG_SCTX_INIT) != 0
+	    && sysctl_ctx_free(&softc->sysctl_ctx) != 0) {
+		xpt_print(periph->path, "can't remove sysctl context\n");
+	}
+
 	disk_destroy(softc->disk);
-	cam_periph_lock(periph);
 	free(softc, M_DEVBUF);
+	cam_periph_lock(periph);
 }
 
 static void
@@ -555,8 +555,6 @@ cdsysctlinit(void *context, int pending)
 	snprintf(tmpstr, sizeof(tmpstr), "CAM CD unit %d", periph->unit_number);
 	snprintf(tmpstr2, sizeof(tmpstr2), "%d", periph->unit_number);
 
-	mtx_lock(&Giant);
-
 	sysctl_ctx_init(&softc->sysctl_ctx);
 	softc->flags |= CD_FLAG_SCTX_INIT;
 	softc->sysctl_tree = SYSCTL_ADD_NODE(&softc->sysctl_ctx,
@@ -565,7 +563,6 @@ cdsysctlinit(void *context, int pending)
 
 	if (softc->sysctl_tree == NULL) {
 		printf("cdsysctlinit: unable to allocate sysctl tree\n");
-		mtx_unlock(&Giant);
 		cam_periph_release(periph);
 		return;
 	}
@@ -579,7 +576,6 @@ cdsysctlinit(void *context, int pending)
 		&softc->minimum_command_size, 0, cdcmdsizesysctl, "I",
 		"Minimum CDB size");
 
-	mtx_unlock(&Giant);
 	cam_periph_release(periph);
 }
 

Modified: stable/7/sys/cam/scsi/scsi_da.c
==============================================================================
--- stable/7/sys/cam/scsi/scsi_da.c	Tue Mar 10 18:41:06 2009	(r189639)
+++ stable/7/sys/cam/scsi/scsi_da.c	Tue Mar 10 18:57:10 2009	(r189640)
@@ -987,6 +987,8 @@ dacleanup(struct cam_periph *periph)
 	softc = (struct da_softc *)periph->softc;
 
 	xpt_print(periph->path, "removing device entry\n");
+	cam_periph_unlock(periph);
+
 	/*
 	 * If we can't free the sysctl tree, oh well...
 	 */
@@ -995,11 +997,10 @@ dacleanup(struct cam_periph *periph)
 		xpt_print(periph->path, "can't remove sysctl context\n");
 	}
 
-	cam_periph_unlock(periph);
 	disk_destroy(softc->disk);
 	callout_drain(&softc->sendordered_c);
-	cam_periph_lock(periph);
 	free(softc, M_DEVBUF);
+	cam_periph_lock(periph);
 }
 
 static void
@@ -1078,7 +1079,6 @@ dasysctlinit(void *context, int pending)
 	snprintf(tmpstr, sizeof(tmpstr), "CAM DA unit %d", periph->unit_number);
 	snprintf(tmpstr2, sizeof(tmpstr2), "%d", periph->unit_number);
 
-	mtx_lock(&Giant);
 	sysctl_ctx_init(&softc->sysctl_ctx);
 	softc->flags |= DA_FLAG_SCTX_INIT;
 	softc->sysctl_tree = SYSCTL_ADD_NODE(&softc->sysctl_ctx,
@@ -1086,7 +1086,6 @@ dasysctlinit(void *context, int pending)
 		CTLFLAG_RD, 0, tmpstr);
 	if (softc->sysctl_tree == NULL) {
 		printf("dasysctlinit: unable to allocate sysctl tree\n");
-		mtx_unlock(&Giant);
 		cam_periph_release(periph);
 		return;
 	}
@@ -1100,7 +1099,6 @@ dasysctlinit(void *context, int pending)
 		&softc->minimum_cmd_size, 0, dacmdsizesysctl, "I",
 		"Minimum CDB size");
 
-	mtx_unlock(&Giant);
 	cam_periph_release(periph);
 }
 

Modified: stable/7/sys/ia64/ia64/mca.c
==============================================================================
--- stable/7/sys/ia64/ia64/mca.c	Tue Mar 10 18:41:06 2009	(r189639)
+++ stable/7/sys/ia64/ia64/mca.c	Tue Mar 10 18:57:10 2009	(r189640)
@@ -42,6 +42,16 @@
 
 MALLOC_DEFINE(M_MCA, "MCA", "Machine Check Architecture");
 
+struct mca_info {
+	STAILQ_ENTRY(mca_info) mi_link;
+	char	mi_name[32];
+	size_t	mi_recsz;
+	char	mi_record[0];
+};
+
+static STAILQ_HEAD(, mca_info) mca_records =
+    STAILQ_HEAD_INITIALIZER(mca_records);
+
 int64_t		mca_info_size[SAL_INFO_TYPES];
 vm_offset_t	mca_info_block;
 struct mtx	mca_info_block_lock;
@@ -76,14 +86,32 @@ mca_sysctl_handler(SYSCTL_HANDLER_ARGS)
 }
 
 void
+ia64_mca_populate(void)
+{
+	struct mca_info *rec;
+
+	mtx_lock_spin(&mca_info_block_lock);
+	while (!STAILQ_EMPTY(&mca_records)) {
+		rec = STAILQ_FIRST(&mca_records);
+		STAILQ_REMOVE_HEAD(&mca_records, mi_link);
+		mtx_unlock_spin(&mca_info_block_lock);
+		(void)SYSCTL_ADD_PROC(NULL, SYSCTL_STATIC_CHILDREN(_hw_mca),
+		    OID_AUTO, rec->mi_name, CTLTYPE_OPAQUE | CTLFLAG_RD,
+		    rec->mi_record, rec->mi_recsz, mca_sysctl_handler, "S,MCA",
+		    "Error record");
+		mtx_lock_spin(&mca_info_block_lock);
+	}
+	mtx_unlock_spin(&mca_info_block_lock);
+}
+
+void
 ia64_mca_save_state(int type)
 {
 	struct ia64_sal_result result;
 	struct mca_record_header *hdr;
-	struct sysctl_oid *oidp;
-	char *name, *state;
+	struct mca_info *rec;
 	uint64_t seqnr;
-	size_t recsz, totsz;
+	size_t recsz;
 
 	/*
 	 * Don't try to get the state if we couldn't get the size of
@@ -95,9 +123,8 @@ ia64_mca_save_state(int type)
 	if (mca_info_block == 0)
 		return;
 
+	mtx_lock_spin(&mca_info_block_lock);
 	while (1) {
-		mtx_lock_spin(&mca_info_block_lock);
-
 		result = ia64_sal_entry(SAL_GET_STATE_INFO, type, 0,
 		    mca_info_block, 0, 0, 0, 0);
 		if (result.sal_status < 0) {
@@ -111,11 +138,13 @@ ia64_mca_save_state(int type)
 
 		mtx_unlock_spin(&mca_info_block_lock);
 
-		totsz = sizeof(struct sysctl_oid) + recsz + 32;
-		oidp = malloc(totsz, M_MCA, M_NOWAIT|M_ZERO);
-		state = (char*)(oidp + 1);
-		name = state + recsz;
-		sprintf(name, "%lld", (long long)seqnr);
+		rec = malloc(sizeof(struct mca_info) + recsz, M_MCA,
+		    M_NOWAIT | M_ZERO);
+		if (rec == NULL)
+			/* XXX: Not sure what to do. */
+			return;
+
+		sprintf(rec->mi_name, "%lld", (long long)seqnr);
 
 		mtx_lock_spin(&mca_info_block_lock);
 
@@ -133,24 +162,14 @@ ia64_mca_save_state(int type)
 			    mca_info_block, 0, 0, 0, 0);
 			if (seqnr != hdr->rh_seqnr) {
 				mtx_unlock_spin(&mca_info_block_lock);
-				free(oidp, M_MCA);
+				free(rec, M_MCA);
+				mtx_lock_spin(&mca_info_block_lock);
 				continue;
 			}
 		}
 
-		bcopy((char*)mca_info_block, state, recsz);
-
-		oidp->oid_parent = &sysctl__hw_mca_children;
-		oidp->oid_number = OID_AUTO;
-		oidp->oid_kind = CTLTYPE_OPAQUE|CTLFLAG_RD|CTLFLAG_DYN;
-		oidp->oid_arg1 = state;
-		oidp->oid_arg2 = recsz;
-		oidp->oid_name = name;
-		oidp->oid_handler = mca_sysctl_handler;
-		oidp->oid_fmt = "S,MCA";
-		oidp->oid_descr = "Error record";
-
-		sysctl_register_oid(oidp);
+		rec->mi_recsz = recsz;
+		bcopy((char*)mca_info_block, rec->mi_record, recsz);
 
 		if (mca_count > 0) {
 			if (seqnr < mca_first)
@@ -161,6 +180,7 @@ ia64_mca_save_state(int type)
 			mca_first = mca_last = seqnr;
 
 		mca_count++;
+		STAILQ_INSERT_TAIL(&mca_records, rec, mi_link);
 
 		/*
 		 * Clear the state so that we get any other records when
@@ -168,8 +188,6 @@ ia64_mca_save_state(int type)
 		 */
 		result = ia64_sal_entry(SAL_CLEAR_STATE_INFO, type, 0, 0, 0,
 		    0, 0, 0);
-
-		mtx_unlock_spin(&mca_info_block_lock);
 	}
 }
 

Modified: stable/7/sys/ia64/include/mca.h
==============================================================================
--- stable/7/sys/ia64/include/mca.h	Tue Mar 10 18:41:06 2009	(r189639)
+++ stable/7/sys/ia64/include/mca.h	Tue Mar 10 18:57:10 2009	(r189640)
@@ -239,6 +239,7 @@ struct mca_pcidev_reg {
 #ifdef _KERNEL
 
 void ia64_mca_init(void);
+void ia64_mca_populate(void);
 void ia64_mca_save_state(int);
 
 #endif /* _KERNEL */

Modified: stable/7/sys/kern/kern_linker.c
==============================================================================
--- stable/7/sys/kern/kern_linker.c	Tue Mar 10 18:41:06 2009	(r189639)
+++ stable/7/sys/kern/kern_linker.c	Tue Mar 10 18:57:10 2009	(r189640)
@@ -292,10 +292,10 @@ linker_file_register_sysctls(linker_file
 	if (linker_file_lookup_set(lf, "sysctl_set", &start, &stop, NULL) != 0)
 		return;
 
-	mtx_lock(&Giant);
+	sysctl_lock();
 	for (oidp = start; oidp < stop; oidp++)
 		sysctl_register_oid(*oidp);
-	mtx_unlock(&Giant);
+	sysctl_unlock();
 }
 
 static void
@@ -309,10 +309,10 @@ linker_file_unregister_sysctls(linker_fi
 	if (linker_file_lookup_set(lf, "sysctl_set", &start, &stop, NULL) != 0)
 		return;
 
-	mtx_lock(&Giant);
+	sysctl_lock();
 	for (oidp = start; oidp < stop; oidp++)
 		sysctl_unregister_oid(*oidp);
-	mtx_unlock(&Giant);
+	sysctl_unlock();
 }
 
 static int

Modified: stable/7/sys/kern/kern_sysctl.c
==============================================================================
--- stable/7/sys/kern/kern_sysctl.c	Tue Mar 10 18:41:06 2009	(r189639)
+++ stable/7/sys/kern/kern_sysctl.c	Tue Mar 10 18:57:10 2009	(r189640)
@@ -64,24 +64,41 @@ static MALLOC_DEFINE(M_SYSCTLOID, "sysct
 static MALLOC_DEFINE(M_SYSCTLTMP, "sysctltmp", "sysctl temp output buffer");
 
 /*
- * Locking - this locks the sysctl tree in memory.
+ * The sysctllock protects the MIB tree.  It also protects sysctl
+ * contexts used with dynamic sysctls.  The sysctl_register_oid() and
+ * sysctl_unregister_oid() routines require the sysctllock to already
+ * be held, so the sysctl_lock() and sysctl_unlock() routines are
+ * provided for the few places in the kernel which need to use that
+ * API rather than using the dynamic API.  Use of the dynamic API is
+ * strongly encouraged for most code.
+ *
+ * This lock is also used to serialize userland sysctl requests.  Some
+ * sysctls wire user memory, and serializing the requests limits the
+ * amount of wired user memory in use.
  */
 static struct sx sysctllock;
 
-#define	SYSCTL_LOCK()		sx_xlock(&sysctllock)
-#define	SYSCTL_UNLOCK()		sx_xunlock(&sysctllock)
-#define	SYSCTL_LOCK_ASSERT()	sx_assert(&sysctllock, SX_XLOCKED)
+#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")
 
 static int sysctl_root(SYSCTL_HANDLER_ARGS);
 
 struct sysctl_oid_list sysctl__children; /* root list */
 
+static int	sysctl_remove_oid_locked(struct sysctl_oid *oidp, int del,
+		    int recurse);
+
 static struct sysctl_oid *
 sysctl_find_oidname(const char *name, struct sysctl_oid_list *list)
 {
 	struct sysctl_oid *oidp;
 
+	SYSCTL_ASSERT_LOCKED();
 	SLIST_FOREACH(oidp, list, oid_link) {
 		if (strcmp(oidp->oid_name, name) == 0) {
 			return (oidp);
@@ -95,6 +112,19 @@ sysctl_find_oidname(const char *name, st
  *
  * Order by number in each list.
  */
+void
+sysctl_lock(void)
+{
+
+	SYSCTL_XLOCK();
+}
+
+void
+sysctl_unlock(void)
+{
+
+	SYSCTL_XUNLOCK();
+}
 
 void
 sysctl_register_oid(struct sysctl_oid *oidp)
@@ -107,6 +137,7 @@ sysctl_register_oid(struct sysctl_oid *o
 	 * First check if another oid with the same name already
 	 * exists in the parent's list.
 	 */
+	SYSCTL_ASSERT_XLOCKED();
 	p = sysctl_find_oidname(oidp->oid_name, parent);
 	if (p != NULL) {
 		if ((p->oid_kind & CTLTYPE) == CTLTYPE_NODE) {
@@ -159,6 +190,7 @@ sysctl_unregister_oid(struct sysctl_oid 
 	struct sysctl_oid *p;
 	int error;
 
+	SYSCTL_ASSERT_XLOCKED();
 	error = ENOENT;
 	if (oidp->oid_number == OID_AUTO) {
 		error = EINVAL;
@@ -190,6 +222,12 @@ sysctl_ctx_init(struct sysctl_ctx_list *
 	if (c == NULL) {
 		return (EINVAL);
 	}
+
+	/*
+	 * No locking here, the caller is responsible for not adding
+	 * new nodes to a context until after this function has
+	 * returned.
+	 */
 	TAILQ_INIT(c);
 	return (0);
 }
@@ -208,8 +246,9 @@ sysctl_ctx_free(struct sysctl_ctx_list *
 	 * XXX This algorithm is a hack. But I don't know any
 	 * XXX better solution for now...
 	 */
+	SYSCTL_XLOCK();
 	TAILQ_FOREACH(e, clist, link) {
-		error = sysctl_remove_oid(e->entry, 0, 0);
+		error = sysctl_remove_oid_locked(e->entry, 0, 0);
 		if (error)
 			break;
 	}
@@ -226,19 +265,22 @@ sysctl_ctx_free(struct sysctl_ctx_list *
 		sysctl_register_oid(e1->entry);
 		e1 = TAILQ_PREV(e1, sysctl_ctx_list, link);
 	}
-	if (error)
+	if (error) {
+		SYSCTL_XUNLOCK();
 		return(EBUSY);
+	}
 	/* Now really delete the entries */
 	e = TAILQ_FIRST(clist);
 	while (e != NULL) {
 		e1 = TAILQ_NEXT(e, link);
-		error = sysctl_remove_oid(e->entry, 1, 0);
+		error = sysctl_remove_oid_locked(e->entry, 1, 0);
 		if (error)
 			panic("sysctl_remove_oid: corrupt tree, entry: %s",
 			    e->entry->oid_name);
 		free(e, M_SYSCTLOID);
 		e = e1;
 	}
+	SYSCTL_XUNLOCK();
 	return (error);
 }
 
@@ -248,6 +290,7 @@ sysctl_ctx_entry_add(struct sysctl_ctx_l
 {
 	struct sysctl_ctx_entry *e;
 
+	SYSCTL_ASSERT_XLOCKED();
 	if (clist == NULL || oidp == NULL)
 		return(NULL);
 	e = malloc(sizeof(struct sysctl_ctx_entry), M_SYSCTLOID, M_WAITOK);
@@ -262,6 +305,7 @@ sysctl_ctx_entry_find(struct sysctl_ctx_
 {
 	struct sysctl_ctx_entry *e;
 
+	SYSCTL_ASSERT_LOCKED();
 	if (clist == NULL || oidp == NULL)
 		return(NULL);
 	TAILQ_FOREACH(e, clist, link) {
@@ -283,13 +327,17 @@ sysctl_ctx_entry_del(struct sysctl_ctx_l
 
 	if (clist == NULL || oidp == NULL)
 		return (EINVAL);
+	SYSCTL_XLOCK();
 	e = sysctl_ctx_entry_find(clist, oidp);
 	if (e != NULL) {
 		TAILQ_REMOVE(clist, e, link);
+		SYSCTL_XUNLOCK();
 		free(e, M_SYSCTLOID);
 		return (0);
-	} else
+	} else {
+		SYSCTL_XUNLOCK();
 		return (ENOENT);
+	}
 }
 
 /*
@@ -301,9 +349,21 @@ sysctl_ctx_entry_del(struct sysctl_ctx_l
 int
 sysctl_remove_oid(struct sysctl_oid *oidp, int del, int recurse)
 {
+	int error;
+
+	SYSCTL_XLOCK();
+	error = sysctl_remove_oid_locked(oidp, del, recurse);
+	SYSCTL_XUNLOCK();
+	return (error);
+}
+
+static int
+sysctl_remove_oid_locked(struct sysctl_oid *oidp, int del, int recurse)
+{
 	struct sysctl_oid *p;
 	int error;
 
+	SYSCTL_ASSERT_XLOCKED();
 	if (oidp == NULL)
 		return(EINVAL);
 	if ((oidp->oid_kind & CTLFLAG_DYN) == 0) {
@@ -322,7 +382,8 @@ sysctl_remove_oid(struct sysctl_oid *oid
 			SLIST_FOREACH(p, SYSCTL_CHILDREN(oidp), oid_link) {
 				if (!recurse)
 					return (ENOTEMPTY);
-				error = sysctl_remove_oid(p, del, recurse);
+				error = sysctl_remove_oid_locked(p, del,
+				    recurse);
 				if (error)
 					return (error);
 			}
@@ -367,6 +428,7 @@ sysctl_add_oid(struct sysctl_ctx_list *c
 	if (parent == NULL)
 		return(NULL);
 	/* Check if the node already exists, otherwise create it */
+	SYSCTL_XLOCK();
 	oidp = sysctl_find_oidname(name, parent);
 	if (oidp != NULL) {
 		if ((oidp->oid_kind & CTLTYPE) == CTLTYPE_NODE) {
@@ -374,8 +436,10 @@ sysctl_add_oid(struct sysctl_ctx_list *c
 			/* Update the context */
 			if (clist != NULL)
 				sysctl_ctx_entry_add(clist, oidp);
+			SYSCTL_XUNLOCK();
 			return (oidp);
 		} else {
+			SYSCTL_XUNLOCK();
 			printf("can't re-use a leaf (%s)!\n", name);
 			return (NULL);
 		}
@@ -413,6 +477,7 @@ sysctl_add_oid(struct sysctl_ctx_list *c
 		sysctl_ctx_entry_add(clist, oidp);
 	/* Register this oid */
 	sysctl_register_oid(oidp);
+	SYSCTL_XUNLOCK();
 	return (oidp);
 }
 
@@ -426,12 +491,14 @@ sysctl_rename_oid(struct sysctl_oid *oid
 	char *newname;
 	void *oldname;
 
-	oldname = (void *)(uintptr_t)(const void *)oidp->oid_name;
 	len = strlen(name);
 	newname = malloc(len + 1, M_SYSCTLOID, M_WAITOK);
 	bcopy(name, newname, len + 1);
 	newname[len] = '\0';
+	SYSCTL_XLOCK();
+	oldname = (void *)(uintptr_t)(const void *)oidp->oid_name;
 	oidp->oid_name = newname;
+	SYSCTL_XUNLOCK();
 	free(oldname, M_SYSCTLOID);
 }
 
@@ -443,15 +510,21 @@ sysctl_move_oid(struct sysctl_oid *oid, 
 {
 	struct sysctl_oid *oidp;
 
-	if (oid->oid_parent == parent)
+	SYSCTL_XLOCK();
+	if (oid->oid_parent == parent) {
+		SYSCTL_XUNLOCK();
 		return (0);
+	}
 	oidp = sysctl_find_oidname(oid->oid_name, parent);
-	if (oidp != NULL)
+	if (oidp != NULL) {
+		SYSCTL_XUNLOCK();
 		return (EEXIST);
+	}
 	sysctl_unregister_oid(oid);
 	oid->oid_parent = parent;
 	oid->oid_number = OID_AUTO;
 	sysctl_register_oid(oid);
+	SYSCTL_XUNLOCK();
 	return (0);
 }
 
@@ -466,8 +539,10 @@ sysctl_register_all(void *arg)
 	struct sysctl_oid **oidp;
 
 	SYSCTL_INIT();
+	SYSCTL_XLOCK();
 	SET_FOREACH(oidp, sysctl_set)
 		sysctl_register_oid(*oidp);
+	SYSCTL_XUNLOCK();
 }
 SYSINIT(sysctl, SI_SUB_KMEM, SI_ORDER_ANY, sysctl_register_all, 0);
 
@@ -497,6 +572,7 @@ sysctl_sysctl_debug_dump_node(struct sys
 	int k;
 	struct sysctl_oid *oidp;
 
+	SYSCTL_ASSERT_LOCKED();
 	SLIST_FOREACH(oidp, l, oid_link) {
 
 		for (k=0; k<i; k++)
@@ -555,6 +631,7 @@ sysctl_sysctl_name(SYSCTL_HANDLER_ARGS)
 	struct sysctl_oid_list *lsp = &sysctl__children, *lsp2;
 	char buf[10];
 
+	SYSCTL_ASSERT_LOCKED();
 	while (namelen) {
 		if (!lsp) {
 			snprintf(buf,sizeof(buf),"%d",*name);
@@ -606,6 +683,7 @@ sysctl_sysctl_next_ls(struct sysctl_oid_
 {
 	struct sysctl_oid *oidp;
 
+	SYSCTL_ASSERT_LOCKED();
 	*len = level;
 	SLIST_FOREACH(oidp, lsp, oid_link) {
 		*next = oidp->oid_number;
@@ -686,7 +764,7 @@ name2oid (char *name, int *oid, int *len
 	struct sysctl_oid_list *lsp = &sysctl__children;
 	char *p;
 
-	SYSCTL_LOCK_ASSERT();
+	SYSCTL_ASSERT_LOCKED();
 
 	if (!*name)
 		return (ENOENT);
@@ -744,7 +822,7 @@ sysctl_sysctl_name2oid(SYSCTL_HANDLER_AR
 	int error, oid[CTL_MAXNAME], len;
 	struct sysctl_oid *op = 0;
 
-	SYSCTL_LOCK_ASSERT();
+	SYSCTL_ASSERT_LOCKED();
 
 	if (!req->newlen) 
 		return (ENOENT);
@@ -1089,9 +1167,9 @@ kernel_sysctl(struct thread *td, int *na
 	req.newfunc = sysctl_new_kernel;
 	req.lock = REQ_LOCKED;
 
-	SYSCTL_LOCK();
+	SYSCTL_SLOCK();
 	error = sysctl_root(0, name, namelen, &req);
-	SYSCTL_UNLOCK();
+	SYSCTL_SUNLOCK();
 
 	if (req.lock == REQ_WIRED && req.validlen > 0)
 		vsunlock(req.oldptr, req.validlen);
@@ -1123,6 +1201,9 @@ kernel_sysctlbyname(struct thread *td, c
 	/*
 	 * XXX: Prone to a possible race condition between lookup and
 	 * execution? Maybe put locking around it?
+	 *
+	 * Userland is just as racy, so I think the current implementation
+	 * is fine.
 	 */
 
 	error = kernel_sysctl(td, oid, 2, oid, &oidlen,
@@ -1234,6 +1315,7 @@ sysctl_find_oid(int *name, u_int namelen
 	struct sysctl_oid *oid;
 	int indx;
 
+	SYSCTL_ASSERT_LOCKED();
 	oid = SLIST_FIRST(&sysctl__children);
 	indx = 0;
 	while (oid && indx < CTL_MAXNAME) {
@@ -1277,7 +1359,7 @@ sysctl_root(SYSCTL_HANDLER_ARGS)
 	struct sysctl_oid *oid;
 	int error, indx, lvl;
 
-	SYSCTL_LOCK_ASSERT();
+	SYSCTL_ASSERT_LOCKED();
 
 	error = sysctl_find_oid(arg1, arg2, &oid, &indx, req);
 	if (error)
@@ -1355,7 +1437,7 @@ struct sysctl_args {
 int
 __sysctl(struct thread *td, struct sysctl_args *uap)
 {
-	int error, name[CTL_MAXNAME];
+	int error, i, name[CTL_MAXNAME];
 	size_t j;
 
 	if (uap->namelen > CTL_MAXNAME || uap->namelen < 2)
@@ -1371,7 +1453,7 @@ __sysctl(struct thread *td, struct sysct
 	if (error && error != ENOMEM)
 		return (error);
 	if (uap->oldlenp) {
-		int i = copyout(&j, uap->oldlenp, sizeof(j));
+		i = copyout(&j, uap->oldlenp, sizeof(j));
 		if (i)
 			return (i);
 	}
@@ -1423,7 +1505,7 @@ userland_sysctl(struct thread *td, int *
 	req.newfunc = sysctl_new_user;
 	req.lock = REQ_LOCKED;
 
-	SYSCTL_LOCK();
+	SYSCTL_XLOCK();
 
 	for (;;) {
 		req.oldidx = 0;
@@ -1434,7 +1516,7 @@ userland_sysctl(struct thread *td, int *
 		uio_yield();
 	}
 
-	SYSCTL_UNLOCK();
+	SYSCTL_XUNLOCK();
 
 	if (req.lock == REQ_WIRED && req.validlen > 0)
 		vsunlock(req.oldptr, req.validlen);

Modified: stable/7/sys/kern/vfs_init.c
==============================================================================
--- stable/7/sys/kern/vfs_init.c	Tue Mar 10 18:41:06 2009	(r189639)
+++ stable/7/sys/kern/vfs_init.c	Tue Mar 10 18:57:10 2009	(r189640)
@@ -165,12 +165,15 @@ vfs_register(struct vfsconf *vfc)
 	 * preserved by re-registering the oid after modifying its
 	 * number.
 	 */
+	sysctl_lock();
 	SLIST_FOREACH(oidp, &sysctl__vfs_children, oid_link)
 		if (strcmp(oidp->oid_name, vfc->vfc_name) == 0) {
 			sysctl_unregister_oid(oidp);
 			oidp->oid_number = vfc->vfc_typenum;
 			sysctl_register_oid(oidp);
+			break;
 		}
+	sysctl_unlock();
 
 	/*
 	 * Initialise unused ``struct vfsops'' fields, to use

Modified: stable/7/sys/sys/sysctl.h
==============================================================================
--- stable/7/sys/sys/sysctl.h	Tue Mar 10 18:41:06 2009	(r189639)
+++ stable/7/sys/sys/sysctl.h	Tue Mar 10 18:57:10 2009	(r189640)
@@ -685,6 +685,8 @@ int	userland_sysctl(struct thread *td, i
 			size_t *retval, int flags);
 int	sysctl_find_oid(int *name, u_int namelen, struct sysctl_oid **noid,
 			int *nindx, struct sysctl_req *req);
+void	sysctl_lock(void);
+void	sysctl_unlock(void);
 int	sysctl_wire_old_buffer(struct sysctl_req *req, size_t len);
 
 #else	/* !_KERNEL */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200903101857.n2AIvBWr070786>