[PATCH] driver-core: Log the BUG() causing driver

[PATCH] driver-core: Log the BUG() causing driver

[Florian Schmaus]

I triggerd the BUG_ON(), which was added in
f48f3febb2cbfd0f2ecee7690835ba745c1034a4, when booting a domU Xen
domain. Since there was no contextual information logged, I needed to
attach kgdb to determine the culprit (the wmi-bmof driver in my case).

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---
 drivers/base/driver.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index ba912558a510..55f8db41df2b 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv)
 	int ret;
 	struct device_driver *other;
 
-	BUG_ON(!drv->bus->p);
+	if (!drv->bus->p) {
+		printk(KERN_ERR "Driver '%s' was unable to register bus_type\n",
+			   drv->name);
+		BUG();
+	}
 
 	if ((drv->bus->probe && drv->probe) ||
 	    (drv->bus->remove && drv->remove) ||
-- 
2.13.6

Re: [PATCH] driver-core: Log the BUG() causing driver

[Greg Kroah-Hartman]

On Wed, Mar 07, 2018 at 10:49:02AM +0100, Florian Schmaus wrote:
> I triggerd the BUG_ON(), which was added in
> f48f3febb2cbfd0f2ecee7690835ba745c1034a4, when booting a domU Xen
> domain.

How?

> Since there was no contextual information logged, I needed to
> attach kgdb to determine the culprit (the wmi-bmof driver in my case).
> 
> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> ---
>  drivers/base/driver.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index ba912558a510..55f8db41df2b 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv)
>  	int ret;
>  	struct device_driver *other;
>  
> -	BUG_ON(!drv->bus->p);
> +	if (!drv->bus->p) {
> +		printk(KERN_ERR "Driver '%s' was unable to register bus_type\n",
> +			   drv->name);
> +		BUG();
> +	}

We could just log the error and return with an error, that would be
nicer, right?  Crashing the system isn't usually a good idea, even at
boot time :)

thanks,

greg k-h

Re: [PATCH] driver-core: Log the BUG() causing driver

[Florian Schmaus]

[-- Attachment #1.1: Type: text/plain, Size: 1971 bytes --]

On 07.03.2018 16:13, Greg Kroah-Hartman wrote:
> On Wed, Mar 07, 2018 at 10:49:02AM +0100, Florian Schmaus wrote:
>> I triggerd the BUG_ON(), which was added in
>> f48f3febb2cbfd0f2ecee7690835ba745c1034a4, when booting a domU Xen
>> domain.
> 
> How?

Basically just by booting the domU kernel with the wmi-bmof driver
compiled in. After I deselected the wmi-bmof driver, the kernel booted
just fine.

I'd probably be able to re-create the faulty kernel and show you the
image and/or kernel config if you want.

>> Since there was no contextual information logged, I needed to
>> attach kgdb to determine the culprit (the wmi-bmof driver in my case).
>>
>> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
>> ---
>>  drivers/base/driver.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
>> index ba912558a510..55f8db41df2b 100644
>> --- a/drivers/base/driver.c
>> +++ b/drivers/base/driver.c
>> @@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv)
>>  	int ret;
>>  	struct device_driver *other;
>>  
>> -	BUG_ON(!drv->bus->p);
>> +	if (!drv->bus->p) {
>> +		printk(KERN_ERR "Driver '%s' was unable to register bus_type\n",
>> +			   drv->name);
>> +		BUG();
>> +	}
> 
> We could just log the error and return with an error, that would be
> nicer, right?  Crashing the system isn't usually a good idea, even at
> boot time :)

It sure is, and I did consider it. I just wasn't sure which return value
to use in that case. Probably the best solution would be to use the
return value of bus_register(), which caused p to become NULL. Although
that possibly requires extending the bus_type struct by an 'int' storing
the return value (Or could the return value be mangled into the 'p'
pointer?).

I'd be happy to hear your thoughts on this.

BTW: Should I retain the 'unlikely' semantic of BUG_ON() in this case?

- Florian


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 642 bytes --]

[PATCH v2] driver-core: Return EBUSY error instead of BUG_ON()

[Florian Schmaus]

I triggerd the BUG_ON() in driver_register(), which was added in
f48f3febb2cbfd0f2ecee7690835ba745c1034a4, when booting a domU Xen
domain. Since there was no contextual information logged, I needed to
attach kgdb to determine the culprit (the wmi-bmof driver in my case).

Instead of running into a BUG_ON() we print an error message
identifying the driver but continue booting.

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---
 drivers/base/driver.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index ba912558a510..63baec586eba 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv)
 	int ret;
 	struct device_driver *other;
 
-	BUG_ON(!drv->bus->p);
+	if (!drv->bus->p) {
+		printk(KERN_ERR "Driver '%s' was unable to register bus_type\n",
+			   drv->name);
+		return -EBUSY;
+	}
 
 	if ((drv->bus->probe && drv->probe) ||
 	    (drv->bus->remove && drv->remove) ||
-- 
2.16.1

Re: [PATCH v2] driver-core: Return EBUSY error instead of BUG_ON()

[Greg Kroah-Hartman]

On Fri, May 04, 2018 at 03:23:57PM +0200, Florian Schmaus wrote:
> I triggerd the BUG_ON() in driver_register(), which was added in
> f48f3febb2cbfd0f2ecee7690835ba745c1034a4, when booting a domU Xen
> domain. Since there was no contextual information logged, I needed to
> attach kgdb to determine the culprit (the wmi-bmof driver in my case).
> 
> Instead of running into a BUG_ON() we print an error message
> identifying the driver but continue booting.
> 
> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> ---
>  drivers/base/driver.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 

What changed from v1?  Always put that below the --- line.

Also, it's best to quote commits with the following format:
	f48f3febb2cb ("driver-core: do not register a driver with bus_type not registered")

v3?

thanks,

greg k-h

Re: [PATCH v2] driver-core: Return EBUSY error instead of BUG_ON()

[Greg Kroah-Hartman]

On Fri, May 04, 2018 at 03:23:57PM +0200, Florian Schmaus wrote:
> I triggerd the BUG_ON() in driver_register(), which was added in
> f48f3febb2cbfd0f2ecee7690835ba745c1034a4, when booting a domU Xen
> domain. Since there was no contextual information logged, I needed to
> attach kgdb to determine the culprit (the wmi-bmof driver in my case).
> 
> Instead of running into a BUG_ON() we print an error message
> identifying the driver but continue booting.
> 
> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> ---
>  drivers/base/driver.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index ba912558a510..63baec586eba 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv)
>  	int ret;
>  	struct device_driver *other;
>  
> -	BUG_ON(!drv->bus->p);
> +	if (!drv->bus->p) {
> +		printk(KERN_ERR "Driver '%s' was unable to register bus_type\n",
> +			   drv->name);
> +		return -EBUSY;
> +	}

In looking at this code, that's a real bug somewhere in the logic of the
bus subsystem.  BUG_ON() is a pretty big hammer, I agree, but your error
message should be a lot more descriptive, and the error value should not
be BUSY, as something went wrong, it's not just a "come back later" type
of thing.

thanks,

greg k-h

[PATCH v3 0/3] return EINVAL error instead of BUG_ON()

[Florian Schmaus]

