* [PATCH v3] usb: uas: wait for pending commands before unbinding
@ 2026-10-27 13:14 Jiayi Li
2026-10-27 14:35 ` Oliver Neukum
0 siblings, 1 reply; 5+ messages in thread
From: Jiayi Li @ 2026-10-27 13:14 UTC (permalink / raw)
To: Oliver Neukum
Cc: Alan Stern, Michal Pecio, Greg Kroah-Hartman, linux-usb,
linux-scsi, usb-storage, linux-kernel, Jiayi Li
USB core normally disables interface endpoints before calling
uas_disconnect(), cancelling URBs while the device may still be
processing the associated commands. Some storage bridges may then
fail to recover after a driver rebind and require a disconnect and
reconnect.
Enable soft unbinding and give already accepted commands a bounded
grace period to complete before teardown. The disconnect sequence is:
-> cancel pending scan work and block new requests
-> stop software retries for accepted commands
-> wait for commands that can still complete normally
-> set resetting and unblock requests
-> cancel work and URBs, then clean up remaining commands
-> synchronize scan work and remove the host
Commands needing a retry are left for the existing cleanup, while
other accepted commands can still complete normally. Stop waiting
when there are no commands left to wait for, USB core marks the device
as not attached, or the grace period expires.
This does not abort tasks already accepted by the device. If a command
cannot finish during the grace period, an affected bridge may still
require a disconnect and reconnect to recover.
Signed-off-by: Jiayi Li <lijiayi@kylinos.cn>
---
Changes in v3:
- Stop software retries during disconnect, including retries triggered
by later READY IUs, and leave those commands for final cleanup.
- Replace flush_work() and anchor waits with command-table polling.
- Cancel pending scan work before blocking the host, while keeping
synchronous cancellation after command cleanup.
v2: https://lore.kernel.org/linux-usb/20261023013931.388916-1-lijiayi@kylinos.cn/
v1: https://lore.kernel.org/linux-usb/20261022123401.319479-1-lijiayi@kylinos.cn/
drivers/usb/storage/uas.c | 68 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 67 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 8655edbd66b168b34ff793509457e7f921028f17..63a0ac515737e4782bba50aa2bc2b54ee8282acd 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>
@@ -62,6 +63,7 @@ enum {
DATA_OUT_URB_INFLIGHT = BIT(10),
COMMAND_ABORTED = BIT(11),
IS_IN_WORK_LIST = BIT(12),
+ COMMAND_NO_RETRY = BIT(13),
};
/* Overrides scsi_pointer */
@@ -119,6 +121,9 @@ static void uas_do_work(struct work_struct *work)
if (!(cmdinfo->state & IS_IN_WORK_LIST))
continue;
+ if (cmdinfo->state & COMMAND_NO_RETRY)
+ continue;
+
err = uas_submit_urbs(cmnd, cmnd->device->hostdata);
if (!err)
cmdinfo->state &= ~IS_IN_WORK_LIST;
@@ -147,7 +152,8 @@ static void uas_add_work(struct scsi_cmnd *cmnd)
lockdep_assert_held(&devinfo->lock);
cmdinfo->state |= IS_IN_WORK_LIST;
- queue_work(workqueue, &devinfo->work);
+ if (!(cmdinfo->state & COMMAND_NO_RETRY))
+ queue_work(workqueue, &devinfo->work);
}
static void uas_zap_pending(struct uas_dev_info *devinfo, int result)
@@ -265,6 +271,10 @@ static void uas_xfer_data(struct urb *urb, struct scsi_cmnd *cmnd,
struct uas_cmd_info *cmdinfo = scsi_cmd_priv(cmnd);
int err;
+ if ((cmdinfo->state & (COMMAND_NO_RETRY | IS_IN_WORK_LIST)) ==
+ (COMMAND_NO_RETRY | IS_IN_WORK_LIST))
+ return;
+
cmdinfo->state |= direction | SUBMIT_STATUS_URB;
err = uas_submit_urbs(cmnd, cmnd->device->hostdata);
if (err) {
@@ -1211,16 +1221,71 @@ 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;
+ struct uas_cmd_info *cmdinfo;
+ unsigned long flags, start_time;
+ int i;
+
+ spin_lock_irqsave(&devinfo->lock, flags);
+ for (i = 0; i < devinfo->qdepth; i++) {
+ if (!devinfo->cmnd[i])
+ continue;
+ cmdinfo = scsi_cmd_priv(devinfo->cmnd[i]);
+ cmdinfo->state |= COMMAND_NO_RETRY;
+ }
+ spin_unlock_irqrestore(&devinfo->lock, flags);
+
+ start_time = jiffies;
+ for (;;) {
+ if (READ_ONCE(devinfo->udev->state) == USB_STATE_NOTATTACHED)
+ return;
+
+ spin_lock_irqsave(&devinfo->lock, flags);
+ for (i = 0; i < devinfo->qdepth; i++) {
+ if (!devinfo->cmnd[i])
+ continue;
+ cmdinfo = scsi_cmd_priv(devinfo->cmnd[i]);
+ if ((cmdinfo->state &
+ (COMMAND_NO_RETRY | IS_IN_WORK_LIST)) !=
+ (COMMAND_NO_RETRY | IS_IN_WORK_LIST))
+ break;
+ }
+ spin_unlock_irqrestore(&devinfo->lock, flags);
+
+ if (i >= devinfo->qdepth)
+ return;
+ if (time_after(jiffies, start_time + 5 * HZ))
+ break;
+
+ 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;
+ cancel_work(&devinfo->scan_work);
+
+ spin_lock_irqsave(&shost->host_lock, flags);
+ scsi_block_requests(shost);
+ spin_unlock_irqrestore(&shost->host_lock, flags);
+
+ uas_pre_disconnect(intf);
+
spin_lock_irqsave(&devinfo->lock, flags);
devinfo->resetting = 1;
spin_unlock_irqrestore(&devinfo->lock, flags);
+ 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 +1325,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,
base-commit: f1ac44c32cba88ad65dcecd4a279661d14e93651
--
2.53.0.windows.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: uas: wait for pending commands before unbinding
2026-10-27 13:14 [PATCH v3] usb: uas: wait for pending commands before unbinding Jiayi Li
@ 2026-10-27 14:35 ` Oliver Neukum
2026-10-28 2:09 ` Jiayi Li
0 siblings, 1 reply; 5+ messages in thread
From: Oliver Neukum @ 2026-10-27 14:35 UTC (permalink / raw)
To: Jiayi Li, Oliver Neukum
Cc: Alan Stern, Michal Pecio, Greg Kroah-Hartman, linux-usb,
linux-scsi, usb-storage, linux-kernel
Hi,
On 27.10.26 14:14, Jiayi Li wrote:
> USB core normally disables interface endpoints before calling
> uas_disconnect(), cancelling URBs while the device may still be
> processing the associated commands. Some storage bridges may then
> fail to recover after a driver rebind and require a disconnect and
> reconnect.
[..]
I regret torturing you again, but there is a simple issue left.
> 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;
>
> + cancel_work(&devinfo->scan_work);
No _sync
> + spin_lock_irqsave(&shost->host_lock, flags);
> + scsi_block_requests(shost);
> + spin_unlock_irqrestore(&shost->host_lock, flags);
> +
> + uas_pre_disconnect(intf);
> +
> spin_lock_irqsave(&devinfo->lock, flags);
> devinfo->resetting = 1;
> spin_unlock_irqrestore(&devinfo->lock, flags);
>
> + scsi_unblock_requests(shost);
> +
> cancel_work_sync(&devinfo->work);
With _sync
I'd say both ought to use cancel_work_sync()
Regards
Oliver
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: uas: wait for pending commands before unbinding
2026-10-27 14:35 ` Oliver Neukum
@ 2026-10-28 2:09 ` Jiayi Li
2026-10-28 8:48 ` Oliver Neukum
0 siblings, 1 reply; 5+ messages in thread
From: Jiayi Li @ 2026-10-28 2:09 UTC (permalink / raw)
To: Oliver Neukum
Cc: Alan Stern, Michal Pecio, Greg Kroah-Hartman, linux-usb,
linux-scsi, usb-storage, linux-kernel
Hi,
No worries, and thank you for taking another look.
On Tue, 27 Oct 2026 at 15:35:34 +0100, Oliver Neukum wrote:
> I'd say both ought to use cancel_work_sync()
The physical disconnect case is why I kept the first call
non-synchronous.
USB core has already terminated the interface URBs, but a running
synchronous scan may still be waiting for SCSI command completion.
Waiting for scan_work at the beginning of uas_disconnect() would
therefore delay the command cleanup below.
In testing, using cancel_work_sync() at the beginning waited for
about 11 seconds before reaching uas_pre_disconnect().
I would therefore keep cancel_work() at the beginning and synchronize
scan_work after the command cleanup.
Thanks,
Jiayi
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: uas: wait for pending commands before unbinding
2026-10-28 2:09 ` Jiayi Li
@ 2026-10-28 8:48 ` Oliver Neukum
2026-10-28 9:21 ` Jiayi Li
0 siblings, 1 reply; 5+ messages in thread
From: Oliver Neukum @ 2026-10-28 8:48 UTC (permalink / raw)
To: Jiayi Li
Cc: Alan Stern, Michal Pecio, Greg Kroah-Hartman, linux-usb,
linux-scsi, usb-storage, linux-kernel
On 28.10.26 03:09, Jiayi Li wrote:
Hi,
> USB core has already terminated the interface URBs, but a running
> synchronous scan may still be waiting for SCSI command completion.
> Waiting for scan_work at the beginning of uas_disconnect() would
> therefore delay the command cleanup below.
Why? What is it waiting for? And why do you think we can skip
that wait safely?
Regards
Oliver
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: uas: wait for pending commands before unbinding
2026-10-28 8:48 ` Oliver Neukum
@ 2026-10-28 9:21 ` Jiayi Li
0 siblings, 0 replies; 5+ messages in thread
From: Jiayi Li @ 2026-10-28 9:21 UTC (permalink / raw)
To: Oliver Neukum
Cc: Alan Stern, Michal Pecio, Greg Kroah-Hartman, linux-usb,
linux-scsi, usb-storage, linux-kernel
Hi,
On Wed, 28 Oct 2026 at 09:48:15 +0100, Oliver Neukum wrote:
> Why? What is it waiting for? And why do you think we can skip
> that wait safely?
The wait is not skipped; it is deferred until after the command
cleanup.
uas_scan_work() calls scsi_scan_host(). In the synchronous scan path,
it may be blocked in scsi_execute_cmd() waiting for the current scan
command, such as INQUIRY or REPORT LUNS, to complete.
On a physical disconnect, USB core disables the interface before
calling uas_disconnect(). The status URB may then complete with
-ESHUTDOWN, and uas_stat_cmplt() exits without clearing
COMMAND_INFLIGHT or completing the SCSI command.
Waiting for scan_work before reaching that cleanup can leave the scan
waiting for SCSI timeout/EH instead.
The first cancel_work() only prevents scan_work from starting if it is
still pending. If scan_work is already running, disconnect proceeds to
cancel the URBs and call uas_zap_pending(), which completes the
remaining command with DID_NO_CONNECT.
The existing cancel_work_sync() is then called after that cleanup and
before scsi_remove_host() and resource release. Therefore the running
scan is still synchronized; the wait is moved after the operation
which allows the scan command to finish, rather than omitted.
Thanks,
Jiayi
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-28 9:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-27 13:14 [PATCH v3] usb: uas: wait for pending commands before unbinding Jiayi Li
2026-10-27 14:35 ` Oliver Neukum
2026-10-28 2:09 ` Jiayi Li
2026-10-28 8:48 ` Oliver Neukum
2026-10-28 9:21 ` Jiayi Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox