linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 0/3] driver core: Add ability to delete device links of unregistered devices
@ 2021-07-10 10:38 Adrian Hunter
  2021-07-10 10:38 ` [PATCH V3 1/3] driver core: Prevent warning when removing a device link from unregistered consumer Adrian Hunter
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Adrian Hunter @ 2021-07-10 10:38 UTC (permalink / raw)
  To: Rafael J . Wysocki
  Cc: Greg Kroah-Hartman, Saravana Kannan, Martin K . Petersen,
	James E . J . Bottomley, linux-scsi, Avri Altman, Bean Huo,
	Can Guo, Asutosh Das, Bart Van Assche, linux-pm, linux-kernel

Hi

There is an issue with the SCSI UFS driver when the optional
BOOT well-known LUN fails to probe, which is not a fatal error.
The issue is that the device and its "managed" device link do not
then get deleted.  The device because the device link has a
reference to it.  The device link because it can only be deleted
by device_del(), but device_add() was never called, so device_del()
never will be either.

Since V2, these patches fix the issue by amending device link removal to
accept removal of a link with an unregistered consumer device, as suggested
by Rafael.


Changes in V3:

    driver core: Prevent warning when removing a device link from unregistered consumer
	New patch split from "driver core: Add ability to delete device
	links of unregistered devices" except first chunk from that patch
	dropped as unnecessary

    driver core: Add ability to delete device links of unregistered devices
	Move warning fix to separate patch.


Changes in V2:

    Take approach suggested by Rafael


Adrian Hunter (3):
      driver core: Prevent warning when removing a device link from unregistered consumer
      driver core: Add ability to delete device links of unregistered devices
      scsi: ufshcd: Fix device links when BOOT WLUN fails to probe

 drivers/base/core.c       |  8 ++++++--
 drivers/scsi/ufs/ufshcd.c | 23 +++++++++++++++++++++--
 2 files changed, 27 insertions(+), 4 deletions(-)


Regards
Adrian

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH V3 1/3] driver core: Prevent warning when removing a device link from unregistered consumer
  2021-07-10 10:38 [PATCH V3 0/3] driver core: Add ability to delete device links of unregistered devices Adrian Hunter
@ 2021-07-10 10:38 ` Adrian Hunter
  2021-07-10 15:19   ` Greg Kroah-Hartman
  2021-07-14 18:22   ` Rafael J. Wysocki
  2021-07-10 10:38 ` [PATCH V3 2/3] driver core: Add ability to delete device links of unregistered devices Adrian Hunter
  2021-07-10 10:38 ` [PATCH V3 3/3] scsi: ufshcd: Fix device links when BOOT WLUN fails to probe Adrian Hunter
  2 siblings, 2 replies; 8+ messages in thread
From: Adrian Hunter @ 2021-07-10 10:38 UTC (permalink / raw)
  To: Rafael J . Wysocki
  Cc: Greg Kroah-Hartman, Saravana Kannan, Martin K . Petersen,
	James E . J . Bottomley, linux-scsi, Avri Altman, Bean Huo,
	Can Guo, Asutosh Das, Bart Van Assche, linux-pm, linux-kernel

sysfs_remove_link() causes a warning if the parent directory does not
exist. That can happen if the device link consumer has not been registered.
So do not attempt sysfs_remove_link() in that case.

Fixes: 287905e68dd29 ("driver core: Expose device link details in sysfs")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/base/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index ea5b85354526..2de8f7d8cf54 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -575,8 +575,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);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH V3 2/3] driver core: Add ability to delete device links of unregistered devices
  2021-07-10 10:38 [PATCH V3 0/3] driver core: Add ability to delete device links of unregistered devices Adrian Hunter
  2021-07-10 10:38 ` [PATCH V3 1/3] driver core: Prevent warning when removing a device link from unregistered consumer Adrian Hunter
@ 2021-07-10 10:38 ` Adrian Hunter
  2021-07-14 18:26   ` Rafael J. Wysocki
  2021-07-10 10:38 ` [PATCH V3 3/3] scsi: ufshcd: Fix device links when BOOT WLUN fails to probe Adrian Hunter
  2 siblings, 1 reply; 8+ messages in thread
From: Adrian Hunter @ 2021-07-10 10:38 UTC (permalink / raw)
  To: Rafael J . Wysocki
  Cc: Greg Kroah-Hartman, Saravana Kannan, Martin K . Petersen,
	James E . J . Bottomley, linux-scsi, Avri Altman, Bean Huo,
	Can Guo, Asutosh Das, Bart Van Assche, linux-pm, linux-kernel

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 (unless the supplier is deleted).