This patch series makes driver_register() emit an error message and
return a failure code instead of triggering a BUG_ON()

I tried to make the error message more descriptive compared to v2 by
including the error of the failed bus registration in [3/3]. I'd be
happy to include some more context if you have further suggestions.


Florian Schmaus (3):
  driver-core: return EINVAL error instead of BUG_ON()
  driver-core: record error on bus registration
  driver-core: print bus registration error value

 drivers/base/bus.c     | 2 ++
 drivers/base/driver.c  | 7 ++++++-
 include/linux/device.h | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.16.1

[PATCH v3 1/3] driver-core: return EINVAL error instead of BUG_ON()

[Florian Schmaus]

I triggerd the BUG_ON() in driver_register() when booting a domU Xen
domain. Since there was no contextual information logged, I needed to
attach kgdb to determine the culprit (the wmi-bmof driver in my
case). The BUG_ON() was added in commit f48f3febb2cb ("driver-core: do
not register a driver with bus_type not registered").

Instead of running into a BUG_ON() we print an error message
identifying the, likely faulty, driver but continue booting.

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---

Notes:
    - return EINVAL (instead of EBUSY)
    - follow common pattern when quoting commits in commit messages

 drivers/base/driver.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index ba912558a510..afd5b08b7677 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv)
 	int ret;
 	struct device_driver *other;
 
-	BUG_ON(!drv->bus->p);
+	if (!drv->bus->p) {
+		printk(KERN_ERR "Driver '%s' was unable to register bus_type\n",
+			   drv->name);
+		return -EINVAL;
+	}
 
 	if ((drv->bus->probe && drv->probe) ||
 	    (drv->bus->remove && drv->remove) ||
-- 
2.16.1

[PATCH v3 2/3] driver-core: record error on bus registration

[Florian Schmaus]

If bus_register() fails on a driver then record the error code so that
it can be inspected later on.

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---
 drivers/base/bus.c     | 2 ++
 include/linux/device.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index ef6183306b40..dce677dddba4 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -152,6 +152,7 @@ static void bus_release(struct kobject *kobj)
 
 	kfree(priv);
 	bus->p = NULL;
+	bus->bus_register_error = 0;
 }
 
 static struct kobj_type bus_ktype = {
@@ -915,6 +916,7 @@ int bus_register(struct bus_type *bus)
 out:
 	kfree(bus->p);
 	bus->p = NULL;
+	bus->bus_register_error = retval;
 	return retval;
 }
 EXPORT_SYMBOL_GPL(bus_register);
diff --git a/include/linux/device.h b/include/linux/device.h
index 0059b99e1f25..5b1f3c08bebe 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -135,6 +135,7 @@ struct bus_type {
 	const struct iommu_ops *iommu_ops;
 
 	struct subsys_private *p;
+	int bus_register_error;
 	struct lock_class_key lock_key;
 
 	bool force_dma;
-- 
2.16.1

[PATCH v3 3/3] driver-core: print bus registration error value

[Florian Schmaus]

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---
 drivers/base/driver.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index afd5b08b7677..c68d35139c0f 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -149,8 +149,9 @@ int driver_register(struct device_driver *drv)
 	struct device_driver *other;
 
 	if (!drv->bus->p) {
-		printk(KERN_ERR "Driver '%s' was unable to register bus_type\n",
-			   drv->name);
+		printk(KERN_ERR "Driver '%s' was unable to register bus_type "
+			   "(error: %d)\n",
+			   drv->name, drv->bus->bus_register_retval);
 		return -EINVAL;
 	}
 
-- 
2.16.1

Re: [PATCH v3 1/3] driver-core: return EINVAL error instead of BUG_ON()

[Greg Kroah-Hartman]

On Tue, May 15, 2018 at 03:51:12PM +0200, Florian Schmaus wrote:
> I triggerd the BUG_ON() in driver_register() when booting a domU Xen
> domain. Since there was no contextual information logged, I needed to
> attach kgdb to determine the culprit (the wmi-bmof driver in my
> case). The BUG_ON() was added in commit f48f3febb2cb ("driver-core: do
> not register a driver with bus_type not registered").
> 
> Instead of running into a BUG_ON() we print an error message
> identifying the, likely faulty, driver but continue booting.
> 
> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> ---
> 
> Notes:
>     - return EINVAL (instead of EBUSY)
>     - follow common pattern when quoting commits in commit messages
> 
>  drivers/base/driver.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index ba912558a510..afd5b08b7677 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv)
>  	int ret;
>  	struct device_driver *other;
>  
> -	BUG_ON(!drv->bus->p);
> +	if (!drv->bus->p) {
> +		printk(KERN_ERR "Driver '%s' was unable to register bus_type\n",
> +			   drv->name);

checkpatch.pl should have said to use pr_err(), right?  :)

Also, why not print out the driver name as well?

thanks,

greg k-h

Re: [PATCH v3 3/3] driver-core: print bus registration error value

[Greg Kroah-Hartman]

On Tue, May 15, 2018 at 03:51:14PM +0200, Florian Schmaus wrote:
> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> ---
>  drivers/base/driver.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index afd5b08b7677..c68d35139c0f 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -149,8 +149,9 @@ int driver_register(struct device_driver *drv)
>  	struct device_driver *other;
>  
>  	if (!drv->bus->p) {
> -		printk(KERN_ERR "Driver '%s' was unable to register bus_type\n",
> -			   drv->name);
> +		printk(KERN_ERR "Driver '%s' was unable to register bus_type "
> +			   "(error: %d)\n",
> +			   drv->name, drv->bus->bus_register_retval);

I don't understand, if a bus was never registered, this is going to fail
in lots of odd ways, including the value being 0, so that would show "no
error"?  Is this really needed?

A better message would be something like:
	"Driver '%s" was unable to register with bus_type '%s' because it was not initialized."

right?

thanks,

greg k-h

Re: [PATCH v3 1/3] driver-core: return EINVAL error instead of BUG_ON()

[Greg Kroah-Hartman]

On Tue, May 15, 2018 at 05:10:35PM +0200, Greg Kroah-Hartman wrote:
> On Tue, May 15, 2018 at 03:51:12PM +0200, Florian Schmaus wrote:
> > I triggerd the BUG_ON() in driver_register() when booting a domU Xen
> > domain. Since there was no contextual information logged, I needed to
> > attach kgdb to determine the culprit (the wmi-bmof driver in my
> > case). The BUG_ON() was added in commit f48f3febb2cb ("driver-core: do
> > not register a driver with bus_type not registered").
> > 
> > Instead of running into a BUG_ON() we print an error message
> > identifying the, likely faulty, driver but continue booting.
> > 
> > Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> > ---
> > 
> > Notes:
> >     - return EINVAL (instead of EBUSY)
> >     - follow common pattern when quoting commits in commit messages
> > 
> >  drivers/base/driver.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> > index ba912558a510..afd5b08b7677 100644
> > --- a/drivers/base/driver.c
> > +++ b/drivers/base/driver.c
> > @@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv)
> >  	int ret;
> >  	struct device_driver *other;
> >  
> > -	BUG_ON(!drv->bus->p);
> > +	if (!drv->bus->p) {
> > +		printk(KERN_ERR "Driver '%s' was unable to register bus_type\n",
> > +			   drv->name);
> 
> checkpatch.pl should have said to use pr_err(), right?  :)
> 
> Also, why not print out the driver name as well?

Sorry, I meant the bus name.

Re: [PATCH v3 3/3] driver-core: print bus registration error value

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2629 bytes --]

Hi Florian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on driver-core/driver-core-testing]
[also build test ERROR on v4.17-rc5 next-20180515]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Florian-Schmaus/return-EINVAL-error-instead-of-BUG_ON/20180516-141149
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/base/driver.c: In function 'driver_register':
>> drivers/base/driver.c:154:28: error: 'struct bus_type' has no member named 'bus_register_retval'; did you mean 'bus_register_error'?
          drv->name, drv->bus->bus_register_retval);
                               ^~~~~~~~~~~~~~~~~~~
                               bus_register_error

