| Message ID | 20210615133519.754763-3-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/10] driver core: Pull required checks into driver_probe_device() | expand |
On Tue, Jun 15 2021, Christoph Hellwig <hch@lst.de> wrote: > really_probe tries to special case errors from ->probe, but due to all > other initialization added to the function over time now a lot of > internal errors hit that code path as well. Untangle that by adding > a new probe_err local variable and apply the special casing only to > that. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/base/dd.c | 72 +++++++++++++++++++++++++++-------------------- > 1 file changed, 42 insertions(+), 30 deletions(-) > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index 7477d3322b3a..fd83817240e6 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -513,12 +513,44 @@ static ssize_t state_synced_show(struct device *dev, > } > static DEVICE_ATTR_RO(state_synced); > > + > +static int call_driver_probe(struct device *dev, struct device_driver *drv) > +{ > + int ret = 0; > + > + if (dev->bus->probe) > + ret = dev->bus->probe(dev); > + else if (drv->probe) > + ret = drv->probe(dev); > + > + switch (ret) { > + case 0: > + break; > + case -EPROBE_DEFER: > + /* Driver requested deferred probing */ > + dev_dbg(dev, "Driver %s requests probe deferral\n", drv->name); > + break; > + case -ENODEV: > + case -ENXIO: > + pr_debug("%s: probe of %s rejects match %d\n", > + drv->name, dev_name(dev), ret); > + break; > + default: > + /* driver matched but the probe failed */ > + pr_warn("%s: probe of %s failed with error %d\n", > + drv->name, dev_name(dev), ret); Convert these two pr_* to dev_* when touching the code anyway? > + break; > + } > + > + return ret; > +} (...) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On Tue, Jun 15, 2021 at 03:53:46PM +0200, Cornelia Huck wrote: > On Tue, Jun 15 2021, Christoph Hellwig <hch@lst.de> wrote: > > > really_probe tries to special case errors from ->probe, but due to all > > other initialization added to the function over time now a lot of > > internal errors hit that code path as well. Untangle that by adding > > a new probe_err local variable and apply the special casing only to > > that. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > --- > > drivers/base/dd.c | 72 +++++++++++++++++++++++++++-------------------- > > 1 file changed, 42 insertions(+), 30 deletions(-) > > > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > > index 7477d3322b3a..fd83817240e6 100644 > > --- a/drivers/base/dd.c > > +++ b/drivers/base/dd.c > > @@ -513,12 +513,44 @@ static ssize_t state_synced_show(struct device *dev, > > } > > static DEVICE_ATTR_RO(state_synced); > > > > + > > +static int call_driver_probe(struct device *dev, struct device_driver *drv) > > +{ > > + int ret = 0; > > + > > + if (dev->bus->probe) > > + ret = dev->bus->probe(dev); > > + else if (drv->probe) > > + ret = drv->probe(dev); > > + > > + switch (ret) { > > + case 0: > > + break; > > + case -EPROBE_DEFER: > > + /* Driver requested deferred probing */ > > + dev_dbg(dev, "Driver %s requests probe deferral\n", drv->name); > > + break; > > + case -ENODEV: > > + case -ENXIO: > > + pr_debug("%s: probe of %s rejects match %d\n", > > + drv->name, dev_name(dev), ret); > > + break; > > + default: > > + /* driver matched but the probe failed */ > > + pr_warn("%s: probe of %s failed with error %d\n", > > + drv->name, dev_name(dev), ret); > > Convert these two pr_* to dev_* when touching the code anyway? It can wait, it's just moving code around for now. thanks, greg k-h
On Tue, Jun 15, 2021 at 03:35:11PM +0200, Christoph Hellwig wrote: > really_probe tries to special case errors from ->probe, but due to all > other initialization added to the function over time now a lot of > internal errors hit that code path as well. Untangle that by adding > a new probe_err local variable and apply the special casing only to > that. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On 6/15/2021 7:05 PM, Christoph Hellwig wrote: > really_probe tries to special case errors from ->probe, but due to all > other initialization added to the function over time now a lot of > internal errors hit that code path as well. Untangle that by adding > a new probe_err local variable and apply the special casing only to > that. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 7477d3322b3a..fd83817240e6 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -513,12 +513,44 @@ static ssize_t state_synced_show(struct device *dev, } static DEVICE_ATTR_RO(state_synced); + +static int call_driver_probe(struct device *dev, struct device_driver *drv) +{ + int ret = 0; + + if (dev->bus->probe) + ret = dev->bus->probe(dev); + else if (drv->probe) + ret = drv->probe(dev); + + switch (ret) { + case 0: + break; + case -EPROBE_DEFER: + /* Driver requested deferred probing */ + dev_dbg(dev, "Driver %s requests probe deferral\n", drv->name); + break; + case -ENODEV: + case -ENXIO: + pr_debug("%s: probe of %s rejects match %d\n", + drv->name, dev_name(dev), ret); + break; + default: + /* driver matched but the probe failed */ + pr_warn("%s: probe of %s failed with error %d\n", + drv->name, dev_name(dev), ret); + break; + } + + return ret; +} + static int really_probe(struct device *dev, struct device_driver *drv) { - int ret = -EPROBE_DEFER; int local_trigger_count = atomic_read(&deferred_trigger_count); bool test_remove = IS_ENABLED(CONFIG_DEBUG_TEST_DRIVER_REMOVE) && !drv->suppress_bind_attrs; + int ret = -EPROBE_DEFER, probe_ret = 0; if (defer_all_probes) { /* @@ -572,14 +604,14 @@ static int really_probe(struct device *dev, struct device_driver *drv) goto probe_failed; } - if (dev->bus->probe) { - ret = dev->bus->probe(dev); - if (ret) - goto probe_failed; - } else if (drv->probe) { - ret = drv->probe(dev); - if (ret) - goto probe_failed; + probe_ret = call_driver_probe(dev, drv); + if (probe_ret) { + /* + * Ignore errors returned by ->probe so that the next driver can + * try its luck. + */ + ret = 0; + goto probe_failed; } if (device_add_groups(dev, drv->dev_groups)) { @@ -650,28 +682,8 @@ static int really_probe(struct device *dev, struct device_driver *drv) dev->pm_domain->dismiss(dev); pm_runtime_reinit(dev); dev_pm_set_driver_flags(dev, 0); - - switch (ret) { - case -EPROBE_DEFER: - /* Driver requested deferred probing */ - dev_dbg(dev, "Driver %s requests probe deferral\n", drv->name); + if (probe_ret == -EPROBE_DEFER) driver_deferred_probe_add_trigger(dev, local_trigger_count); - break; - case -ENODEV: - case -ENXIO: - pr_debug("%s: probe of %s rejects match %d\n", - drv->name, dev_name(dev), ret); - break; - default: - /* driver matched but the probe failed */ - pr_warn("%s: probe of %s failed with error %d\n", - drv->name, dev_name(dev), ret); - } - /* - * Ignore errors returned by ->probe so that the next driver can try - * its luck. - */ - ret = 0; done: atomic_dec(&probe_count); wake_up_all(&probe_waitqueue);
really_probe tries to special case errors from ->probe, but due to all other initialization added to the function over time now a lot of internal errors hit that code path as well. Untangle that by adding a new probe_err local variable and apply the special casing only to that. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/base/dd.c | 72 +++++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 30 deletions(-)