Date: Sat, 3 Dec 2011 14:25:23 +0100 From: Marius Strobl <marius@alchemy.franken.de> To: Ivan Klymenko <fidaj@ukr.net> Cc: svn-src-all@freebsd.org Subject: Re: svn commit: r228209 - head/sys/kern Message-ID: <20111203132522.GA54475@alchemy.franken.de> In-Reply-To: <20111203125551.156724e1@nonamehost.> References: <201112022119.pB2LJEqJ009294@svn.freebsd.org> <20111203125551.156724e1@nonamehost.>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Dec 03, 2011 at 12:55:51PM +0200, Ivan Klymenko wrote: > ?? Fri, 2 Dec 2011 21:19:14 +0000 (UTC) > Marius Strobl <marius@FreeBSD.org> ??????????: > > > Author: marius > > Date: Fri Dec 2 21:19:14 2011 > > New Revision: 228209 > > URL: http://svn.freebsd.org/changeset/base/228209 > > > > Log: > > - In device_probe_child(9) check the return value of > > device_set_driver(9) when actually setting a driver as especially > > ENOMEM is fatal in these cases. > > - Annotate other calls to device_set_devclass(9) and > > device_set_driver(9) without the return value being checked and that > > are okay to fail. > > Reviewed by: yongari (slightly earlier version) > > > > Modified: > > head/sys/kern/subr_bus.c > > > > Modified: head/sys/kern/subr_bus.c > > ============================================================================== > > --- head/sys/kern/subr_bus.c Fri Dec 2 20:28:45 2011 > > (r228208) +++ head/sys/kern/subr_bus.c Fri Dec 2 21:19:14 > > 2011 (r228209) @@ -1129,7 +1129,7 @@ > > devclass_driver_deleted(devclass_t buscl dev->parent->devclass == > > busclass) { if ((error = device_detach(dev)) != 0) > > return (error); > > - device_set_driver(dev, NULL); > > + (void)device_set_driver(dev, NULL); > > BUS_PROBE_NOMATCH(dev->parent, dev); > > devnomatch(dev); > > dev->flags |= DF_DONENOMATCH; > > @@ -2007,19 +2007,22 @@ device_probe_child(device_t dev, device_ > > for (dl = first_matching_driver(dc, child); > > dl; > > dl = next_matching_driver(dc, child, dl)) { > > - > > /* If this driver's pass is too high, then > > ignore it. */ if (dl->pass > bus_current_pass) > > continue; > > > > PDEBUG(("Trying %s", > > DRIVERNAME(dl->driver))); > > - device_set_driver(child, dl->driver); > > + result = device_set_driver(child, > > dl->driver); > > + if (result == ENOMEM) > > + return (result); > > + else if (result != 0) > > + continue; > > if (!hasclass) { > > if (device_set_devclass(child, > > dl->driver->name)) { printf("driver bug: Unable to set devclass > > (devname: %s)\n", (child ? device_get_name(child) : > > "no device")); > > - device_set_driver(child, > > NULL); > > + > > (void)device_set_driver(child, NULL); continue; > > } > > } > > @@ -2033,7 +2036,7 @@ device_probe_child(device_t dev, device_ > > /* Reset flags and devclass before the next > > probe. */ child->devflags = 0; > > if (!hasclass) > > - device_set_devclass(child, NULL); > > + (void)device_set_devclass(child, > > NULL); > > /* > > * If the driver returns SUCCESS, there can > > be @@ -2050,7 +2053,7 @@ device_probe_child(device_t dev, device_ > > * certainly doesn't match. > > */ > > if (result > 0) { > > - device_set_driver(child, NULL); > > + (void)device_set_driver(child, NULL); > > continue; > > } > > > > @@ -2113,7 +2116,9 @@ device_probe_child(device_t dev, device_ > > if (result != 0) > > return (result); > > } > > - device_set_driver(child, best->driver); > > + result = device_set_driver(child, best->driver); > > + if (result != 0) > > + return (result); > > resource_int_value(best->driver->name, child->unit, > > "flags", &child->devflags); > > > > @@ -2722,8 +2727,8 @@ device_attach(device_t dev) > > dev->driver->name, dev->unit, error); > > /* Unset the class; set in device_probe_child */ > > if (dev->devclass == NULL) > > - device_set_devclass(dev, NULL); > > - device_set_driver(dev, NULL); > > + (void)device_set_devclass(dev, NULL); > > + (void)device_set_driver(dev, NULL); > > device_sysctl_fini(dev); > > dev->state = DS_NOTPRESENT; > > return (error); > > @@ -2776,7 +2781,7 @@ device_detach(device_t dev) > > devclass_delete_device(dev->devclass, dev); > > > > dev->state = DS_NOTPRESENT; > > - device_set_driver(dev, NULL); > > + (void)device_set_driver(dev, NULL); > > device_set_desc(dev, NULL); > > device_sysctl_fini(dev); > > > > @@ -4613,7 +4618,6 @@ print_driver(driver_t *driver, int inden > > print_driver_short(driver, indent); > > } > > > > - > > static void > > print_driver_list(driver_list_t drivers, int indent) > > { > > Thank you! > Now I can again connect a USB external drive... In what way did it fail before? This change actually doesn't fix any issue encountered in practice that I'm aware of. I just spotted this possibility for a problem while trying to debug what appears to be a race in newbus and I can't think of anything besides a panic due to trying to use unallocated memory this change would fix. But even then if device_set_driver(9) fails with ENOMEM the panic would be just avoided but you'd still end up with no driver being attached ... > But, nevertheless, remains a problem when you connect an external drive > via FireWire... Marius
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111203132522.GA54475>