vim +154 drivers/base/driver.c

   137	
   138	/**
   139	 * driver_register - register driver with bus
   140	 * @drv: driver to register
   141	 *
   142	 * We pass off most of the work to the bus_add_driver() call,
   143	 * since most of the things we have to do deal with the bus
   144	 * structures.
   145	 */
   146	int driver_register(struct device_driver *drv)
   147	{
   148		int ret;
   149		struct device_driver *other;
   150	
   151		if (!drv->bus->p) {
   152			printk(KERN_ERR "Driver '%s' was unable to register bus_type "
   153				   "(error: %d)\n",
 > 154				   drv->name, drv->bus->bus_register_retval);
   155			return -EINVAL;
   156		}
   157	
   158		if ((drv->bus->probe && drv->probe) ||
   159		    (drv->bus->remove && drv->remove) ||
   160		    (drv->bus->shutdown && drv->shutdown))
   161			printk(KERN_WARNING "Driver '%s' needs updating - please use "
   162				"bus_type methods\n", drv->name);
   163	
   164		other = driver_find(drv->name, drv->bus);
   165		if (other) {
   166			printk(KERN_ERR "Error: Driver '%s' is already registered, "
   167				"aborting...\n", drv->name);
   168			return -EBUSY;
   169		}
   170	
   171		ret = bus_add_driver(drv);
   172		if (ret)
   173			return ret;
   174		ret = driver_add_groups(drv, drv->groups);
   175		if (ret) {
   176			bus_remove_driver(drv);
   177			return ret;
   178		}
   179		kobject_uevent(&drv->p->kobj, KOBJ_ADD);
   180	
   181		return ret;
   182	}
   183	EXPORT_SYMBOL_GPL(driver_register);
   184	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6309 bytes --]

Re: [PATCH v3 3/3] driver-core: print bus registration error value

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2534 bytes --]

Hi Florian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on driver-core/driver-core-testing]
[also build test ERROR on v4.17-rc5 next-20180515]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Florian-Schmaus/return-EINVAL-error-instead-of-BUG_ON/20180516-141149
config: i386-randconfig-a0-201819 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/base/driver.c: In function 'driver_register':
>> drivers/base/driver.c:154:26: error: 'struct bus_type' has no member named 'bus_register_retval'
          drv->name, drv->bus->bus_register_retval);
                             ^

vim +154 drivers/base/driver.c

   137	
   138	/**
   139	 * driver_register - register driver with bus
   140	 * @drv: driver to register
   141	 *
   142	 * We pass off most of the work to the bus_add_driver() call,
   143	 * since most of the things we have to do deal with the bus
   144	 * structures.
   145	 */
   146	int driver_register(struct device_driver *drv)
   147	{
   148		int ret;
   149		struct device_driver *other;
   150	
   151		if (!drv->bus->p) {
   152			printk(KERN_ERR "Driver '%s' was unable to register bus_type "
   153				   "(error: %d)\n",
 > 154				   drv->name, drv->bus->bus_register_retval);
   155			return -EINVAL;
   156		}
   157	
   158		if ((drv->bus->probe && drv->probe) ||
   159		    (drv->bus->remove && drv->remove) ||
   160		    (drv->bus->shutdown && drv->shutdown))
   161			printk(KERN_WARNING "Driver '%s' needs updating - please use "
   162				"bus_type methods\n", drv->name);
   163	
   164		other = driver_find(drv->name, drv->bus);
   165		if (other) {
   166			printk(KERN_ERR "Error: Driver '%s' is already registered, "
   167				"aborting...\n", drv->name);
   168			return -EBUSY;
   169		}
   170	
   171		ret = bus_add_driver(drv);
   172		if (ret)
   173			return ret;
   174		ret = driver_add_groups(drv, drv->groups);
   175		if (ret) {
   176			bus_remove_driver(drv);
   177			return ret;
   178		}
   179		kobject_uevent(&drv->p->kobj, KOBJ_ADD);
   180	
   181		return ret;
   182	}
   183	EXPORT_SYMBOL_GPL(driver_register);
   184	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34081 bytes --]

Re: [PATCH v3 2/3] driver-core: record error on bus registration

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 20297 bytes --]

Hi Florian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on v4.17-rc5 next-20180515]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Florian-Schmaus/return-EINVAL-error-instead-of-BUG_ON/20180516-141149
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.msdu_retries' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.msdu_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info'
   kernel/sched/fair.c:3731: warning: Function parameter or member 'flags' not described in 'attach_entity_load_avg'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf'
   include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array'
   include/linux/gpio/driver.h:142: warning: Function parameter or member 'request_key' not described in 'gpio_irq_chip'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.sign' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.realbits' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.storagebits' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.shift' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.repeat' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.endianness' not described in 'iio_chan_spec'
   include/linux/iio/hw-consumer.h:1: warning: no structured comments found
>> include/linux/device.h:143: warning: Function parameter or member 'bus_register_error' not described in 'bus_type'
   include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry'
   include/linux/mtd/rawnand.h:752: warning: Function parameter or member 'timings.sdr' not described in 'nand_data_interface'
   include/linux/mtd/rawnand.h:817: warning: Function parameter or member 'buf' not described in 'nand_op_data_instr'
   include/linux/mtd/rawnand.h:817: warning: Function parameter or member 'buf.in' not described in 'nand_op_data_instr'
   include/linux/mtd/rawnand.h:817: warning: Function parameter or member 'buf.out' not described in 'nand_op_data_instr'
   include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.cmd' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.data' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.waitrdy' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:1010: warning: Function parameter or member 'ctx' not described in 'nand_op_parser_pattern_elem'
   include/linux/mtd/rawnand.h:1010: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_parser_pattern_elem'
   include/linux/mtd/rawnand.h:1010: warning: Function parameter or member 'ctx.data' not described in 'nand_op_parser_pattern_elem'
   include/linux/mtd/rawnand.h:1313: warning: Function parameter or member 'manufacturer.desc' not described in 'nand_chip'
   include/linux/mtd/rawnand.h:1313: warning: Function parameter or member 'manufacturer.priv' not described in 'nand_chip'
   include/linux/regulator/driver.h:222: warning: Function parameter or member 'resume_early' not described in 'regulator_ops'
   drivers/regulator/core.c:4306: warning: Excess function parameter 'state' description in 'regulator_suspend_late'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb'
   drivers/usb/typec/mux.c:186: warning: Function parameter or member 'mux' not described in 'typec_mux_unregister'
   drivers/usb/typec/mux.c:186: warning: Excess function parameter 'sw' description in 'typec_mux_unregister'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver'
   drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'attach' not described in 'drm_gem_unmap_dma_buf'
   drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'sgt' not described in 'drm_gem_unmap_dma_buf'
   drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'dir' not described in 'drm_gem_unmap_dma_buf'
   drivers/gpu/drm/drm_prime.c:438: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kmap_atomic'
   drivers/gpu/drm/drm_prime.c:438: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kmap_atomic'
   drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kunmap_atomic'
   drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kunmap_atomic'
   drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'addr' not described in 'drm_gem_dmabuf_kunmap_atomic'
   drivers/gpu/drm/drm_prime.c:461: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kmap'
   drivers/gpu/drm/drm_prime.c:461: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kmap'
   drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kunmap'
   drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kunmap'
   drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'addr' not described in 'drm_gem_dmabuf_kunmap'
   include/media/v4l2-dev.h:42: warning: Enum value 'VFL_TYPE_MAX' not described in enum 'vfl_devnode_type'
   include/linux/skbuff.h:850: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'skb_mstamp' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member '__unused' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'pfmemalloc' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'offload_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'offload_mr_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:234: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.rmem_alloc' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.len' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.head' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.tail' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'adj_list.upper' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'adj_list.lower' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'switchdev_ops' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'

