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...@intel.com> > > Tested-by: Ross Zwisler <ross.zwis...@linux.intel.com> > > Cc: <sta...@vger.kernel.org> # 4.2+ > > Signed-off-by: Sudip Mukherjee <sudip.mukher...@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; };