[PATCH 3/3] bcache: do not assign in if condition in bcache_device_init()
Fixes an error condition reported by checkpatch.pl which is caused by
assigning a variable in an if condition.
Signed-off-by: Florian Schmaus
---
drivers/md/bcache/super.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index f09f4f046315..4a2a028c8754 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -796,11 +796,12 @@ static int bcache_device_init(struct bcache_device *d,
unsigned block_size,
return idx;
if (bioset_init(>bio_split, 4, offsetof(struct bbio, bio),
- BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER) ||
- !(d->disk = alloc_disk(BCACHE_MINORS))) {
- ida_simple_remove(_device_idx, idx);
- return -ENOMEM;
- }
+ BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
+ goto err;
+
+ d->disk = alloc_disk(BCACHE_MINORS);
+ if (!d->disk)
+ goto err;
set_capacity(d->disk, sectors);
snprintf(d->disk->disk_name, DISK_NAME_LEN, "bcache%i", idx);
@@ -834,6 +835,11 @@ static int bcache_device_init(struct bcache_device *d,
unsigned block_size,
blk_queue_write_cache(q, true, true);
return 0;
+
+err:
+ ida_simple_remove(_device_idx, idx);
+ return -ENOMEM;
+
}
/* Cached device */
--
2.16.4
[PATCH 3/3] bcache: do not assign in if condition in bcache_device_init()
Fixes an error condition reported by checkpatch.pl which is caused by
assigning a variable in an if condition.
Signed-off-by: Florian Schmaus
---
drivers/md/bcache/super.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index f09f4f046315..4a2a028c8754 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -796,11 +796,12 @@ static int bcache_device_init(struct bcache_device *d,
unsigned block_size,
return idx;
if (bioset_init(>bio_split, 4, offsetof(struct bbio, bio),
- BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER) ||
- !(d->disk = alloc_disk(BCACHE_MINORS))) {
- ida_simple_remove(_device_idx, idx);
- return -ENOMEM;
- }
+ BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER))
+ goto err;
+
+ d->disk = alloc_disk(BCACHE_MINORS);
+ if (!d->disk)
+ goto err;
set_capacity(d->disk, sectors);
snprintf(d->disk->disk_name, DISK_NAME_LEN, "bcache%i", idx);
@@ -834,6 +835,11 @@ static int bcache_device_init(struct bcache_device *d,
unsigned block_size,
blk_queue_write_cache(q, true, true);
return 0;
+
+err:
+ ida_simple_remove(_device_idx, idx);
+ return -ENOMEM;
+
}
/* Cached device */
--
2.16.4
[PATCH 0/3] bcache: Fix variable assignment in if condition in super.c
This patch set attempts to improve bcache's super.c code by moving the assignment of variables out of 'if' conditions. It does not address all occurences of this style issue (which is reported as error by checkpatch.pl). Florian Schmaus (3): bcache: do not assign in if condition register_bcache() bcache: do not assign in if condition in bcache_init() bcache: do not assign in if condition in bcache_device_init() drivers/md/bcache/super.c | 36 ++-- 1 file changed, 26 insertions(+), 10 deletions(-) -- 2.16.4
[PATCH 2/3] bcache: do not assign in if condition in bcache_init()
Fixes an error condition reported by checkpatch.pl which is caused by
assigning a variable in an if condition.
Signed-off-by: Florian Schmaus
---
drivers/md/bcache/super.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index db7177d422e5..f09f4f046315 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -2328,9 +2328,15 @@ static int __init bcache_init(void)
return bcache_major;
}
- if (!(bcache_wq = alloc_workqueue("bcache", WQ_MEM_RECLAIM, 0)) ||
- !(bcache_kobj = kobject_create_and_add("bcache", fs_kobj)) ||
- bch_request_init() ||
+ bcache_wq = alloc_workqueue("bcache", WQ_MEM_RECLAIM, 0);
+ if (!bcache_wq)
+ goto err;
+
+ bcache_kobj = kobject_create_and_add("bcache", fs_kobj);
+ if (!bcache_kobj)
+ goto err;
+
+ if (bch_request_init() ||
bch_debug_init(bcache_kobj) || closure_debug_init() ||
sysfs_create_files(bcache_kobj, files))
goto err;
--
2.16.4
[PATCH 1/3] bcache: do not assign in if condition register_bcache()
Fixes an error condition reported by checkpatch.pl which is caused by assigning a variable in an if condition. Signed-off-by: Florian Schmaus --- drivers/md/bcache/super.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c index fa4058e43202..db7177d422e5 100644 --- a/drivers/md/bcache/super.c +++ b/drivers/md/bcache/super.c @@ -2163,8 +2163,12 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr, if (!try_module_get(THIS_MODULE)) return -EBUSY; - if (!(path = kstrndup(buffer, size, GFP_KERNEL)) || - !(sb = kmalloc(sizeof(struct cache_sb), GFP_KERNEL))) + path = kstrndup(buffer, size, GFP_KERNEL); + if (!path) + goto err; + + sb = kmalloc(sizeof(struct cache_sb), GFP_KERNEL); + if (!sb) goto err; err = "failed to open device"; -- 2.16.4
[PATCH 2/3] bcache: do not assign in if condition in bcache_init()
Fixes an error condition reported by checkpatch.pl which is caused by
assigning a variable in an if condition.
Signed-off-by: Florian Schmaus
---
drivers/md/bcache/super.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index db7177d422e5..f09f4f046315 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -2328,9 +2328,15 @@ static int __init bcache_init(void)
return bcache_major;
}
- if (!(bcache_wq = alloc_workqueue("bcache", WQ_MEM_RECLAIM, 0)) ||
- !(bcache_kobj = kobject_create_and_add("bcache", fs_kobj)) ||
- bch_request_init() ||
+ bcache_wq = alloc_workqueue("bcache", WQ_MEM_RECLAIM, 0);
+ if (!bcache_wq)
+ goto err;
+
+ bcache_kobj = kobject_create_and_add("bcache", fs_kobj);
+ if (!bcache_kobj)
+ goto err;
+
+ if (bch_request_init() ||
bch_debug_init(bcache_kobj) || closure_debug_init() ||
sysfs_create_files(bcache_kobj, files))
goto err;
--
2.16.4
[PATCH 1/3] bcache: do not assign in if condition register_bcache()
Fixes an error condition reported by checkpatch.pl which is caused by assigning a variable in an if condition. Signed-off-by: Florian Schmaus --- drivers/md/bcache/super.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c index fa4058e43202..db7177d422e5 100644 --- a/drivers/md/bcache/super.c +++ b/drivers/md/bcache/super.c @@ -2163,8 +2163,12 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr, if (!try_module_get(THIS_MODULE)) return -EBUSY; - if (!(path = kstrndup(buffer, size, GFP_KERNEL)) || - !(sb = kmalloc(sizeof(struct cache_sb), GFP_KERNEL))) + path = kstrndup(buffer, size, GFP_KERNEL); + if (!path) + goto err; + + sb = kmalloc(sizeof(struct cache_sb), GFP_KERNEL); + if (!sb) goto err; err = "failed to open device"; -- 2.16.4
[PATCH 0/3] bcache: Fix variable assignment in if condition in super.c
This patch set attempts to improve bcache's super.c code by moving the assignment of variables out of 'if' conditions. It does not address all occurences of this style issue (which is reported as error by checkpatch.pl). Florian Schmaus (3): bcache: do not assign in if condition register_bcache() bcache: do not assign in if condition in bcache_init() bcache: do not assign in if condition in bcache_device_init() drivers/md/bcache/super.c | 36 ++-- 1 file changed, 26 insertions(+), 10 deletions(-) -- 2.16.4
[PATCH v6 3/3] driver-core: print bus registration error value
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 <f...@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
[PATCH v6 3/3] driver-core: print bus registration error value
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
---
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
[PATCH v6 2/3] driver-core: record error on bus registration
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 <f...@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 = >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(>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 2/3] driver-core: record error on bus registration
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
---
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 = >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(>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 1/3] driver-core: return EINVAL error instead of BUG_ON()
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 <f...@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 0/3] driver-core: return EINVAL error instead of BUG_ON()
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.gd20...@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()
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
---
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 0/3] driver-core: return EINVAL error instead of BUG_ON()
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.gd20...@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 v5 3/3] driver-core: print bus registration error value
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 <f...@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
[PATCH v5 3/3] driver-core: print bus registration error value
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
---
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
[PATCH v5 2/3] driver-core: record error on bus registration
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 <f...@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 = >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 2/3] driver-core: record error on bus registration
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
---
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 = >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 1/3] driver-core: return EINVAL error instead of BUG_ON()
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 <f...@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 1/3] driver-core: return EINVAL error instead of BUG_ON()
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
---
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 0/3] return EINVAL error instead of BUG_ON()
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 0/3] return EINVAL error instead of BUG_ON()
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
Re: [PATCH v4 1/3] driver-core: return EINVAL error instead of BUG_ON()
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 <f...@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
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v4 1/3] driver-core: return EINVAL error instead of BUG_ON()
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
>> ---
>>
>> 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
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v4 3/3] driver-core: print bus registration error value
On 16.05.2018 14:05, Florian Schmaus wrote:
> Signed-off-by: Florian Schmaus <f...@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 = >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
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v4 3/3] driver-core: print bus registration error value
On 16.05.2018 14:05, Florian Schmaus wrote:
> Signed-off-by: Florian Schmaus
> ---
>
> 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 = >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
signature.asc
Description: OpenPGP digital signature
[PATCH v4 1/3] driver-core: return EINVAL error instead of BUG_ON()
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 <f...@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 0/3] return EINVAL error instead of BUG_ON()
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()
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
---
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 0/3] return EINVAL error instead of BUG_ON()
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 2/3] driver-core: record error on bus registration
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 <f...@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 2/3] driver-core: record error on bus registration
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
---
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
Signed-off-by: Florian Schmaus <f...@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 = >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
[PATCH v4 3/3] driver-core: print bus registration error value
Signed-off-by: Florian Schmaus
---
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 = >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
[PATCH v3 1/3] driver-core: return EINVAL error instead of BUG_ON()
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 <f...@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 0/3] return EINVAL error instead of BUG_ON()
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()
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
---
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 0/3] return EINVAL error instead of BUG_ON()
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 2/3] driver-core: record error on bus registration
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 <f...@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 2/3] driver-core: record error on bus registration
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
---
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
Signed-off-by: Florian Schmaus <f...@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
[PATCH v3 3/3] driver-core: print bus registration error value
Signed-off-by: Florian Schmaus
---
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
[PATCH v2] driver-core: Return EBUSY error instead of BUG_ON()
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 <f...@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
[PATCH v2] driver-core: Return EBUSY error instead of BUG_ON()
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
---
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] driver-core: Log the BUG() causing driver
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 <f...@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
signature.asc
Description: OpenPGP digital signature
Re: [PATCH] driver-core: Log the BUG() causing driver
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
>> ---
>> 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
signature.asc
Description: OpenPGP digital signature
[PATCH] driver-core: Log the BUG() causing driver
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 <f...@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
[PATCH] driver-core: Log the BUG() causing driver
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
---
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
