Message ID | 20210709064341.6206-2-adrian.hunter@intel.com |
---|---|
State | New |
Headers | show |
Series | [V2,1/2] driver core: Add ability to delete device links of unregistered devices | expand |
On Fri, Jul 9, 2021 at 8:43 AM Adrian Hunter <adrian.hunter@intel.com> wrote: > > Managed device links are deleted by device_del(). However it is possible to > add a device link to a consumer before device_add(), and then discover an > error prevents the device from being used. In that case normally references > to the device would be dropped and the device would be deleted. However the > device link holds a reference to the device, so the device link and device > remain indefinitely. > > Amend device link removal to accept removal of a link with an > unregistered consumer device. > > To make that work nicely, the devlink_remove_symlinks() function must be > amended to cope with the absence of the consumer's sysfs presence, > otherwise sysfs_remove_link() will generate a warning. > > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Fixes: b294ff3e34490 ("scsi: ufs: core: Enable power management for wlun") > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/base/core.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index ea5b85354526..24bacdb315c6 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -562,7 +562,8 @@ static void devlink_remove_symlinks(struct device *dev, > struct device *con = link->consumer; > char *buf; > > - sysfs_remove_link(&link->link_dev.kobj, "consumer"); > + if (device_is_registered(con)) > + sysfs_remove_link(&link->link_dev.kobj, "consumer"); I think that this is needed regardless of the changes in device_link_put_kref(), because if somebody decides to delete a stateless device link before registering the consumer device, sysfs_remove_link() will still complain, won't it? > sysfs_remove_link(&link->link_dev.kobj, "supplier"); > > len = max(strlen(dev_bus_name(sup)) + strlen(dev_name(sup)), > @@ -575,8 +576,10 @@ static void devlink_remove_symlinks(struct device *dev, > return; > } > > - snprintf(buf, len, "supplier:%s:%s", dev_bus_name(sup), dev_name(sup)); > - sysfs_remove_link(&con->kobj, buf); > + if (device_is_registered(con)) { > + snprintf(buf, len, "supplier:%s:%s", dev_bus_name(sup), dev_name(sup)); > + sysfs_remove_link(&con->kobj, buf); > + } And here too, if I'm not mistaken. So in that case it would be better to put the above changes into a separate patch and add a Fixes tag to it. > snprintf(buf, len, "consumer:%s:%s", dev_bus_name(con), dev_name(con)); > sysfs_remove_link(&sup->kobj, buf); > kfree(buf); > @@ -885,6 +888,8 @@ static void device_link_put_kref(struct device_link *link) > { > if (link->flags & DL_FLAG_STATELESS) > kref_put(&link->kref, __device_link_del); > + else if (!device_is_registered(link->consumer)) > + __device_link_del(&link->kref); > else > WARN(1, "Unable to drop a managed device link reference\n"); > } > -- > 2.17.1 >
diff --git a/drivers/base/core.c b/drivers/base/core.c index ea5b85354526..24bacdb315c6 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -562,7 +562,8 @@ static void devlink_remove_symlinks(struct device *dev, struct device *con = link->consumer; char *buf; - sysfs_remove_link(&link->link_dev.kobj, "consumer"); + if (device_is_registered(con)) + sysfs_remove_link(&link->link_dev.kobj, "consumer"); sysfs_remove_link(&link->link_dev.kobj, "supplier"); len = max(strlen(dev_bus_name(sup)) + strlen(dev_name(sup)), @@ -575,8 +576,10 @@ static void devlink_remove_symlinks(struct device *dev, return; } - snprintf(buf, len, "supplier:%s:%s", dev_bus_name(sup), dev_name(sup)); - sysfs_remove_link(&con->kobj, buf); + if (device_is_registered(con)) { + snprintf(buf, len, "supplier:%s:%s", dev_bus_name(sup), dev_name(sup)); + sysfs_remove_link(&con->kobj, buf); + } snprintf(buf, len, "consumer:%s:%s", dev_bus_name(con), dev_name(con)); sysfs_remove_link(&sup->kobj, buf); kfree(buf); @@ -885,6 +888,8 @@ static void device_link_put_kref(struct device_link *link) { if (link->flags & DL_FLAG_STATELESS) kref_put(&link->kref, __device_link_del); + else if (!device_is_registered(link->consumer)) + __device_link_del(&link->kref); else WARN(1, "Unable to drop a managed device link reference\n"); }
Managed device links are deleted by device_del(). However it is possible to add a device link to a consumer before device_add(), and then discover an error prevents the device from being used. In that case normally references to the device would be dropped and the device would be deleted. However the device link holds a reference to the device, so the device link and device remain indefinitely. Amend device link removal to accept removal of a link with an unregistered consumer device. To make that work nicely, the devlink_remove_symlinks() function must be amended to cope with the absence of the consumer's sysfs presence, otherwise sysfs_remove_link() will generate a warning. Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Fixes: b294ff3e34490 ("scsi: ufs: core: Enable power management for wlun") Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/base/core.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)