All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb: uas: wait for pending commands before unbinding
@ 2026-10-22 12:34 Jiayi Li
  2026-10-22 12:48 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jiayi Li @ 2026-10-22 12:34 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Alan Stern, Michal Pecio, Greg Kroah-Hartman, linux-usb,
	linux-scsi, usb-storage, linux-kernel

USB core normally disables interface endpoints before calling
uas_disconnect(). This cancels pending URBs while the device may still
be processing the associated commands. Some storage bridges then stop
responding after a driver rebind and need a disconnect and reconnect
to recover.

Enable soft unbinding and block new SCSI requests before waiting for
pending commands to finish. This keeps the endpoints available until
the wait completes. Stop waiting when USB core marks the device as
not attached, and continue with the existing cleanup on timeout.

Keep requests blocked when leaving the preparation step, including on
timeout. Set resetting before unblocking the host so that queued
requests cannot submit new URBs during teardown.

---
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 2651629..58a67d7 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/blkdev.h>
+#include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/module.h>
@@ -1211,16 +1212,55 @@ static int uas_reset_resume(struct usb_interface *intf)
 	return 0;
 }
 
+static void uas_pre_disconnect(struct usb_interface *intf)
+{
+	struct Scsi_Host *shost = usb_get_intfdata(intf);
+	struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata;
+	unsigned long flags, start_time;
+
+	spin_lock_irqsave(shost->host_lock, flags);
+	scsi_block_requests(shost);
+	spin_unlock_irqrestore(shost->host_lock, flags);
+
+	start_time = jiffies;
+	for (;;) {
+		if (READ_ONCE(devinfo->udev->state) == USB_STATE_NOTATTACHED)
+			return;
+
+		flush_work(&devinfo->work);
+
+		if (!usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000))
+			break;
+
+		if (!usb_wait_anchor_empty_timeout(&devinfo->data_urbs, 500))
+			break;
+
+		if (time_after(jiffies, start_time + 5 * HZ))
+			break;
+		if (uas_cmnd_list_empty(devinfo))
+			return;
+
+		msleep(20);
+	}
+
+	shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__);
+}
+
 static void uas_disconnect(struct usb_interface *intf)
 {
 	struct Scsi_Host *shost = usb_get_intfdata(intf);
 	struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata;
 	unsigned long flags;
 
+	uas_pre_disconnect(intf);
+
 	spin_lock_irqsave(&devinfo->lock, flags);
 	devinfo->resetting = 1;
 	spin_unlock_irqrestore(&devinfo->lock, flags);
 
+	if (shost->host_self_blocked)
+		scsi_unblock_requests(shost);
+
 	cancel_work_sync(&devinfo->work);
 	usb_kill_anchored_urbs(&devinfo->cmd_urbs);
 	usb_kill_anchored_urbs(&devinfo->sense_urbs);
@@ -1260,6 +1300,7 @@ static void uas_shutdown(struct usb_interface *intf)
 
 static struct usb_driver uas_driver = {
 	.name = "uas",
+	.soft_unbind = 1,
 	.probe = uas_probe,
 	.disconnect = uas_disconnect,
 	.pre_reset = uas_pre_reset,

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

* Re: [PATCH] usb: uas: wait for pending commands before unbinding
  2026-10-22 12:34 [PATCH] usb: uas: wait for pending commands before unbinding Jiayi Li
@ 2026-10-22 12:48 ` sashiko-bot
  2026-10-22 13:05 ` Greg Kroah-Hartman
  2026-10-22 13:29 ` Oliver Neukum
  2 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-22 12:48 UTC (permalink / raw)
  To: Jiayi Li; +Cc: linux-usb, linux-scsi

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] usb: uas: pass &shost->host_lock pointer in uas_pre_disconnect()
--

commit 8e251b22758d30ff0f9e6d2743bdddf6dd61fc28
Author: Jiayi Li <lijiayi@kylinos.cn>

usb: uas: wait for pending commands before unbinding

This commit enables soft_unbind on the uas driver and introduces
uas_pre_disconnect() to block new SCSI requests and wait for pending
commands and URBs to complete before unbinding.

> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 8655edbd66b16..4ece8f15bd170 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c

[ ... ]

> @@ -1211,16 +1212,55 @@ static int uas_reset_resume(struct usb_interface *intf)
>  	return 0;
>  }
>  
> +static void uas_pre_disconnect(struct usb_interface *intf)
> +{
> +	struct Scsi_Host *shost = usb_get_intfdata(intf);
> +	struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata;
> +	unsigned long flags, start_time;
> +
> +	spin_lock_irqsave(shost->host_lock, flags);
> +	scsi_block_requests(shost);
> +	spin_unlock_irqrestore(shost->host_lock, flags);

[Severity: Low]
In uas_pre_disconnect() in drivers/usb/storage/uas.c, shost->host_lock is
passed by value to spin_lock_irqsave() and spin_unlock_irqrestore(), which
causes a build failure when CONFIG_USB_UAS is enabled.

In struct Scsi_Host (include/scsi/scsi_host.h), host_lock is an embedded
spinlock_t rather than a pointer (changed in commit e83b47309f73 ("scsi:
core: Drop Scsi_Host.default_lock")):

    struct Scsi_Host {
        ...
        spinlock_t		host_lock;

Other callers in drivers/usb/storage/uas.c, such as uas_reset_resume(), pass
&shost->host_lock:

    spin_lock_irqsave(&shost->host_lock, flags);
    scsi_report_bus_reset(shost, 0);
    spin_unlock_irqrestore(&shost->host_lock, flags);

Should uas_pre_disconnect() pass &shost->host_lock here as well?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022123401.319479-1-lijiayi@kylinos.cn?part=1

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

* Re: [PATCH] usb: uas: wait for pending commands before unbinding
  2026-10-22 12:34 [PATCH] usb: uas: wait for pending commands before unbinding Jiayi Li
  2026-10-22 12:48 ` sashiko-bot
@ 2026-10-22 13:05 ` Greg Kroah-Hartman
  2026-10-22 13:29 ` Oliver Neukum
  2 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2026-10-22 13:05 UTC (permalink / raw)
  To: Jiayi Li
  Cc: Oliver Neukum, Alan Stern, Michal Pecio, linux-usb, linux-scsi,
	usb-storage, linux-kernel

On Thu, Oct 22, 2026 at 08:34:01PM +0800, Jiayi Li wrote:
> USB core normally disables interface endpoints before calling
> uas_disconnect(). This cancels pending URBs while the device may still
> be processing the associated commands. Some storage bridges then stop
> responding after a driver rebind and need a disconnect and reconnect
> to recover.
> 
> Enable soft unbinding and block new SCSI requests before waiting for
> pending commands to finish. This keeps the endpoints available until
> the wait completes. Stop waiting when USB core marks the device as
> not attached, and continue with the existing cleanup on timeout.
> 
> Keep requests blocked when leaving the preparation step, including on
> timeout. Set resetting before unblocking the host so that queued
> requests cannot submit new URBs during teardown.
> 
> ---
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 2651629..58a67d7 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <linux/blkdev.h>
> +#include <linux/delay.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  #include <linux/module.h>
> @@ -1211,16 +1212,55 @@ static int uas_reset_resume(struct usb_interface *intf)
>  	return 0;
>  }
>  
> +static void uas_pre_disconnect(struct usb_interface *intf)
> +{
> +	struct Scsi_Host *shost = usb_get_intfdata(intf);
> +	struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata;
> +	unsigned long flags, start_time;
> +
> +	spin_lock_irqsave(shost->host_lock, flags);
> +	scsi_block_requests(shost);
> +	spin_unlock_irqrestore(shost->host_lock, flags);
> +
> +	start_time = jiffies;
> +	for (;;) {
> +		if (READ_ONCE(devinfo->udev->state) == USB_STATE_NOTATTACHED)
> +			return;
> +
> +		flush_work(&devinfo->work);
> +
> +		if (!usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000))
> +			break;
> +
> +		if (!usb_wait_anchor_empty_timeout(&devinfo->data_urbs, 500))
> +			break;
> +
> +		if (time_after(jiffies, start_time + 5 * HZ))
> +			break;
> +		if (uas_cmnd_list_empty(devinfo))
> +			return;
> +
> +		msleep(20);
> +	}
> +
> +	shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__);
> +}
> +
>  static void uas_disconnect(struct usb_interface *intf)
>  {
>  	struct Scsi_Host *shost = usb_get_intfdata(intf);
>  	struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata;
>  	unsigned long flags;
>  
> +	uas_pre_disconnect(intf);
> +
>  	spin_lock_irqsave(&devinfo->lock, flags);
>  	devinfo->resetting = 1;
>  	spin_unlock_irqrestore(&devinfo->lock, flags);
>  
> +	if (shost->host_self_blocked)
> +		scsi_unblock_requests(shost);
> +
>  	cancel_work_sync(&devinfo->work);
>  	usb_kill_anchored_urbs(&devinfo->cmd_urbs);
>  	usb_kill_anchored_urbs(&devinfo->sense_urbs);
> @@ -1260,6 +1300,7 @@ static void uas_shutdown(struct usb_interface *intf)
>  
>  static struct usb_driver uas_driver = {
>  	.name = "uas",
> +	.soft_unbind = 1,
>  	.probe = uas_probe,
>  	.disconnect = uas_disconnect,
>  	.pre_reset = uas_pre_reset,

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch does not have a Signed-off-by: line.  Please read the
  kernel file, Documentation/process/submitting-patches.rst and resend
  it after adding that line.  Note, the line needs to be in the body of
  the email, before the patch, not at the bottom of the patch or in the
  email signature.


If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

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

* Re: [PATCH] usb: uas: wait for pending commands before unbinding
  2026-10-22 12:34 [PATCH] usb: uas: wait for pending commands before unbinding Jiayi Li
  2026-10-22 12:48 ` sashiko-bot
  2026-10-22 13:05 ` Greg Kroah-Hartman
@ 2026-10-22 13:29 ` Oliver Neukum
  2026-10-23  1:48   ` Jiayi Li
  2 siblings, 1 reply; 5+ messages in thread
From: Oliver Neukum @ 2026-10-22 13:29 UTC (permalink / raw)
  To: Jiayi Li
  Cc: Alan Stern, Michal Pecio, Greg Kroah-Hartman, linux-usb,
	linux-scsi, usb-storage, linux-kernel

Hi,

thank you for doing this.

On 22.10.26 14:34, Jiayi Li wrote:

> +static void uas_pre_disconnect(struct usb_interface *intf)
> +{
> +	struct Scsi_Host *shost = usb_get_intfdata(intf);
> +	struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata;
> +	unsigned long flags, start_time;
> +
> +	spin_lock_irqsave(shost->host_lock, flags);
> +	scsi_block_requests(shost);

No, on pure grounds of maintainability, please put the
blocking of the host in the direct handler for disconnect.
Unblocking in the caller something that was never blocked
would forever trip up anybody reading the code.

	Regards
		Oliver



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

* Re: [PATCH] usb: uas: wait for pending commands before unbinding
  2026-10-22 13:29 ` Oliver Neukum
@ 2026-10-23  1:48   ` Jiayi Li
  0 siblings, 0 replies; 5+ messages in thread
From: Jiayi Li @ 2026-10-23  1:48 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Alan Stern, Michal Pecio, Greg Kroah-Hartman, linux-usb,
	linux-scsi, usb-storage, linux-kernel

Hi Oliver,

> please put the blocking of the host in the direct handler for disconnect.

Thank you for the review. I have sent v2 with the host blocking moved
into uas_disconnect(), as you suggested.

Thanks,
Jiayi

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

end of thread, other threads:[~2026-10-23  1:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-22 12:34 [PATCH] usb: uas: wait for pending commands before unbinding Jiayi Li
2026-10-22 12:48 ` sashiko-bot
2026-10-22 13:05 ` Greg Kroah-Hartman
2026-10-22 13:29 ` Oliver Neukum
2026-10-23  1:48   ` Jiayi Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.