Port-i386 archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: panic: config_attach: duplicate uhub3
On Tue, Dec 08, 2009 at 03:01:44AM +0000, Quentin Garnier wrote:
> On Thu, Dec 03, 2009 at 05:23:48PM -0600, David Young wrote:
> > On Sun, Nov 29, 2009 at 02:46:12PM +0100, Jarle Greipsland wrote:
> > > Some console output (10-finger copy):
> > >
> > > ehci0 at pci0 dev 26 function 7: vendor 0x8086 product 0x293c (rev. 0x03)
> > > ehci0: interrupting at ioapic0 pin 23
> > > ehci0: companion controllers, 2 ports each: uhci0 uhci1 uhci2
> > > usb3 at ehci0: USB revision 2.0
> > > [ ... ]
> > > uhub2 at usb2: vendor 0x8086 UHCI root hub, class 9/0, rev 1.00/1.00,
> > > addr 1
> > > uhub3 at usb3: vendor 0x8086 EHCI root hub, class 9/0, rev 2.00/1.00,
> > > addr 1
> > > panic: config_attach: duplicate uhub3
> > >
> > > How can I help fix this problem?
> >
> > Apply this patch to sys/kern/subr_autoconf.c and send the output.
>
> The problem is that the selection of dev->dv_unit and the filling of the
> cd_devs slot doesn't happen atomically. Maybe use a magic non-NULL
> value like -1 to indicate the cd_devs slot is taken but does not yet
> contain an actual value in config_devalloc()?
Attached is a patch that selects dv_unit and fills cd_devs atomically.
Dave
--
David Young OJC Technologies
dyoung%ojctech.com@localhost Urbana, IL * (217) 278-3933
Index: sys/kern/subr_autoconf.c
===================================================================
RCS file: /cvsroot/src/sys/kern/subr_autoconf.c,v
retrieving revision 1.189
diff -p -u -u -p -r1.189 subr_autoconf.c
--- sys/kern/subr_autoconf.c 29 Nov 2009 15:17:30 -0000 1.189
+++ sys/kern/subr_autoconf.c 9 Dec 2009 01:10:10 -0000
@@ -951,52 +951,59 @@ static void
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;
+ for (new = MAX(4, cd->cd_ndevs); new <= n; new += new)
+ ;
- /*
- * 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]));
+ 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--;
}
static void
config_devlink(device_t dev)
{
- struct cfdriver *cd = dev->dv_cfdriver;
+ cfdriver_t cd = device_cfdriver(dev);
int s;
+ KASSERT(cd->cd_devs[dev->dv_unit] == dev);
+
/* 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;
/* It is safe to add a device to the tail of the list while
* readers and writers are in the list.
@@ -1006,6 +1013,17 @@ config_devlink(device_t dev)
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
@@ -1016,8 +1034,7 @@ static void
config_devdelete(device_t dev)
{
device_lock_t dvl = device_getlock(dev);
- int priv = (dev->dv_flags & DVF_PRIV_ALLOC);
- struct cfdriver *cd = dev->dv_cfdriver;
+ cfdriver_t cd = device_cfdriver(dev);
device_t *devs = NULL;
int i, ndevs = 0;
@@ -1065,21 +1082,62 @@ config_devdelete(device_t dev)
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));
+ config_devfree(dev);
KASSERT(!mutex_owned(&alldevs_mtx));
mutex_enter(&alldevs_mtx);
}
+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
+config_unit_alloc(device_t dev, cfdriver_t cd, cfdata_t cf)
+{
+ int s, unit;
+
+ s = config_alldevs_lock();
+ for (;;) {
+ config_collect_garbage();
+ unit = config_unit_nextfree(cd, cf);
+ if (unit == -1)
+ break;
+ if (unit < cd->cd_ndevs) {
+ cd->cd_devs[unit] = dev;
+ dev->dv_unit = unit;
+ break;
+ }
+ config_makeroom(unit, cd);
+ }
+ config_alldevs_unlock(s);
+
+ return unit;
+}
+
static device_t
config_devalloc(const device_t parent, const cfdata_t cf, const int *locs)
{
- struct cfdriver *cd;
- struct cfattach *ca;
+ cfdriver_t cd;
+ cfattach_t ca;
size_t lname, lunit;
const char *xunit;
int myunit;
@@ -1088,9 +1146,6 @@ config_devalloc(const device_t parent, c
void *dev_private;
const struct cfiattrdata *ia;
device_lock_t dvl;
-#ifndef __BROKEN_CONFIG_UNIT_USAGE
- int s;
-#endif
cd = config_cfdriver_lookup(cf->cf_name);
if (cd == NULL)
@@ -1104,36 +1159,6 @@ config_devalloc(const device_t parent, c
ca->ca_devsize < sizeof(struct device))
panic("config_devalloc: %s", cf->cf_atname);
-#ifndef __BROKEN_CONFIG_UNIT_USAGE
- s = config_alldevs_lock();
- config_collect_garbage();
- if (cf->cf_fstate == FSTATE_STAR) {
- for (myunit = cf->cf_unit; myunit < cd->cd_ndevs; myunit++)
- if (cd->cd_devs[myunit] == NULL)
- break;
- /*
- * myunit is now the unit of the first NULL device pointer,
- * or max(cd->cd_ndevs,cf->cf_unit).
- */
- } else {
- myunit = cf->cf_unit;
- if (myunit < cd->cd_ndevs && cd->cd_devs[myunit] != NULL)
- myunit = -1;
- }
- config_alldevs_unlock(s);
- if (myunit == -1)
- return NULL;
-#else
- myunit = cf->cf_unit;
-#endif /* ! __BROKEN_CONFIG_UNIT_USAGE */
-
- /* compute length of name and decimal expansion of unit number */
- lname = strlen(cd->cd_name);
- xunit = number(&num[sizeof(num)], myunit);
- lunit = &num[sizeof(num)] - xunit;
- if (lname + lunit > sizeof(dev->dv_xname))
- panic("config_devalloc: device name too long");
-
/* get memory for all device vars */
KASSERT((ca->ca_flags & DVF_PRIV_ALLOC) || ca->ca_devsize >=
sizeof(struct device));
if (ca->ca_devsize > 0) {
@@ -1153,6 +1178,19 @@ config_devalloc(const device_t parent, c
if (dev == NULL)
panic("config_devalloc: memory allocation for device_t failed");
+ myunit = config_unit_alloc(dev, cd, cf);
+ if (myunit == -1) {
+ config_devfree(dev);
+ return NULL;
+ }
+
+ /* compute length of name and decimal expansion of unit number */
+ lname = strlen(cd->cd_name);
+ xunit = number(&num[sizeof(num)], myunit);
+ lunit = &num[sizeof(num)] - xunit;
+ if (lname + lunit > sizeof(dev->dv_xname))
+ panic("config_devalloc: device name too long");
+
dvl = device_getlock(dev);
mutex_init(&dvl->dvl_mtx, MUTEX_DEFAULT, IPL_NONE);
@@ -1162,7 +1200,6 @@ config_devalloc(const device_t parent, c
dev->dv_cfdata = cf;
dev->dv_cfdriver = cd;
dev->dv_cfattach = ca;
- dev->dv_unit = myunit;
dev->dv_activity_count = 0;
dev->dv_activity_handlers = NULL;
dev->dv_private = dev_private;
@@ -1220,10 +1257,6 @@ config_attach_loc(device_t parent, cfdat
KASSERT(cf->cf_fstate == FSTATE_NOTFOUND);
cf->cf_fstate = FSTATE_FOUND;
}
-#ifdef __BROKEN_CONFIG_UNIT_USAGE
- else
- cf->cf_unit++;
-#endif
config_devlink(dev);
@@ -1257,14 +1290,6 @@ config_attach_loc(device_t parent, cfdat
cf->cf_unit == dev->dv_unit) {
if (cf->cf_fstate == FSTATE_NOTFOUND)
cf->cf_fstate = FSTATE_FOUND;
-#ifdef __BROKEN_CONFIG_UNIT_USAGE
- /*
- * Bump the unit number on all starred cfdata
- * entries for this device.
- */
- if (cf->cf_fstate == FSTATE_STAR)
- cf->cf_unit++;
-#endif /* __BROKEN_CONFIG_UNIT_USAGE */
}
}
}
@@ -1474,16 +1499,6 @@ config_detach(device_t dev, int flags)
if (cf->cf_fstate == FSTATE_FOUND &&
cf->cf_unit == dev->dv_unit)
cf->cf_fstate = FSTATE_NOTFOUND;
-#ifdef __BROKEN_CONFIG_UNIT_USAGE
- /*
- * Note that we can only re-use a starred
- * unit number if the unit being detached
- * had the last assigned unit number.
- */
- if (cf->cf_fstate == FSTATE_STAR &&
- cf->cf_unit == dev->dv_unit + 1)
- cf->cf_unit--;
-#endif /* __BROKEN_CONFIG_UNIT_USAGE */
}
}
}
Home |
Main Index |
Thread Index |
Old Index