Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys Per rmind@'s suggestion, avoid an acquire/release-mutex ...
details: https://anonhg.NetBSD.org/src/rev/0391ce2a66e3
branches: trunk
changeset: 750060:0391ce2a66e3
user: dyoung <dyoung%NetBSD.org@localhost>
date: Tue Dec 15 03:02:24 2009 +0000
description:
Per rmind@'s suggestion, avoid an acquire/release-mutex dance by
collecting garbage in two phases: in the first stage, with
alldevs_mtx held, gather all of the objects to be freed onto a
list. Drop alldevs_mtx, and in the second stage, free all the
collected objects.
Also per rmind@'s suggestion, remove KASSERT(!mutex_owned(&alldevs_mtx))
throughout, it is not useful.
Find a free unit number and allocate it for a new device_t atomically.
Before, two threads would sometimes find the same free unit number
and race to allocate it. The loser panicked. Now there is no
race.
In support of the changes above, extract some new subroutines that
are private to this module: config_unit_nextfree(), config_unit_alloc(),
config_devfree(), config_dump_garbage().
Delete all of the #ifdef __BROKEN_CONFIG_UNIT_USAGE code. Only
the sun3 port still depends on __BROKEN_CONFIG_UNIT_USAGE, it's
not hard for the port to do without, and port-sun3@ had fair warning
that it was going away (>1 week, or a few years' warning, depending
how far back you look!).
diffstat:
sys/kern/subr_autoconf.c | 319 +++++++++++++++++++++++++---------------------
sys/sys/device.h | 6 +-
2 files changed, 181 insertions(+), 144 deletions(-)
diffs (truncated from 580 to 300 lines):
diff -r c3cfe28bf901 -r 0391ce2a66e3 sys/kern/subr_autoconf.c
--- a/sys/kern/subr_autoconf.c Tue Dec 15 03:01:48 2009 +0000
+++ b/sys/kern/subr_autoconf.c Tue Dec 15 03:02:24 2009 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: subr_autoconf.c,v 1.189 2009/11/29 15:17:30 pooka Exp $ */
+/* $NetBSD: subr_autoconf.c,v 1.190 2009/12/15 03:02:24 dyoung Exp $ */
/*
* Copyright (c) 1996, 2000 Christopher G. Demetriou
@@ -77,7 +77,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.189 2009/11/29 15:17:30 pooka Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_autoconf.c,v 1.190 2009/12/15 03:02:24 dyoung Exp $");
#ifdef _KERNEL_OPT
#include "opt_ddb.h"
@@ -166,12 +166,14 @@
static void mapply(struct matchinfo *, cfdata_t);
static device_t config_devalloc(const device_t, const cfdata_t, const int *);
static void config_devdelete(device_t);
+static void config_devunlink(device_t, struct devicelist *);
static void config_makeroom(int, struct cfdriver *);
static void config_devlink(device_t);
static void config_alldevs_unlock(int);
static int config_alldevs_lock(void);
-static void config_collect_garbage(void);
+static void config_collect_garbage(struct devicelist *);
+static void config_dump_garbage(struct devicelist *);
static void pmflock_debug(device_t, const char *, int);
@@ -368,10 +370,11 @@
int
config_cfdriver_detach(struct cfdriver *cd)
{
+ struct devicelist garbage = TAILQ_HEAD_INITIALIZER(garbage);
int i, rc = 0, s;
s = config_alldevs_lock();
- config_collect_garbage();
+ config_collect_garbage(&garbage);
/* Make sure there are no active instances. */
for (i = 0; i < cd->cd_ndevs; i++) {
if (cd->cd_devs[i] != NULL) {
@@ -380,6 +383,7 @@
}
}
config_alldevs_unlock(s);
+ config_dump_garbage(&garbage);
if (rc != 0)
return rc;
@@ -441,6 +445,7 @@
int
config_cfattach_detach(const char *driver, struct cfattach *ca)
{
+ struct devicelist garbage = TAILQ_HEAD_INITIALIZER(garbage);
struct cfdriver *cd;
device_t dev;
int i, rc = 0, s;
@@ -450,7 +455,7 @@
return ESRCH;
s = config_alldevs_lock();
- config_collect_garbage();
+ config_collect_garbage(&garbage);
/* Make sure there are no active instances. */
for (i = 0; i < cd->cd_ndevs; i++) {
if ((dev = cd->cd_devs[i]) == NULL)
@@ -461,6 +466,7 @@
}
}
config_alldevs_unlock(s);
+ config_dump_garbage(&garbage);
if (rc != 0)
return rc;
@@ -951,105 +957,123 @@
config_makeroom(int n, struct cfdriver *cd)
{
int old, new;
- device_t *nsp;
+ device_t *osp, *nsp;
alldevs_nwrite++;
- mutex_exit(&alldevs_mtx);
-
- if (n < cd->cd_ndevs)
- goto out;
-
- /*
- * Need to expand the array.
- */
- old = cd->cd_ndevs;
- if (old == 0)
- new = 4;
- else
- new = old * 2;
- while (new <= n)
- new *= 2;
- nsp = kmem_alloc(sizeof(device_t [new]), KM_SLEEP);
- if (nsp == NULL)
- panic("config_attach: %sing dev array",
- old != 0 ? "expand" : "creat");
- memset(nsp + old, 0, sizeof(device_t [new - old]));
- if (old != 0) {
- memcpy(nsp, cd->cd_devs, sizeof(device_t [old]));
- kmem_free(cd->cd_devs, sizeof(device_t [old]));
+
+ for (new = MAX(4, cd->cd_ndevs); new <= n; new += new)
+ ;
+
+ while (n >= cd->cd_ndevs) {
+ /*
+ * Need to expand the array.
+ */
+ old = cd->cd_ndevs;
+ osp = cd->cd_devs;
+
+ /* Release alldevs_mtx around allocation, which may
+ * sleep.
+ */
+ mutex_exit(&alldevs_mtx);
+ nsp = kmem_alloc(sizeof(device_t[new]), KM_SLEEP);
+ if (nsp == NULL)
+ panic("%s: could not expand cd_devs", __func__);
+ mutex_enter(&alldevs_mtx);
+
+ /* If another thread moved the array while we did
+ * not hold alldevs_mtx, try again.
+ */
+ if (cd->cd_devs != osp) {
+ kmem_free(nsp, sizeof(device_t[new]));
+ continue;
+ }
+
+ memset(nsp + old, 0, sizeof(device_t[new - old]));
+ if (old != 0)
+ memcpy(nsp, cd->cd_devs, sizeof(device_t[old]));
+
+ cd->cd_ndevs = new;
+ cd->cd_devs = nsp;
+ if (old != 0)
+ kmem_free(osp, sizeof(device_t[old]));
}
- cd->cd_ndevs = new;
- cd->cd_devs = nsp;
-out:
- mutex_enter(&alldevs_mtx);
alldevs_nwrite--;
}
+/*
+ * Put dev into the devices list.
+ */
static void
config_devlink(device_t dev)
{
- struct cfdriver *cd = dev->dv_cfdriver;
int s;
- /* put this device in the devices array */
s = config_alldevs_lock();
- config_makeroom(dev->dv_unit, cd);
- if (cd->cd_devs[dev->dv_unit])
- panic("config_attach: duplicate %s", device_xname(dev));
- cd->cd_devs[dev->dv_unit] = dev;
-
+
+ KASSERT(device_cfdriver(dev)->cd_devs[dev->dv_unit] == dev);
+
+ dev->dv_add_gen = alldevs_gen;
/* It is safe to add a device to the tail of the list while
* readers and writers are in the list.
*/
- dev->dv_add_gen = alldevs_gen;
- TAILQ_INSERT_TAIL(&alldevs, dev, dv_list); /* link up */
+ TAILQ_INSERT_TAIL(&alldevs, dev, dv_list);
config_alldevs_unlock(s);
}
+static void
+config_devfree(device_t dev)
+{
+ int priv = (dev->dv_flags & DVF_PRIV_ALLOC);
+
+ if (dev->dv_cfattach->ca_devsize > 0)
+ kmem_free(dev->dv_private, dev->dv_cfattach->ca_devsize);
+ if (priv)
+ kmem_free(dev, sizeof(*dev));
+}
+
/*
- * Caller must hold alldevs_mtx. config_devdelete() may release and
- * re-acquire alldevs_mtx, so callers should re-check conditions such as
- * alldevs_nwrite == 0 && alldevs_nread == 0 after config_devdelete()
- * returns.
+ * Caller must hold alldevs_mtx.
*/
static void
-config_devdelete(device_t dev)
+config_devunlink(device_t dev, struct devicelist *garbage)
{
- device_lock_t dvl = device_getlock(dev);
- int priv = (dev->dv_flags & DVF_PRIV_ALLOC);
- struct cfdriver *cd = dev->dv_cfdriver;
- device_t *devs = NULL;
- int i, ndevs = 0;
+ struct device_garbage *dg = &dev->dv_garbage;
+ cfdriver_t cd = device_cfdriver(dev);
+ int i;
KASSERT(mutex_owned(&alldevs_mtx));
- /* Unlink from device list. */
+ /* Unlink from device list. Link to garbage list. */
TAILQ_REMOVE(&alldevs, dev, dv_list);
+ TAILQ_INSERT_TAIL(garbage, dev, dv_list);
/* Remove from cfdriver's array. */
cd->cd_devs[dev->dv_unit] = NULL;
/*
- * If the device now has no units in use, deallocate its softc array.
+ * If the device now has no units in use, unlink its softc array.
*/
for (i = 0; i < cd->cd_ndevs; i++) {
if (cd->cd_devs[i] != NULL)
break;
}
- /* nothing found. unlink, now. deallocate below. */
+ /* Nothing found. Unlink, now. Deallocate, later. */
if (i == cd->cd_ndevs) {
- ndevs = cd->cd_ndevs;
- devs = cd->cd_devs;
+ dg->dg_ndevs = cd->cd_ndevs;
+ dg->dg_devs = cd->cd_devs;
cd->cd_devs = NULL;
cd->cd_ndevs = 0;
}
-
- mutex_exit(&alldevs_mtx);
-
- KASSERT(!mutex_owned(&alldevs_mtx));
-
- if (devs != NULL)
- kmem_free(devs, sizeof(device_t [ndevs]));
+}
+
+static void
+config_devdelete(device_t dev)
+{
+ struct device_garbage *dg = &dev->dv_garbage;
+ device_lock_t dvl = device_getlock(dev);
+
+ if (dg->dg_devs != NULL)
+ kmem_free(dg->dg_devs, sizeof(device_t[dg->dg_ndevs]));
cv_destroy(&dvl->dvl_cv);
mutex_destroy(&dvl->dvl_mtx);
@@ -1065,21 +1089,61 @@
kmem_free(dev->dv_locators, amount);
}
- if (dev->dv_cfattach->ca_devsize > 0)
- kmem_free(dev->dv_private, dev->dv_cfattach->ca_devsize);
- if (priv)
- kmem_free(dev, sizeof(*dev));
-
- KASSERT(!mutex_owned(&alldevs_mtx));
-
- mutex_enter(&alldevs_mtx);
+ config_devfree(dev);
+}
+
+static int
+config_unit_nextfree(cfdriver_t cd, cfdata_t cf)
+{
+ int unit;
+
+ if (cf->cf_fstate == FSTATE_STAR) {
+ for (unit = cf->cf_unit; unit < cd->cd_ndevs; unit++)
+ if (cd->cd_devs[unit] == NULL)
+ break;
+ /*
+ * unit is now the unit of the first NULL device pointer,
+ * or max(cd->cd_ndevs,cf->cf_unit).
+ */
+ } else {
+ unit = cf->cf_unit;
+ if (unit < cd->cd_ndevs && cd->cd_devs[unit] != NULL)
+ unit = -1;
+ }
+ return unit;
+}
+
+static int
Home |
Main Index |
Thread Index |
Old Index