On 6/14/2021 8:38 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

drivers/base/dd.c | 72 +++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 31 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 7477d3322b3a..999bc737a8f0 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -513,12 +513,42 @@ 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 -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);
  

There should be case 0, that is, success case before default case as below: + case 0: + /* Driver returned success */ + break;

Otherwise even in case of success, above warning would mislead that probe has failed.

Thanks, Kirti

• 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,15 +602,15 @@ 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)) { dev_err(dev, "device_add_groups() failed\n");

@@ -650,28 +680,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);