From owner-svn-src-all@FreeBSD.ORG Sat Dec 3 13:25:25 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A5BBC106566C for ; Sat, 3 Dec 2011 13:25:25 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id 1265C8FC12 for ; Sat, 3 Dec 2011 13:25:24 +0000 (UTC) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.4/8.14.4/ALCHEMY.FRANKEN.DE) with ESMTP id pB3DPN4J080129; Sat, 3 Dec 2011 14:25:23 +0100 (CET) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.4/8.14.4/Submit) id pB3DPNBi080128; Sat, 3 Dec 2011 14:25:23 +0100 (CET) (envelope-from marius) Date: Sat, 3 Dec 2011 14:25:23 +0100 From: Marius Strobl To: Ivan Klymenko Message-ID: <20111203132522.GA54475@alchemy.franken.de> References: <201112022119.pB2LJEqJ009294@svn.freebsd.org> <20111203125551.156724e1@nonamehost.> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20111203125551.156724e1@nonamehost.> User-Agent: Mutt/1.4.2.3i Cc: svn-src-all@freebsd.org Subject: Re: svn commit: r228209 - head/sys/kern X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 03 Dec 2011 13:25:25 -0000 On Sat, Dec 03, 2011 at 12:55:51PM +0200, Ivan Klymenko wrote: > ?? Fri, 2 Dec 2011 21:19:14 +0000 (UTC) > Marius Strobl ??????????: > > > 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