Re: [PATCH] parport: register driver later

From: Sudip Mukherjee
Date: Sun Mar 06 2016 - 13:02:23 EST

On Sat, Mar 05, 2016 at 12:19:32PM -0800, Greg KH wrote:
> On Fri, Mar 04, 2016 at 04:20:59PM +0530, Sudip Mukherjee wrote:
> > If the parport bus is not yet registered and any device using parallel
> > port tries to register with the bus we get a stackdump with a message
> > of Kernel bug.
> >
> > Reported-by: Fengguang Wu <fengguang.wu@xxxxxxxxx>
> > Tested-by: Ross Zwisler <ross.zwisler@xxxxxxxxxxxxxxx>
> > Cc: <stable@xxxxxxxxxxxxxxx> # 4.2+
> > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@xxxxxxxxxxxxxxx>
> > ---
> >
> > We should actually have some deferred probe here. But considering that
> > you will be closing your trees soon so a quick fix to solve the problem
> > for now. We will revisit this when we remove the old api (hopefully v4.7).
> >
> > drivers/parport/share.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/parport/share.c b/drivers/parport/share.c
> > index 3308427..176b2b6 100644
> > --- a/drivers/parport/share.c
> > +++ b/drivers/parport/share.c
> > @@ -273,6 +273,9 @@ int __parport_register_driver(struct parport_driver *drv, struct module *owner,
> > /* using device model */
> > int ret;
> >
> > + if (!parport_bus_type.p)
> > + return -EAGAIN;
>
> I really don't like it when busses poke into the driver-core
> internal-only structures like this. Why can't you have your own "have
> been registered" flag instead if you really need it? Don't rely on the
> driver core here to be doing this always this way, perhaps p could be
> NULL and it only is created later on somehow?

I saw that in i2c and spmi and followed. Sent you v2 for your review. I
will send a patch to remove the use of 'p' in those places.

>
> I need to rename 'p' to "do_not_touch_you_have_been_warned" or something
> else...

something like this (compile tested) ?
(do you want me to send a proper patch?):

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 6470eb8..4fa350e 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -40,7 +40,7 @@ static int __must_check bus_rescan_devices_helper(struct device *dev,
static struct bus_type *bus_get(struct bus_type *bus)
{
if (bus) {
- kset_get(&bus->p->subsys);
+ kset_get(&bus->do_not_touch_u_r_warned->subsys);
return bus;
}
return NULL;
@@ -49,7 +49,7 @@ static struct bus_type *bus_get(struct bus_type *bus)
static void bus_put(struct bus_type *bus)
{
if (bus)
- kset_put(&bus->p->subsys);
+ kset_put(&bus->do_not_touch_u_r_warned->subsys);
}

static ssize_t drv_attr_show(struct kobject *kobj, struct attribute *attr,
@@ -130,7 +130,9 @@ int bus_create_file(struct bus_type *bus, struct bus_attribute *attr)
{
int error;
if (bus_get(bus)) {
- error = sysfs_create_file(&bus->p->subsys.kobj, &attr->attr);
+ error = sysfs_create_file(&bus->
+ do_not_touch_u_r_warned->
+ subsys.kobj, &attr->attr);
bus_put(bus);
} else
error = -EINVAL;
@@ -141,7 +143,8 @@ EXPORT_SYMBOL_GPL(bus_create_file);
void bus_remove_file(struct bus_type *bus, struct bus_attribute *attr)
{
if (bus_get(bus)) {
- sysfs_remove_file(&bus->p->subsys.kobj, &attr->attr);
+ sysfs_remove_file(&bus->do_not_touch_u_r_warned->
+ subsys.kobj, &attr->attr);
bus_put(bus);
}
}
@@ -153,7 +156,7 @@ static void bus_release(struct kobject *kobj)
struct bus_type *bus = priv->bus;

kfree(priv);
- bus->p = NULL;
+ bus->do_not_touch_u_r_warned = NULL;
}

static struct kobj_type bus_ktype = {
@@ -237,16 +240,17 @@ static DRIVER_ATTR_WO(bind);

static ssize_t show_drivers_autoprobe(struct bus_type *bus, char *buf)
{
- return sprintf(buf, "%d\n", bus->p->drivers_autoprobe);
+ return sprintf(buf, "%d\n",
+ bus->do_not_touch_u_r_warned->drivers_autoprobe);
}

static ssize_t store_drivers_autoprobe(struct bus_type *bus,
const char *buf, size_t count)
{
if (buf[0] == '0')
- bus->p->drivers_autoprobe = 0;
+ bus->do_not_touch_u_r_warned->drivers_autoprobe = 0;
else
- bus->p->drivers_autoprobe = 1;
+ bus->do_not_touch_u_r_warned->drivers_autoprobe = 1;
return count;
}

@@ -304,10 +308,10 @@ int bus_for_each_dev(struct bus_type *bus, struct device *start,
struct device *dev;
int error = 0;

- if (!bus || !bus->p)
+ if (!bus || !bus->do_not_touch_u_r_warned)
return -EINVAL;

- klist_iter_init_node(&bus->p->klist_devices, &i,
+ klist_iter_init_node(&bus->do_not_touch_u_r_warned->klist_devices, &i,
(start ? &start->p->knode_bus : NULL));
while ((dev = next_device(&i)) && !error)
error = fn(dev, data);
@@ -338,10 +342,11 @@ struct device *bus_find_device(struct bus_type *bus,
struct klist_iter i;
struct device *dev;

- if (!bus || !bus->p)
+ if (!bus || !bus->do_not_touch_u_r_warned)
return NULL;

- klist_iter_init_node(&bus->p->klist_devices, &i,
+ klist_iter_init_node(&bus->do_not_touch_u_r_warned->
+ klist_devices, &i,
(start ? &start->p->knode_bus : NULL));
while ((dev = next_device(&i)))
if (match(dev, data) && get_device(dev))
@@ -395,7 +400,9 @@ struct device *subsys_find_device_by_id(struct bus_type *subsys, unsigned int id
return NULL;

if (hint) {
- klist_iter_init_node(&subsys->p->klist_devices, &i, &hint->p->knode_bus);
+ klist_iter_init_node(&subsys->do_not_touch_u_r_warned->
+ klist_devices, &i,
+ &hint->p->knode_bus);
dev = next_device(&i);
if (dev && dev->id == id && get_device(dev)) {
klist_iter_exit(&i);
@@ -404,7 +411,8 @@ struct device *subsys_find_device_by_id(struct bus_type *subsys, unsigned int id
klist_iter_exit(&i);
}

- klist_iter_init_node(&subsys->p->klist_devices, &i, NULL);
+ klist_iter_init_node(&subsys->do_not_touch_u_r_warned->
+ klist_devices, &i, NULL);
while ((dev = next_device(&i))) {
if (dev->id == id && get_device(dev)) {
klist_iter_exit(&i);
@@ -457,8 +465,8 @@ int bus_for_each_drv(struct bus_type *bus, struct device_driver *start,
if (!bus)
return -EINVAL;

- klist_iter_init_node(&bus->p->klist_drivers, &i,
- start ? &start->p->knode_bus : NULL);
+ klist_iter_init_node(&bus->do_not_touch_u_r_warned->klist_drivers,
+ &i, start ? &start->p->knode_bus : NULL);
while ((drv = next_driver(&i)) && !error)
error = fn(drv, data);
klist_iter_exit(&i);
@@ -516,20 +524,24 @@ int bus_add_device(struct device *dev)
error = device_add_groups(dev, bus->dev_groups);
if (error)
goto out_id;
- error = sysfs_create_link(&bus->p->devices_kset->kobj,
- &dev->kobj, dev_name(dev));
+ error = sysfs_create_link(&bus->do_not_touch_u_r_warned->
+ devices_kset->kobj, &dev->kobj,
+ dev_name(dev));
if (error)
goto out_groups;
error = sysfs_create_link(&dev->kobj,
- &dev->bus->p->subsys.kobj, "subsystem");
+ &dev->bus->do_not_touch_u_r_warned->
+ subsys.kobj, "subsystem");
if (error)
goto out_subsys;
- klist_add_tail(&dev->p->knode_bus, &bus->p->klist_devices);
+ klist_add_tail(&dev->p->knode_bus,
+ &bus->do_not_touch_u_r_warned->klist_devices);
}
return 0;

out_subsys:
- sysfs_remove_link(&bus->p->devices_kset->kobj, dev_name(dev));
+ sysfs_remove_link(&bus->do_not_touch_u_r_warned->devices_kset->kobj,
+ dev_name(dev));
out_groups:
device_remove_groups(dev, bus->dev_groups);
out_id:
@@ -553,14 +565,15 @@ void bus_probe_device(struct device *dev)
if (!bus)
return;

- if (bus->p->drivers_autoprobe)
+ if (bus->do_not_touch_u_r_warned->drivers_autoprobe)
device_initial_probe(dev);

- mutex_lock(&bus->p->mutex);
- list_for_each_entry(sif, &bus->p->interfaces, node)
+ mutex_lock(&bus->do_not_touch_u_r_warned->mutex);
+ list_for_each_entry(sif, &bus->do_not_touch_u_r_warned->interfaces,
+ node)
if (sif->add_dev)
sif->add_dev(dev, sif);
- mutex_unlock(&bus->p->mutex);
+ mutex_unlock(&bus->do_not_touch_u_r_warned->mutex);
}

/**
@@ -581,14 +594,15 @@ void bus_remove_device(struct device *dev)
if (!bus)
return;

- mutex_lock(&bus->p->mutex);
- list_for_each_entry(sif, &bus->p->interfaces, node)
+ mutex_lock(&bus->do_not_touch_u_r_warned->mutex);
+ list_for_each_entry(sif, &bus->do_not_touch_u_r_warned->interfaces,
+ node)
if (sif->remove_dev)
sif->remove_dev(dev, sif);
- mutex_unlock(&bus->p->mutex);
+ mutex_unlock(&bus->do_not_touch_u_r_warned->mutex);

sysfs_remove_link(&dev->kobj, "subsystem");
- sysfs_remove_link(&dev->bus->p->devices_kset->kobj,
+ sysfs_remove_link(&dev->bus->do_not_touch_u_r_warned->devices_kset->kobj,
dev_name(dev));
device_remove_attrs(dev->bus, dev);
device_remove_groups(dev, dev->bus->dev_groups);
@@ -691,14 +705,15 @@ int bus_add_driver(struct device_driver *drv)
klist_init(&priv->klist_devices, NULL, NULL);
priv->driver = drv;
drv->p = priv;
- priv->kobj.kset = bus->p->drivers_kset;
+ priv->kobj.kset = bus->do_not_touch_u_r_warned->drivers_kset;
error = kobject_init_and_add(&priv->kobj, &driver_ktype, NULL,
"%s", drv->name);
if (error)
goto out_unregister;

- klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers);
- if (drv->bus->p->drivers_autoprobe) {
+ klist_add_tail(&priv->knode_bus,
+ &bus->do_not_touch_u_r_warned->klist_drivers);
+ if (drv->bus->do_not_touch_u_r_warned->drivers_autoprobe) {
if (driver_allows_async_probing(drv)) {
pr_debug("bus: '%s': probing driver %s asynchronously\n",
drv->bus->name, drv->name);
@@ -840,13 +855,15 @@ struct bus_type *find_bus(char *name)
static int bus_add_groups(struct bus_type *bus,
const struct attribute_group **groups)
{
- return sysfs_create_groups(&bus->p->subsys.kobj, groups);
+ return sysfs_create_groups(&bus->do_not_touch_u_r_warned->subsys.kobj,
+ groups);
}

static void bus_remove_groups(struct bus_type *bus,
const struct attribute_group **groups)
{
- sysfs_remove_groups(&bus->p->subsys.kobj, groups);
+ sysfs_remove_groups(&bus->do_not_touch_u_r_warned->subsys.kobj,
+ groups);
}

static void klist_devices_get(struct klist_node *n)
@@ -871,7 +888,8 @@ static ssize_t bus_uevent_store(struct bus_type *bus,
enum kobject_action action;

if (kobject_action_type(buf, count, &action) == 0)
- kobject_uevent(&bus->p->subsys.kobj, action);
+ kobject_uevent(&bus->do_not_touch_u_r_warned->subsys.kobj,
+ action);
return count;
}
static BUS_ATTR(uevent, S_IWUSR, NULL, bus_uevent_store);
@@ -895,7 +913,7 @@ int bus_register(struct bus_type *bus)
return -ENOMEM;

priv->bus = bus;
- bus->p = priv;
+ bus->do_not_touch_u_r_warned = priv;

BLOCKING_INIT_NOTIFIER_HEAD(&priv->bus_notifier);

@@ -948,16 +966,16 @@ int bus_register(struct bus_type *bus)
bus_groups_fail:
remove_probe_files(bus);
bus_probe_files_fail:
- kset_unregister(bus->p->drivers_kset);
+ kset_unregister(bus->do_not_touch_u_r_warned->drivers_kset);
bus_drivers_fail:
- kset_unregister(bus->p->devices_kset);
+ kset_unregister(bus->do_not_touch_u_r_warned->devices_kset);
bus_devices_fail:
bus_remove_file(bus, &bus_attr_uevent);
bus_uevent_fail:
- kset_unregister(&bus->p->subsys);
+ kset_unregister(&bus->do_not_touch_u_r_warned->subsys);
out:
- kfree(bus->p);
- bus->p = NULL;
+ kfree(bus->do_not_touch_u_r_warned);
+ bus->do_not_touch_u_r_warned = NULL;
return retval;
}
EXPORT_SYMBOL_GPL(bus_register);
@@ -976,34 +994,37 @@ void bus_unregister(struct bus_type *bus)
device_unregister(bus->dev_root);
bus_remove_groups(bus, bus->bus_groups);
remove_probe_files(bus);
- kset_unregister(bus->p->drivers_kset);
- kset_unregister(bus->p->devices_kset);
+ kset_unregister(bus->do_not_touch_u_r_warned->drivers_kset);
+ kset_unregister(bus->do_not_touch_u_r_warned->devices_kset);
bus_remove_file(bus, &bus_attr_uevent);
- kset_unregister(&bus->p->subsys);
+ kset_unregister(&bus->do_not_touch_u_r_warned->subsys);
}
EXPORT_SYMBOL_GPL(bus_unregister);

int bus_register_notifier(struct bus_type *bus, struct notifier_block *nb)
{
- return blocking_notifier_chain_register(&bus->p->bus_notifier, nb);
+ return blocking_notifier_chain_register(&bus->do_not_touch_u_r_warned->
+ bus_notifier, nb);
}
EXPORT_SYMBOL_GPL(bus_register_notifier);

int bus_unregister_notifier(struct bus_type *bus, struct notifier_block *nb)
{
- return blocking_notifier_chain_unregister(&bus->p->bus_notifier, nb);
+ return blocking_notifier_chain_unregister(&bus->
+ do_not_touch_u_r_warned->
+ bus_notifier, nb);
}
EXPORT_SYMBOL_GPL(bus_unregister_notifier);

struct kset *bus_get_kset(struct bus_type *bus)
{
- return &bus->p->subsys;
+ return &bus->do_not_touch_u_r_warned->subsys;
}
EXPORT_SYMBOL_GPL(bus_get_kset);

struct klist *bus_get_device_klist(struct bus_type *bus)
{
- return &bus->p->klist_devices;
+ return &bus->do_not_touch_u_r_warned->klist_devices;
}
EXPORT_SYMBOL_GPL(bus_get_device_klist);

@@ -1076,7 +1097,8 @@ void subsys_dev_iter_init(struct subsys_dev_iter *iter, struct bus_type *subsys,

if (start)
start_knode = &start->p->knode_bus;
- klist_iter_init_node(&subsys->p->klist_devices, &iter->ki, start_knode);
+ klist_iter_init_node(&subsys->do_not_touch_u_r_warned->klist_devices,
+ &iter->ki, start_knode);
iter->type = type;
}
EXPORT_SYMBOL_GPL(subsys_dev_iter_init);
@@ -1135,15 +1157,16 @@ int subsys_interface_register(struct subsys_interface *sif)
if (!subsys)
return -EINVAL;

- mutex_lock(&subsys->p->mutex);
- list_add_tail(&sif->node, &subsys->p->interfaces);
+ mutex_lock(&subsys->do_not_touch_u_r_warned->mutex);
+ list_add_tail(&sif->node,
+ &subsys->do_not_touch_u_r_warned->interfaces);
if (sif->add_dev) {
subsys_dev_iter_init(&iter, subsys, NULL, NULL);
while ((dev = subsys_dev_iter_next(&iter)))
sif->add_dev(dev, sif);
subsys_dev_iter_exit(&iter);
}
- mutex_unlock(&subsys->p->mutex);
+ mutex_unlock(&subsys->do_not_touch_u_r_warned->mutex);

return 0;
}
@@ -1160,7 +1183,7 @@ void subsys_interface_unregister(struct subsys_interface *sif)

subsys = sif->subsys;

- mutex_lock(&subsys->p->mutex);
+ mutex_lock(&subsys->do_not_touch_u_r_warned->mutex);
list_del_init(&sif->node);
if (sif->remove_dev) {
subsys_dev_iter_init(&iter, subsys, NULL, NULL);
@@ -1168,7 +1191,7 @@ void subsys_interface_unregister(struct subsys_interface *sif)
sif->remove_dev(dev, sif);
subsys_dev_iter_exit(&iter);
}
- mutex_unlock(&subsys->p->mutex);
+ mutex_unlock(&subsys->do_not_touch_u_r_warned->mutex);

bus_put(subsys);
}
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 0a8bdad..c64c746 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1113,7 +1113,9 @@ int device_add(struct device *dev)
* after dpm_sysfs_add() and before kobject_uevent().
*/
if (dev->bus)
- blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
+ blocking_notifier_call_chain(&dev->bus->
+ do_not_touch_u_r_warned->
+ bus_notifier,
BUS_NOTIFY_ADD_DEVICE, dev);

kobject_uevent(&dev->kobj, KOBJ_ADD);
@@ -1238,7 +1240,9 @@ void device_del(struct device *dev)
* before dpm_sysfs_remove().
*/
if (dev->bus)
- blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
+ blocking_notifier_call_chain(&dev->bus->
+ do_not_touch_u_r_warned->
+ bus_notifier,
BUS_NOTIFY_DEL_DEVICE, dev);
dpm_sysfs_remove(dev);
if (parent)
@@ -1273,7 +1277,9 @@ void device_del(struct device *dev)
if (platform_notify_remove)
platform_notify_remove(dev);
if (dev->bus)
- blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
+ blocking_notifier_call_chain(&dev->bus->
+ do_not_touch_u_r_warned->
+ bus_notifier,
BUS_NOTIFY_REMOVED_DEVICE, dev);
kobject_uevent(&dev->kobj, KOBJ_REMOVE);
cleanup_device_parent(dev);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 16688f5..43baef6 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -260,7 +260,9 @@ static void driver_bound(struct device *dev)
driver_deferred_probe_trigger();

if (dev->bus)
- blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
+ blocking_notifier_call_chain(&dev->bus->
+ do_not_touch_u_r_warned->
+ bus_notifier,
BUS_NOTIFY_BOUND_DRIVER, dev);
}

@@ -269,7 +271,9 @@ static int driver_sysfs_add(struct device *dev)
int ret;

if (dev->bus)
- blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
+ blocking_notifier_call_chain(&dev->bus->
+ do_not_touch_u_r_warned->
+ bus_notifier,
BUS_NOTIFY_BIND_DRIVER, dev);

ret = sysfs_create_link(&dev->driver->p->kobj, &dev->kobj,
@@ -316,7 +320,9 @@ int device_bind_driver(struct device *dev)
if (!ret)
driver_bound(dev);
else if (dev->bus)
- blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
+ blocking_notifier_call_chain(&dev->bus->
+ do_not_touch_u_r_warned->
+ bus_notifier,
BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
return ret;
}
@@ -396,7 +402,9 @@ static int really_probe(struct device *dev, struct device_driver *drv)

probe_failed:
if (dev->bus)
- blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
+ blocking_notifier_call_chain(&dev->bus->
+ do_not_touch_u_r_warned->
+ bus_notifier,
BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
pinctrl_bind_failed:
devres_release_all(dev);
@@ -770,7 +778,9 @@ static void __device_release_driver(struct device *dev)
driver_sysfs_remove(dev);

if (dev->bus)
- blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
+ blocking_notifier_call_chain(&dev->bus->
+ do_not_touch_u_r_warned->
+ bus_notifier,
BUS_NOTIFY_UNBIND_DRIVER,
dev);

@@ -790,7 +800,9 @@ static void __device_release_driver(struct device *dev)
klist_remove(&dev->p->knode_driver);
device_pm_check_callbacks(dev);
if (dev->bus)
- blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
+ blocking_notifier_call_chain(&dev->bus->
+ do_not_touch_u_r_warned->
+ bus_notifier,
BUS_NOTIFY_UNBOUND_DRIVER,
dev);
}
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 4eabfe2..b8aa7cc 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -150,7 +150,7 @@ int driver_register(struct device_driver *drv)
int ret;
struct device_driver *other;

- BUG_ON(!drv->bus->p);
+ BUG_ON(!drv->bus->do_not_touch_u_r_warned);

if ((drv->bus->probe && drv->probe) ||
(drv->bus->remove && drv->remove) ||
@@ -210,7 +210,8 @@ EXPORT_SYMBOL_GPL(driver_unregister);
*/
struct device_driver *driver_find(const char *name, struct bus_type *bus)
{
- struct kobject *k = kset_find_obj(bus->p->drivers_kset, name);
+ struct kobject *k = kset_find_obj(bus->do_not_touch_u_r_warned->
+ drivers_kset, name);
struct driver_private *priv;

if (k) {
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index f437afa..cbfd005 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -689,12 +689,14 @@ int __init_or_module __platform_driver_probe(struct platform_driver *drv,
* if the probe was successful, and make sure any forced probes of
* new devices fail.
*/
- spin_lock(&drv->driver.bus->p->klist_drivers.k_lock);
+ spin_lock(&drv->driver.bus->do_not_touch_u_r_warned->
+ klist_drivers.k_lock);
drv->probe = NULL;
if (code == 0 && list_empty(&drv->driver.p->klist_devices.k_list))
retval = -ENODEV;
drv->driver.probe = platform_drv_probe_fail;
- spin_unlock(&drv->driver.bus->p->klist_drivers.k_lock);
+ spin_unlock(&drv->driver.bus->do_not_touch_u_r_warned->
+ klist_drivers.k_lock);

if (code != retval)
platform_driver_unregister(drv);
diff --git a/include/linux/device.h b/include/linux/device.h
index 002c597..fb73e9d 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -130,7 +130,7 @@ struct bus_type {

const struct iommu_ops *iommu_ops;

- struct subsys_private *p;
+ struct subsys_private *do_not_touch_u_r_warned;
struct lock_class_key lock_key;
};