vim +143 include/linux/device.h

^1da177e Linus Torvalds 2005-04-16 @143  

:::::: The code at line 143 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6370 bytes --]

[PATCH v4 0/3] return EINVAL error instead of BUG_ON()

[Florian Schmaus]

This patch series makes driver_register() emit an error message and
return a failure code instead of triggering a BUG_ON().

The first patch will now print out the name of the faulty driver *and*
the name of the involved bus. The second patch records the error code
if bus_register() fails. The third and final patch of the series
extends the first patch so that the recorded error code is also print
out if non-zero. Otherwhise, if drv->bus->p is NULL but the error code
is zero, then probably bus_register() was never called before.

Florian Schmaus (3):
  driver-core: return EINVAL error instead of BUG_ON()
  driver-core: record error on bus registration
  driver-core: print bus registration error value

 drivers/base/bus.c     |  2 ++
 drivers/base/driver.c  | 14 +++++++++++++-
 include/linux/device.h |  1 +
 3 files changed, 16 insertions(+), 1 deletion(-)

-- 
2.16.1

[PATCH v4 1/3] driver-core: return EINVAL error instead of BUG_ON()

[Florian Schmaus]

I triggerd the BUG_ON() in driver_register() when booting a domU Xen
domain. Since there was no contextual information logged, I needed to
attach kgdb to determine the culprit (the wmi-bmof driver in my
case). The BUG_ON() was added in commit f48f3febb2cb ("driver-core: do
not register a driver with bus_type not registered").

Instead of running into a BUG_ON() we print an error message
identifying the, likely faulty, driver but continue booting.

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---

Notes:
    - Also print out the bus name
    - Use pr_err() instead of printk()

 drivers/base/driver.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index ba912558a510..203fa731e3ee 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -148,7 +148,12 @@ int driver_register(struct device_driver *drv)
 	int ret;
 	struct device_driver *other;
 
-	BUG_ON(!drv->bus->p);
+	if (!drv->bus->p) {
+		pr_err("Driver '%s' was unable to register with bus_type '%s'"
+			   " because it was not initialized.\n",
+			   drv->name, drv->bus->name);
+		return -EINVAL;
+	}
 
 	if ((drv->bus->probe && drv->probe) ||
 	    (drv->bus->remove && drv->remove) ||
-- 
2.16.1

[PATCH v4 2/3] driver-core: record error on bus registration

[Florian Schmaus]

If bus_register() fails on a driver then record the error code so that
it can be inspected later on.

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---

Notes:
    - Also record ENOMEM error if initial alloc fails

 drivers/base/bus.c     | 2 ++
 include/linux/device.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index ef6183306b40..dce677dddba4 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -152,6 +152,7 @@ static void bus_release(struct kobject *kobj)
 
 	kfree(priv);
 	bus->p = NULL;
+	bus->bus_register_error = 0;
 }
 
 static struct kobj_type bus_ktype = {
@@ -915,6 +916,7 @@ int bus_register(struct bus_type *bus)
 out:
 	kfree(bus->p);
 	bus->p = NULL;
+	bus->bus_register_error = retval;
 	return retval;
 }
 EXPORT_SYMBOL_GPL(bus_register);
diff --git a/include/linux/device.h b/include/linux/device.h
index 0059b99e1f25..5b1f3c08bebe 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -135,6 +135,7 @@ struct bus_type {
 	const struct iommu_ops *iommu_ops;
 
 	struct subsys_private *p;
+	int bus_register_error;
 	struct lock_class_key lock_key;
 
 	bool force_dma;
-- 
2.16.1

[PATCH v4 3/3] driver-core: print bus registration error value

[Florian Schmaus]

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---

Notes:
    - Use correct member name 'bus_register_error'
    - Only print out error code if it is non-zero

 drivers/base/bus.c    |  4 +++-
 drivers/base/driver.c | 13 ++++++++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index dce677dddba4..5814ecb07648 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -849,8 +849,10 @@ int bus_register(struct bus_type *bus)
 	struct lock_class_key *key = &bus->lock_key;
 
 	priv = kzalloc(sizeof(struct subsys_private), GFP_KERNEL);
-	if (!priv)
+	if (!priv) {
+		bus->bus_register_error = -ENOMEM;
 		return -ENOMEM;
+	}
 
 	priv->bus = bus;
 	bus->p = priv;
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 203fa731e3ee..a798aeae08c7 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -149,9 +149,16 @@ int driver_register(struct device_driver *drv)
 	struct device_driver *other;
 
 	if (!drv->bus->p) {
-		pr_err("Driver '%s' was unable to register with bus_type '%s'"
-			   " because it was not initialized.\n",
-			   drv->name, drv->bus->name);
+		if (drv->bus->bus_register_error) {
+			pr_err("Driver '%s' was unable to register with bus_type '%s'",
+				   " (error: %d).\n",
+				   drv->name, drv->bus->name,
+				   drv->bus->bus_register_error);
+		} else {
+			pr_err("Driver '%s' was unable to register with bus_type '%s'",
+				   " because it was not initialized.\n",
+				   drv->name, drv->bus->name);
+		}
 		return -EINVAL;
 	}
 
-- 
2.16.1

Re: [PATCH v4 3/3] driver-core: print bus registration error value

[Florian Schmaus]

[-- Attachment #1.1: Type: text/plain, Size: 981 bytes --]

On 16.05.2018 14:05, Florian Schmaus wrote:
> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> ---
> 
> Notes:
>     - Use correct member name 'bus_register_error'
>     - Only print out error code if it is non-zero
> 
>  drivers/base/bus.c    |  4 +++-
>  drivers/base/driver.c | 13 ++++++++++---
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index dce677dddba4..5814ecb07648 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -849,8 +849,10 @@ int bus_register(struct bus_type *bus)
>  	struct lock_class_key *key = &bus->lock_key;
>  
>  	priv = kzalloc(sizeof(struct subsys_private), GFP_KERNEL);
> -	if (!priv)
> +	if (!priv) {
> +		bus->bus_register_error = -ENOMEM;
>  		return -ENOMEM;
> +	}
>  
>  	priv->bus = bus;
>  	bus->p = priv;

I'm sorry, this change was meant to be part of 2/3 (and not 3/3). Shall
I send a v5 where this is fixed?

- Florian


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [PATCH v4 3/3] driver-core: print bus registration error value

[Greg Kroah-Hartman]

On Wed, May 16, 2018 at 02:05:27PM +0200, Florian Schmaus wrote:
> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> ---

I can't take patches without any changelog text at all, sorry.

thanks,

greg k-h

Re: [PATCH v4 3/3] driver-core: print bus registration error value

[Greg Kroah-Hartman]

On Wed, May 16, 2018 at 02:09:54PM +0200, Florian Schmaus wrote:
> On 16.05.2018 14:05, Florian Schmaus wrote:
> > Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> > ---
> > 
> > Notes:
> >     - Use correct member name 'bus_register_error'
> >     - Only print out error code if it is non-zero
> > 
> >  drivers/base/bus.c    |  4 +++-
> >  drivers/base/driver.c | 13 ++++++++++---
> >  2 files changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> > index dce677dddba4..5814ecb07648 100644
> > --- a/drivers/base/bus.c
> > +++ b/drivers/base/bus.c
> > @@ -849,8 +849,10 @@ int bus_register(struct bus_type *bus)
> >  	struct lock_class_key *key = &bus->lock_key;
> >  
> >  	priv = kzalloc(sizeof(struct subsys_private), GFP_KERNEL);
> > -	if (!priv)
> > +	if (!priv) {
> > +		bus->bus_register_error = -ENOMEM;
> >  		return -ENOMEM;
> > +	}
> >  
> >  	priv->bus = bus;
> >  	bus->p = priv;
> 
> I'm sorry, this change was meant to be part of 2/3 (and not 3/3). Shall
> I send a v5 where this is fixed?

Well, I can't take it as-is, so of course :)

thanks,

greg k-h

Re: [PATCH v4 1/3] driver-core: return EINVAL error instead of BUG_ON()

[Greg Kroah-Hartman]

On Wed, May 16, 2018 at 02:05:25PM +0200, Florian Schmaus wrote:
> I triggerd the BUG_ON() in driver_register() when booting a domU Xen
> domain. Since there was no contextual information logged, I needed to
> attach kgdb to determine the culprit (the wmi-bmof driver in my
> case). The BUG_ON() was added in commit f48f3febb2cb ("driver-core: do
> not register a driver with bus_type not registered").
> 
> Instead of running into a BUG_ON() we print an error message
> identifying the, likely faulty, driver but continue booting.
> 
> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> ---
> 
> Notes:
>     - Also print out the bus name
>     - Use pr_err() instead of printk()
> 
>  drivers/base/driver.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index ba912558a510..203fa731e3ee 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -148,7 +148,12 @@ int driver_register(struct device_driver *drv)
>  	int ret;
>  	struct device_driver *other;
>  
> -	BUG_ON(!drv->bus->p);
> +	if (!drv->bus->p) {
> +		pr_err("Driver '%s' was unable to register with bus_type '%s'"
> +			   " because it was not initialized.\n",

Are you using checkpatch.pl?

Anyway, long strings should never be split across lines.  This needs to
be one line.

Sometimes making small changes is hard :)

thanks,

greg k-h

Re: [PATCH v4 1/3] driver-core: return EINVAL error instead of BUG_ON()

[Florian Schmaus]

[-- Attachment #1.1: Type: text/plain, Size: 1727 bytes --]

On 16.05.2018 17:39, Greg Kroah-Hartman wrote:
> On Wed, May 16, 2018 at 02:05:25PM +0200, Florian Schmaus wrote:
>> I triggerd the BUG_ON() in driver_register() when booting a domU Xen
>> domain. Since there was no contextual information logged, I needed to
>> attach kgdb to determine the culprit (the wmi-bmof driver in my
>> case). The BUG_ON() was added in commit f48f3febb2cb ("driver-core: do
>> not register a driver with bus_type not registered").
>>
>> Instead of running into a BUG_ON() we print an error message
>> identifying the, likely faulty, driver but continue booting.
>>
>> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
>> ---
>>
>> Notes:
>>     - Also print out the bus name
>>     - Use pr_err() instead of printk()
>>
>>  drivers/base/driver.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
>> index ba912558a510..203fa731e3ee 100644
>> --- a/drivers/base/driver.c
>> +++ b/drivers/base/driver.c
>> @@ -148,7 +148,12 @@ int driver_register(struct device_driver *drv)
>>  	int ret;
>>  	struct device_driver *other;
>>  
>> -	BUG_ON(!drv->bus->p);
>> +	if (!drv->bus->p) {
>> +		pr_err("Driver '%s' was unable to register with bus_type '%s'"
>> +			   " because it was not initialized.\n",
> 
> Are you using checkpatch.pl?

I do, I thought this was one of the rare occasions where it is sensible
to split the lines, also given that the other strings within the same
function are also split.

> Anyway, long strings should never be split across lines.  This needs to
> be one line.

Fixed in v5.

> Sometimes making small changes is hard :)

True words ;)

- Florian


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

[PATCH v5 0/3] return EINVAL error instead of BUG_ON()

[Florian Schmaus]

This patch series makes driver_register() emit an error message and
return a failure code instead of triggering a BUG_ON().

The first patch will cause driver_register() to fail gracefully if the
driver's bus was not initialized while printing out the name of the
faulty driver *and* the name of the involved bus. The second patch
records the error code if bus_register() fails. The third and final
patch of the series extends the first patch so that the recorded error
code is also print out if non-zero. Otherwhise, if drv->bus->p is NULL
but the error code is zero, then probably bus_register() was never
called before.

Florian Schmaus (3):
  driver-core: return EINVAL error instead of BUG_ON()
  driver-core: record error on bus registration
  driver-core: print bus registration error value

 drivers/base/bus.c     |  6 +++++-
 drivers/base/driver.c  | 12 +++++++++++-
 include/linux/device.h |  1 +
 3 files changed, 17 insertions(+), 2 deletions(-)

-- 
2.16.1

[PATCH v5 1/3] driver-core: return EINVAL error instead of BUG_ON()

[Florian Schmaus]

I triggerd the BUG_ON() in driver_register() when booting a domU Xen
domain. Since there was no contextual information logged, I needed to
attach kgdb to determine the culprit (the wmi-bmof driver in my
case). The BUG_ON() was added in commit f48f3febb2cb ("driver-core: do
not register a driver with bus_type not registered").

Instead of running into a BUG_ON() we print an error message
identifying the, likely faulty, driver but continue booting.

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---

Notes:
    - Do not split long strings across lines.

 drivers/base/driver.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index ba912558a510..16b81d1c6cb7 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv)
 	int ret;
 	struct device_driver *other;
 
-	BUG_ON(!drv->bus->p);
+	if (!drv->bus->p) {
+		pr_err("Driver '%s' was unable to register with bus_type '%s' because it was not initialized.\n",
+			   drv->name, drv->bus->name);
+		return -EINVAL;
+	}
 
 	if ((drv->bus->probe && drv->probe) ||
 	    (drv->bus->remove && drv->remove) ||
-- 
2.16.1

[PATCH v5 2/3] driver-core: record error on bus registration

[Florian Schmaus]

If bus_register() fails on a driver then record the error code so that
it can be inspected later on.

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---

Notes:
    - Also record ENOMEM error code.

 drivers/base/bus.c     | 6 +++++-
 include/linux/device.h | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index ef6183306b40..5814ecb07648 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -152,6 +152,7 @@ static void bus_release(struct kobject *kobj)
 
 	kfree(priv);
 	bus->p = NULL;
+	bus->bus_register_error = 0;
 }
 
 static struct kobj_type bus_ktype = {
@@ -848,8 +849,10 @@ int bus_register(struct bus_type *bus)
 	struct lock_class_key *key = &bus->lock_key;
 
 	priv = kzalloc(sizeof(struct subsys_private), GFP_KERNEL);
-	if (!priv)
+	if (!priv) {
+		bus->bus_register_error = -ENOMEM;
 		return -ENOMEM;
+	}
 
 	priv->bus = bus;
 	bus->p = priv;
@@ -915,6 +918,7 @@ int bus_register(struct bus_type *bus)
 out:
 	kfree(bus->p);
 	bus->p = NULL;
+	bus->bus_register_error = retval;
 	return retval;
 }
 EXPORT_SYMBOL_GPL(bus_register);
diff --git a/include/linux/device.h b/include/linux/device.h
index 0059b99e1f25..5b1f3c08bebe 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -135,6 +135,7 @@ struct bus_type {
 	const struct iommu_ops *iommu_ops;
 
 	struct subsys_private *p;
+	int bus_register_error;
 	struct lock_class_key lock_key;
 
 	bool force_dma;
-- 
2.16.1

[PATCH v5 3/3] driver-core: print bus registration error value

[Florian Schmaus]

If driver_register() is called with an device driver which previously
called bus_register() but failed, then print out the bus_register()
error code.

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---

Notes:
    - Do not split long strings across lines.

 drivers/base/driver.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 16b81d1c6cb7..a1fe7cb43c7e 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -149,8 +149,14 @@ int driver_register(struct device_driver *drv)
 	struct device_driver *other;
 
 	if (!drv->bus->p) {
-		pr_err("Driver '%s' was unable to register with bus_type '%s' because it was not initialized.\n",
-			   drv->name, drv->bus->name);
+		if (drv->bus->bus_register_error) {
+			pr_err("Driver '%s' was unable to register with bus_type '%s' because of error: %d.\n",
+				   drv->name, drv->bus->name,
+				   drv->bus->bus_register_error);
+		} else {
+			pr_err("Driver '%s' was unable to register with bus_type '%s' because it was not initialized.\n",
+				   drv->name, drv->bus->name);
+		}
 		return -EINVAL;
 	}
 
-- 
2.16.1

Re: [PATCH v5 1/3] driver-core: return EINVAL error instead of BUG_ON()

[Greg Kroah-Hartman]

On Wed, May 16, 2018 at 06:08:14PM +0200, Florian Schmaus wrote:
> I triggerd the BUG_ON() in driver_register() when booting a domU Xen
> domain. Since there was no contextual information logged, I needed to
> attach kgdb to determine the culprit (the wmi-bmof driver in my
> case). The BUG_ON() was added in commit f48f3febb2cb ("driver-core: do
> not register a driver with bus_type not registered").
> 
> Instead of running into a BUG_ON() we print an error message
> identifying the, likely faulty, driver but continue booting.
> 
> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> ---
> 
> Notes:
>     - Do not split long strings across lines.
> 
>  drivers/base/driver.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index ba912558a510..16b81d1c6cb7 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv)
>  	int ret;
>  	struct device_driver *other;
>  
> -	BUG_ON(!drv->bus->p);
> +	if (!drv->bus->p) {
> +		pr_err("Driver '%s' was unable to register with bus_type '%s' because it was not initialized.\n",

The bus is not initialized, not the driver, right?  This kind of makes
it sound the other way around :(

thanks,

greg k-h

Re: [PATCH v5 2/3] driver-core: record error on bus registration

[Greg Kroah-Hartman]

On Wed, May 16, 2018 at 06:08:15PM +0200, Florian Schmaus wrote:
> If bus_register() fails on a driver then record the error code so that
> it can be inspected later on.

I don't think this is really needed, has it ever triggered before?

thanks,

greg k-h

Re: [PATCH v5 3/3] driver-core: print bus registration error value

[Greg Kroah-Hartman]

On Wed, May 16, 2018 at 06:08:16PM +0200, Florian Schmaus wrote:
> If driver_register() is called with an device driver which previously
> called bus_register() but failed, then print out the bus_register()
> error code.
> 
> Signed-off-by: Florian Schmaus <flo@geekplace.eu>
> ---
> 
> Notes:
>     - Do not split long strings across lines.
> 
>  drivers/base/driver.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index 16b81d1c6cb7..a1fe7cb43c7e 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -149,8 +149,14 @@ int driver_register(struct device_driver *drv)
>  	struct device_driver *other;
>  
>  	if (!drv->bus->p) {
> -		pr_err("Driver '%s' was unable to register with bus_type '%s' because it was not initialized.\n",
> -			   drv->name, drv->bus->name);
> +		if (drv->bus->bus_register_error) {
> +			pr_err("Driver '%s' was unable to register with bus_type '%s' because of error: %d.\n",
> +				   drv->name, drv->bus->name,
> +				   drv->bus->bus_register_error);

The error was with the bus registration, not the driver registration.

And really, when has this ever happened?  Why would a bus registration
fail and later allow a driver to be registered?  The bus registration
error would have shown up somewhere, no need to keep it around for no
real good reason.

So I don't think that patches 2 and 3 are needed at all here, sorry.

thanks,

greg k-h

Re: [PATCH v4 3/3] driver-core: print bus registration error value

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5887 bytes --]

Hi Florian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on v4.17-rc5 next-20180516]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Florian-Schmaus/return-EINVAL-error-instead-of-BUG_ON/20180517-145147
config: x86_64-randconfig-x002-201819 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:7:0,
                    from include/linux/kernel.h:14,
                    from include/linux/list.h:9,
                    from include/linux/kobject.h:19,
                    from include/linux/device.h:16,
                    from drivers/base/driver.c:11:
   drivers/base/driver.c: In function 'driver_register':
>> include/linux/kern_levels.h:5:18: warning: too many arguments for format [-Wformat-extra-args]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
    #define KERN_ERR KERN_SOH "3" /* error conditions */
                     ^~~~~~~~
>> include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
     printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
            ^~~~~~~~
>> drivers/base/driver.c:153:4: note: in expansion of macro 'pr_err'
       pr_err("Driver '%s' was unable to register with bus_type '%s'",
       ^~~~~~
>> include/linux/kern_levels.h:5:18: warning: too many arguments for format [-Wformat-extra-args]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
    #define KERN_ERR KERN_SOH "3" /* error conditions */
                     ^~~~~~~~
>> include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
     printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
            ^~~~~~~~
   drivers/base/driver.c:158:4: note: in expansion of macro 'pr_err'
       pr_err("Driver '%s' was unable to register with bus_type '%s'",
       ^~~~~~
--
   In file included from include/linux/printk.h:7:0,
                    from include/linux/kernel.h:14,
                    from include/linux/list.h:9,
                    from include/linux/kobject.h:19,
                    from include/linux/device.h:16,
                    from drivers//base/driver.c:11:
   drivers//base/driver.c: In function 'driver_register':
>> include/linux/kern_levels.h:5:18: warning: too many arguments for format [-Wformat-extra-args]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
    #define KERN_ERR KERN_SOH "3" /* error conditions */
                     ^~~~~~~~
>> include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
     printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
            ^~~~~~~~
   drivers//base/driver.c:153:4: note: in expansion of macro 'pr_err'
       pr_err("Driver '%s' was unable to register with bus_type '%s'",
       ^~~~~~
>> include/linux/kern_levels.h:5:18: warning: too many arguments for format [-Wformat-extra-args]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
    #define KERN_ERR KERN_SOH "3" /* error conditions */
                     ^~~~~~~~
>> include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
     printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
            ^~~~~~~~
   drivers//base/driver.c:158:4: note: in expansion of macro 'pr_err'
       pr_err("Driver '%s' was unable to register with bus_type '%s'",
       ^~~~~~

vim +/pr_err +153 drivers/base/driver.c

   137	
   138	/**
   139	 * driver_register - register driver with bus
   140	 * @drv: driver to register
   141	 *
   142	 * We pass off most of the work to the bus_add_driver() call,
   143	 * since most of the things we have to do deal with the bus
   144	 * structures.
   145	 */
   146	int driver_register(struct device_driver *drv)
   147	{
   148		int ret;
   149		struct device_driver *other;
   150	
   151		if (!drv->bus->p) {
   152			if (drv->bus->bus_register_error) {
 > 153				pr_err("Driver '%s' was unable to register with bus_type '%s'",
   154					   " (error: %d).\n",
   155					   drv->name, drv->bus->name,
   156					   drv->bus->bus_register_error);
   157			} else {
   158				pr_err("Driver '%s' was unable to register with bus_type '%s'",
   159					   " because it was not initialized.\n",
   160					   drv->name, drv->bus->name);
   161			}
   162			return -EINVAL;
   163		}
   164	
   165		if ((drv->bus->probe && drv->probe) ||
   166		    (drv->bus->remove && drv->remove) ||
   167		    (drv->bus->shutdown && drv->shutdown))
   168			printk(KERN_WARNING "Driver '%s' needs updating - please use "
   169				"bus_type methods\n", drv->name);
   170	
   171		other = driver_find(drv->name, drv->bus);
   172		if (other) {
   173			printk(KERN_ERR "Error: Driver '%s' is already registered, "
   174				"aborting...\n", drv->name);
   175			return -EBUSY;
   176		}
   177	
   178		ret = bus_add_driver(drv);
   179		if (ret)
   180			return ret;
   181		ret = driver_add_groups(drv, drv->groups);
   182		if (ret) {
   183			bus_remove_driver(drv);
   184			return ret;
   185		}
   186		kobject_uevent(&drv->p->kobj, KOBJ_ADD);
   187	
   188		return ret;
   189	}
   190	EXPORT_SYMBOL_GPL(driver_register);
   191	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24347 bytes --]

Re: [PATCH v4 2/3] driver-core: record error on bus registration

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 20297 bytes --]

Hi Florian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on v4.17-rc5 next-20180516]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Florian-Schmaus/return-EINVAL-error-instead-of-BUG_ON/20180517-145147
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   include/net/mac80211.h:2083: warning: bad line: >
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.msdu_retries' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.msdu_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info'
   net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info'
   kernel/sched/fair.c:3731: warning: Function parameter or member 'flags' not described in 'attach_entity_load_avg'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf'
   include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array'
   include/linux/gpio/driver.h:142: warning: Function parameter or member 'request_key' not described in 'gpio_irq_chip'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.sign' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.realbits' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.storagebits' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.shift' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.repeat' not described in 'iio_chan_spec'
   include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.endianness' not described in 'iio_chan_spec'
   include/linux/iio/hw-consumer.h:1: warning: no structured comments found
>> include/linux/device.h:143: warning: Function parameter or member 'bus_register_error' not described in 'bus_type'
   include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry'
   include/linux/mtd/rawnand.h:752: warning: Function parameter or member 'timings.sdr' not described in 'nand_data_interface'
   include/linux/mtd/rawnand.h:817: warning: Function parameter or member 'buf' not described in 'nand_op_data_instr'
   include/linux/mtd/rawnand.h:817: warning: Function parameter or member 'buf.in' not described in 'nand_op_data_instr'
   include/linux/mtd/rawnand.h:817: warning: Function parameter or member 'buf.out' not described in 'nand_op_data_instr'
   include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.cmd' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.data' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.waitrdy' not described in 'nand_op_instr'
   include/linux/mtd/rawnand.h:1010: warning: Function parameter or member 'ctx' not described in 'nand_op_parser_pattern_elem'
   include/linux/mtd/rawnand.h:1010: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_parser_pattern_elem'
   include/linux/mtd/rawnand.h:1010: warning: Function parameter or member 'ctx.data' not described in 'nand_op_parser_pattern_elem'
   include/linux/mtd/rawnand.h:1313: warning: Function parameter or member 'manufacturer.desc' not described in 'nand_chip'
   include/linux/mtd/rawnand.h:1313: warning: Function parameter or member 'manufacturer.priv' not described in 'nand_chip'
   include/linux/regulator/driver.h:222: warning: Function parameter or member 'resume_early' not described in 'regulator_ops'
   drivers/regulator/core.c:4306: warning: Excess function parameter 'state' description in 'regulator_suspend_late'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb'
   drivers/usb/typec/mux.c:186: warning: Function parameter or member 'mux' not described in 'typec_mux_unregister'
   drivers/usb/typec/mux.c:186: warning: Excess function parameter 'sw' description in 'typec_mux_unregister'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver'
   drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'attach' not described in 'drm_gem_unmap_dma_buf'
   drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'sgt' not described in 'drm_gem_unmap_dma_buf'
   drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'dir' not described in 'drm_gem_unmap_dma_buf'
   drivers/gpu/drm/drm_prime.c:438: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kmap_atomic'
   drivers/gpu/drm/drm_prime.c:438: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kmap_atomic'
   drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kunmap_atomic'
   drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kunmap_atomic'
   drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'addr' not described in 'drm_gem_dmabuf_kunmap_atomic'
   drivers/gpu/drm/drm_prime.c:461: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kmap'
   drivers/gpu/drm/drm_prime.c:461: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kmap'
   drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kunmap'
   drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kunmap'
   drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'addr' not described in 'drm_gem_dmabuf_kunmap'
   include/media/v4l2-dev.h:42: warning: Enum value 'VFL_TYPE_MAX' not described in enum 'vfl_devnode_type'
   include/linux/skbuff.h:850: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'skb_mstamp' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member '__unused' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'pfmemalloc' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'offload_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'offload_mr_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:850: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:234: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
   include/net/sock.h:234: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.rmem_alloc' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.len' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.head' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.tail' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
   include/net/sock.h:488: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'adj_list.upper' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'adj_list.lower' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'switchdev_ops' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device'
   include/linux/netdevice.h:1955: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'

vim +143 include/linux/device.h

^1da177e Linus Torvalds 2005-04-16 @143  

:::::: The code at line 143 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6370 bytes --]

[PATCH v6 0/3] driver-core: return EINVAL error instead of BUG_ON()

[Florian Schmaus]

This patch series makes driver_register() emit an error message and
return a failure code instead of triggering a BUG_ON().

The first patch will cause driver_register() to fail gracefully if the
driver's bus was not initialized while printing out the name of the
faulty driver *and* the name of the involved bus. The second patch
records the error code if bus_register() fails. The third and final
patch of the series extends the first patch so that the recorded error
code is also print out if non-zero. Otherwise, if drv->bus->p is NULL
but the error code is zero, then probably bus_register() was never
called before.

Greg questioned [1] whether [2/3] and [3/3] are necessary:

> And really, when has this ever happened?  Why would a bus registration
> fail and later allow a driver to be registered?

I initially assumed that this is what cause me hitting the BUG_ON()
which [1/3] replaces: The bus registration failed and then the driver
attempts to register itself. But I did not had a chance to verify
that. I'll try to do so after my vacation. Meanwhile I hope that at
least [1/3] is considered an improvement of the kernel. If so, feel
free to pick it up.

1: Message-ID: <20180516163707.GD20425@kroah.com>

Florian Schmaus (3):
  driver-core: return EINVAL error instead of BUG_ON()
  driver-core: record error on bus registration
  driver-core: print bus registration error value

 drivers/base/bus.c     |  9 +++++++--
 drivers/base/driver.c  | 12 +++++++++++-
 include/linux/device.h |  1 +
 3 files changed, 19 insertions(+), 3 deletions(-)

-- 
2.16.1

[PATCH v6 1/3] driver-core: return EINVAL error instead of BUG_ON()

[Florian Schmaus]

I triggerd the BUG_ON() in driver_register() when booting a domU Xen
domain. Since there was no contextual information logged, I needed to
attach kgdb to determine the culprit (the wmi-bmof driver in my
case). The BUG_ON() was added in commit f48f3febb2cb ("driver-core: do
not register a driver with bus_type not registered").

Instead of running into a BUG_ON() we print an error message
identifying the, likely faulty, driver but continue booting.

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---

Notes:
    Make it clear in the error message that the bus was not initialized
    and *not* the driver.

 drivers/base/driver.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index ba912558a510..857c8f1b876e 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv)
 	int ret;
 	struct device_driver *other;
 
-	BUG_ON(!drv->bus->p);
+	if (!drv->bus->p) {
+		pr_err("Driver '%s' was unable to register with bus_type '%s' because the bus was not initialized.\n",
+			   drv->name, drv->bus->name);
+		return -EINVAL;
+	}
 
 	if ((drv->bus->probe && drv->probe) ||
 	    (drv->bus->remove && drv->remove) ||
-- 
2.16.1

[PATCH v6 2/3] driver-core: record error on bus registration

[Florian Schmaus]

If bus_register() fails on a driver then record the error code so that
it can be inspected later on.

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---

Notes:
    Also record ENOMEM error

 drivers/base/bus.c     | 9 +++++++--
 include/linux/device.h | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index ef6183306b40..6e27741eb03e 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -152,6 +152,7 @@ static void bus_release(struct kobject *kobj)
 
 	kfree(priv);
 	bus->p = NULL;
+	bus->bus_register_error = 0;
 }
 
 static struct kobj_type bus_ktype = {
@@ -848,8 +849,10 @@ int bus_register(struct bus_type *bus)
 	struct lock_class_key *key = &bus->lock_key;
 
 	priv = kzalloc(sizeof(struct subsys_private), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
+	if (!priv) {
+		retval = -ENOMEM;
+		goto bus_alloc_fail;
+	}
 
 	priv->bus = bus;
 	bus->p = priv;
@@ -914,7 +917,9 @@ int bus_register(struct bus_type *bus)
 	kset_unregister(&bus->p->subsys);
 out:
 	kfree(bus->p);
+bus_alloc_fail:
 	bus->p = NULL;
+	bus->bus_register_error = retval;
 	return retval;
 }
 EXPORT_SYMBOL_GPL(bus_register);
diff --git a/include/linux/device.h b/include/linux/device.h
index 0059b99e1f25..5b1f3c08bebe 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -135,6 +135,7 @@ struct bus_type {
 	const struct iommu_ops *iommu_ops;
 
 	struct subsys_private *p;
+	int bus_register_error;
 	struct lock_class_key lock_key;
 
 	bool force_dma;
-- 
2.16.1

[PATCH v6 3/3] driver-core: print bus registration error value

[Florian Schmaus]

If driver_register() is called with an device driver which previously
called bus_register() but failed, then print out the bus_register()
error code.

Signed-off-by: Florian Schmaus <flo@geekplace.eu>
---

Notes:
    Make it clear in the error message that the error code is from the bus
    registration and *not* from the driver registration.

 drivers/base/driver.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 857c8f1b876e..b21fed9687c9 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -149,8 +149,14 @@ int driver_register(struct device_driver *drv)
 	struct device_driver *other;
 
 	if (!drv->bus->p) {
-		pr_err("Driver '%s' was unable to register with bus_type '%s' because the bus was not initialized.\n",
-			   drv->name, drv->bus->name);
+		if (drv->bus->bus_register_error) {
+			pr_err("Driver '%s' was unable to register with bus_type '%s' because bus registration failed with error %d.\n",
+				   drv->name, drv->bus->name,
+				   drv->bus->bus_register_error);
+		} else {
+			pr_err("Driver '%s' was unable to register with bus_type '%s' because the bus was not initialized.\n",
+				   drv->name, drv->bus->name);
+		}
 		return -EINVAL;
 	}
 
-- 
2.16.1

Re: [PATCH v6 0/3] driver-core: return EINVAL error instead of BUG_ON()

[Greg Kroah-Hartman]

On Wed, May 23, 2018 at 05:59:10PM +0200, Florian Schmaus wrote:
> This patch series makes driver_register() emit an error message and
> return a failure code instead of triggering a BUG_ON().
> 
> The first patch will cause driver_register() to fail gracefully if the
> driver's bus was not initialized while printing out the name of the
> faulty driver *and* the name of the involved bus. The second patch
> records the error code if bus_register() fails. The third and final
> patch of the series extends the first patch so that the recorded error
> code is also print out if non-zero. Otherwise, if drv->bus->p is NULL
> but the error code is zero, then probably bus_register() was never
> called before.
> 
> Greg questioned [1] whether [2/3] and [3/3] are necessary:
> 
> > And really, when has this ever happened?  Why would a bus registration
> > fail and later allow a driver to be registered?
> 
> I initially assumed that this is what cause me hitting the BUG_ON()
> which [1/3] replaces: The bus registration failed and then the driver
> attempts to register itself. But I did not had a chance to verify
> that. I'll try to do so after my vacation. Meanwhile I hope that at
> least [1/3] is considered an improvement of the kernel. If so, feel
> free to pick it up.

I've picked up patch 1, I don't think patches 2 or 3 are needed, due to
the extreme unfrequency that this problem ever happens :)

thanks,

greg k-h