Amend device link removal to accept removal of a link with an
unregistered consumer device.

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 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 2de8f7d8cf54..983e895d4ced 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -887,6 +887,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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH V3 3/3] scsi: ufshcd: Fix device links when BOOT WLUN fails to probe
  2021-07-10 10:38 [PATCH V3 0/3] driver core: Add ability to delete device links of unregistered devices Adrian Hunter
  2021-07-10 10:38 ` [PATCH V3 1/3] driver core: Prevent warning when removing a device link from unregistered consumer Adrian Hunter
  2021-07-10 10:38 ` [PATCH V3 2/3] driver core: Add ability to delete device links of unregistered devices Adrian Hunter
@ 2021-07-10 10:38 ` Adrian Hunter
  2 siblings, 0 replies; 8+ messages in thread
From: Adrian Hunter @ 2021-07-10 10:38 UTC (permalink / raw)
  To: Rafael J . Wysocki
  Cc: Greg Kroah-Hartman, Saravana Kannan, Martin K . Petersen,
	James E . J . Bottomley, linux-scsi, Avri Altman, Bean Huo,
	Can Guo, Asutosh Das, Bart Van Assche, linux-pm, linux-kernel

If a LUN fails to probe (e.g. absent BOOT WLUN), the device will not have
been registered but can still have a device link holding a reference to the
device. The unwanted device link will prevent runtime suspend indefinitely,
and cause some warnings if the supplier is ever deleted (e.g. by unbinding
the UFS host controller). Fix by explicitly deleting the device link when
SCSI destroys the SCSI device.

Fixes: b294ff3e34490 ("scsi: ufs: core: Enable power management for wlun")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/scsi/ufs/ufshcd.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 708b3b62fc4d..9864a8ee0263 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5020,15 +5020,34 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
 static void ufshcd_slave_destroy(struct scsi_device *sdev)
 {
 	struct ufs_hba *hba;
+	unsigned long flags;
 
 	hba = shost_priv(sdev->host);
 	/* Drop the reference as it won't be needed anymore */
 	if (ufshcd_scsi_to_upiu_lun(sdev->lun) == UFS_UPIU_UFS_DEVICE_WLUN) {
-		unsigned long flags;
-
 		spin_lock_irqsave(hba->host->host_lock, flags);
 		hba->sdev_ufs_device = NULL;
 		spin_unlock_irqrestore(hba->host->host_lock, flags);
+	} else if (hba->sdev_ufs_device) {
+		struct device *supplier = NULL;
+
+		/* Ensure UFS Device WLUN exists and does not disappear */
+		spin_lock_irqsave(hba->host->host_lock, flags);
+		if (hba->sdev_ufs_device) {
+			supplier = &hba->sdev_ufs_device->sdev_gendev;
+			get_device(supplier);
+		}
+		spin_unlock_irqrestore(hba->host->host_lock, flags);
+
+		if (supplier) {
+			/*
+			 * If a LUN fails to probe (e.g. absent BOOT WLUN), the
+			 * device will not have been registered but can still
+			 * have a device link holding a reference to the device.
+			 */
+			device_link_remove(&sdev->sdev_gendev, supplier);
+			put_device(supplier);
+		}
 	}
 }
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3 1/3] driver core: Prevent warning when removing a device link from unregistered consumer
  2021-07-10 10:38 ` [PATCH V3 1/3] driver core: Prevent warning when removing a device link from unregistered consumer Adrian Hunter
@ 2021-07-10 15:19   ` Greg Kroah-Hartman
  2021-07-14 18:21     ` Rafael J. Wysocki
  2021-07-14 18:22   ` Rafael J. Wysocki
  1 sibling, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2021-07-10 15:19 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Rafael J . Wysocki, Saravana Kannan, Martin K . Petersen,
	James E . J . Bottomley, linux-scsi, Avri Altman, Bean Huo,
	Can Guo, Asutosh Das, Bart Van Assche, linux-pm, linux-kernel

On Sat, Jul 10, 2021 at 01:38:17PM +0300, Adrian Hunter wrote:
> sysfs_remove_link() causes a warning if the parent directory does not
> exist. That can happen if the device link consumer has not been registered.
> So do not attempt sysfs_remove_link() in that case.
> 
> Fixes: 287905e68dd29 ("driver core: Expose device link details in sysfs")
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  drivers/base/core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

No Cc: stable for this?  Why not?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3 1/3] driver core: Prevent warning when removing a device link from unregistered consumer
  2021-07-10 15:19   ` Greg Kroah-Hartman
