On Fri, 2015-11-27 at 00:09 +0100, Rafael J. Wysocki wrote: > On Thursday, November 26, 2015 05:19:07 PM Andy Shevchenko wrote: > > In case ->probe() fails the notifier does not inform a subscriber > > about this. > > In the result it might happend that some resources that had been > > allocated will > > stay allocated and therefore lead to resource leak. > > > > Introduce a new notification to inform the subscriber that > > ->probe() failed. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > > I'd rather say the problem is that the users of > BUS_NOTIFY_BIND_DRIVER have no > chance to do any cleanup in case of a probe failure (there may be > problems even > if resources aren't leaked). Thanks, Rafael, all of your comments sound reasonable for me. Will be taken into consideration in next version. > > > --- > > drivers/base/dd.c | 8 ++++++-- > > include/linux/device.h | 1 + > > 2 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > > index a641cf3..ac071a5 100644 > > --- a/drivers/base/dd.c > > +++ b/drivers/base/dd.c > > @@ -290,7 +290,7 @@ static int really_probe(struct device *dev, > > struct device_driver *drv) > > /* If using pinctrl, bind pins now before probing */ > > ret = pinctrl_bind_pins(dev); > > if (ret) > > - goto probe_failed; > > + goto pinctrl_bind_failed; > > > > if (driver_sysfs_add(dev)) { > > printk(KERN_ERR "%s: driver_sysfs_add(%s) > > failed\n", > > @@ -334,6 +334,11 @@ static int really_probe(struct device *dev, > > struct device_driver *drv) > > goto done; > > > > probe_failed: > > + if (dev->bus) > > + blocking_notifier_call_chain(&dev->bus->p- > > >bus_notifier, > > + BUS_NOTIFY_BIND_DRIVE > > R_ERROR, > > + dev); > > Well, if we do that, device_bind_driver() needs to send that > notification too > in case it doesn't call driver_bound(). > > > +pinctrl_bind_failed: > > devres_release_all(dev); > > driver_sysfs_remove(dev); > > dev->driver = NULL; > > @@ -701,7 +706,6 @@ static void __device_release_driver(struct > > device *dev) > > blocking_notifier_call_chain(&dev->bus->p- > > >bus_notifier, > > BUS_NOTIFY_UN > > BOUND_DRIVER, > > dev); > > - > > } > > } > > > > diff --git a/include/linux/device.h b/include/linux/device.h > > index b8f411b..87cf423 100644 > > --- a/include/linux/device.h > > +++ b/include/linux/device.h > > @@ -191,6 +191,7 @@ extern int bus_unregister_notifier(struct > > bus_type *bus, > > unbound */ > > #define BUS_NOTIFY_UNBOUND_DRIVER 0x00000007 /* driver is > > unbound > > from the > > device */ > > +#define BUS_NOTIFY_BIND_DRIVER_ERROR 0x80000004 /* driver > > fails to be bound */ > > I'd call it BUS_NOTIFY_DRIVER_NOT_BOUND. > > > > > extern struct kset *bus_get_kset(struct bus_type *bus); > > extern struct klist *bus_get_device_klist(struct bus_type *bus); > > > > Thanks, > Rafael > -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html