[PATCH] parport: register driver later

[PATCH] parport: register driver later

[Sudip Mukherjee]

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@intel.com>
Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: <stable@vger.kernel.org> # 4.2+ 
Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
---

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;
+
 		/* initialize common driver fields */
 		drv->driver.name = drv->name;
 		drv->driver.bus = &parport_bus_type;
-- 
1.9.1

Re: [PATCH] parport: register driver later

[Greg KH]

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@intel.com>
> Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: <stable@vger.kernel.org> # 4.2+ 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> ---
> 
> 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 need to rename 'p' to "do_not_touch_you_have_been_warned" or something
else...

thanks,

greg k-h

Re: [PATCH] parport: register driver later

[Sudip Mukherjee]

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@intel.com>
> > Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > Cc: <stable@vger.kernel.org> # 4.2+ 
> > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> > ---
> > 
> > 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;
 };
 

Re: [PATCH] parport: register driver later

[Greg KH]

On Sun, Mar 06, 2016 at 11:26:29PM +0530, Sudip Mukherjee wrote:
> 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@intel.com>
> > > Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > > Cc: <stable@vger.kernel.org> # 4.2+ 
> > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> > > ---
> > > 
> > > 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.

Thanks, that would be great.

> > 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?):

Heh, no, it was kind of a joke, there's probably a better way to do it
without making the driver core look like a mess...

thanks,

greg k-h