@ 2021-07-14 18:21     ` Rafael J. Wysocki
  0 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2021-07-14 18:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Adrian Hunter, Rafael J . Wysocki, Saravana Kannan,
	Martin K . Petersen, James E . J . Bottomley,
	open list:TARGET SUBSYSTEM, Avri Altman, Bean Huo, Can Guo,
	Asutosh Das, Bart Van Assche, Linux PM,
	Linux Kernel Mailing List

On Sat, Jul 10, 2021 at 5:19 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sat, Jul 10, 2021 at 01:38:17PM +0300, Adrian Hunter wrote:
> > sysfs_remove_link() causes a warning if the parent directory does not
> > exist. That can happen if the device link consumer has not been registered.
> > So do not attempt sysfs_remove_link() in that case.
> >
> > Fixes: 287905e68dd29 ("driver core: Expose device link details in sysfs")
> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > ---
> >  drivers/base/core.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
>
> No Cc: stable for this?  Why not?

AFAICS that would be

Cc: 5.9+ <stable@vger.kernel.org> # 5.9+

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3 1/3] driver core: Prevent warning when removing a device link from unregistered consumer
  2021-07-10 10:38 ` [PATCH V3 1/3] driver core: Prevent warning when removing a device link from unregistered consumer Adrian Hunter
  2021-07-10 15:19   ` Greg Kroah-Hartman
@ 2021-07-14 18:22   ` Rafael J. Wysocki
  1 sibling, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2021-07-14 18:22 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Rafael J . Wysocki, Greg Kroah-Hartman, Saravana Kannan,
	Martin K . Petersen, James E . J . Bottomley,
	open list:TARGET SUBSYSTEM, Avri Altman, Bean Huo, Can Guo,
	Asutosh Das, Bart Van Assche, Linux PM,
	Linux Kernel Mailing List

On Sat, Jul 10, 2021 at 12:38 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> sysfs_remove_link() causes a warning if the parent directory does not
> exist. That can happen if the device link consumer has not been registered.
> So do not attempt sysfs_remove_link() in that case.
>
> Fixes: 287905e68dd29 ("driver core: Expose device link details in sysfs")
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Reviewed-by: Rafael J. Wysocki <rafael@kernel.org>

> ---
>  drivers/base/core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index ea5b85354526..2de8f7d8cf54 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -575,8 +575,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);
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3 2/3] driver core: Add ability to delete device links of unregistered devices
  2021-07-10 10:38 ` [PATCH V3 2/3] driver core: Add ability to delete device links of unregistered devices Adrian Hunter
@ 2021-07-14 18:26   ` Rafael J. Wysocki
  0 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2021-07-14 18:26 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Rafael J . Wysocki, Greg Kroah-Hartman, Saravana Kannan,
	Martin K . Petersen, James E . J . Bottomley,
	open list:TARGET SUBSYSTEM, Avri Altman, Bean Huo, Can Guo,
	Asutosh Das, Bart Van Assche, Linux PM,
	Linux Kernel Mailing List

On Sat, Jul 10, 2021 at 12:38 PM 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

I would say "discovering an error"

> 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 (unless the supplier is deleted).
>
> Amend device link removal to accept removal of a link with an
> unregistered consumer device.
>
> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Fixes: b294ff3e34490 ("scsi: ufs: core: Enable power management for wlun")

This patch by itself doesn't fix the above one, so the tag doesn't
seem to be suitable here.

Frankly, I would consider folding it into the third patch in the
series in which case please feel free to add

Reviewed-by: Rafael J. Wysocki <rafael@kernel.org>

to the resulting combo patch.

> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  drivers/base/core.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 2de8f7d8cf54..983e895d4ced 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -887,6 +887,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
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-07-14 18:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-10 10:38 [PATCH V3 0/3] driver core: Add ability to delete device links of unregistered devices Adrian Hunter
2021-07-10 10:38 ` [PATCH V3 1/3] driver core: Prevent warning when removing a device link from unregistered consumer Adrian Hunter
2021-07-10 15:19   ` Greg Kroah-Hartman
2021-07-14 18:21     ` Rafael J. Wysocki
2021-07-14 18:22   ` Rafael J. Wysocki
2021-07-10 10:38 ` [PATCH V3 2/3] driver core: Add ability to delete device links of unregistered devices Adrian Hunter
2021-07-14 18:26   ` Rafael J. Wysocki
2021-07-10 10:38 ` [PATCH V3 3/3] scsi: ufshcd: Fix device links when BOOT WLUN fails to probe Adrian Hunter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).