[PATCH v2] usb: uas: quiesce SCSI before stopping endpoints on unbind

[PATCH v2] usb: uas: quiesce SCSI before stopping endpoints on unbind

[Jiayi Li]

Unbinding uas while READ commands are in flight can leave the storage
device unusable after the driver is rebound.  The first post-bind
INQUIRY Data-In transfer completes with -EOVERFLOW, and SCSI error
handling eventually offlines the device:

  scsi host7: uas
  scsi 7:0:0:0: tag#4 data cmplt err -75 uas-tag 1 inflight: CMD
  scsi 7:0:0:0: tag#4 CDB: Inquiry 12 00 00 00 24 00
  scsi 7:0:0:0: tag#4 uas_eh_abort_handler 0 uas-tag 1 inflight: CMD
  usb 2-2: reset SuperSpeed USB device number 2 using xhci_hcd
  scsi host7: uas_eh_device_reset_handler success
  ...
  scsi 7:0:0:0: tag#7 CDB: Test Unit Ready 00 00 00 00 00 00
  scsi host7: uas_eh_device_reset_handler success
  sd 7:0:0:0: Device offlined - not ready after error recovery

A command URB may already have delivered a SCSI command when usbcore
disables the interface endpoints and kills the data and status URBs
before ->disconnect.  uas_disconnect() then removes the SCSI host only
after killing its anchored URBs, so SCSI teardown cannot first quiesce
the accepted commands.  A newly bound UAS instance can encounter the
residual transport state.

The failure reproduced with a VIA Labs 2109:0715 storage bridge on both
Zhaoxin 1d17:9204 and Intel 8086:a2af xHCI controllers.  USB device reset
and xHCI unbind/rebind did not recover the device; physical reconnection
did.

Set soft_unbind so the endpoints remain available during driver unbind.
Cancel pending scanning and remove the SCSI host before setting resetting
and killing the anchored URBs.  Use the same teardown order for every
disconnect path and rely on SCSI host removal to handle a device that can
no longer communicate.

With the change, 10 of 10 zero-delay unbind/rebind iterations with 30
READ commands in flight reattached the disk and completed a post-bind
O_DIRECT read.  Unbind took 82 to 109 ms, with no UAS completion error or
command timeout after rebind.

Signed-off-by: Jiayi Li <lijiayi@kylinos.cn>
---
Changes in v2:
- Remove the USB_STATE_NOTATTACHED-based disconnect classification.
- Use the same teardown ordering for all disconnect paths.
- Reword the commit message.

Link: https://lore.kernel.org/lkml/20260920012358.3362053-1-lijiayi@kylinos.cn/

 drivers/usb/storage/uas.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 8655edbd66b16..8752cecb45915 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -1217,6 +1217,14 @@ static void uas_disconnect(struct usb_interface *intf)
 	struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata;
 	unsigned long flags;
 
+	/*
+	 * Prevent SCSI scanning (if it hasn't started yet)
+	 * or wait for the SCSI-scanning routine to stop.
+	 */
+	cancel_work_sync(&devinfo->scan_work);
+
+	scsi_remove_host(shost);
+
 	spin_lock_irqsave(&devinfo->lock, flags);
 	devinfo->resetting = 1;
 	spin_unlock_irqrestore(&devinfo->lock, flags);
@@ -1227,13 +1235,6 @@ static void uas_disconnect(struct usb_interface *intf)
 	usb_kill_anchored_urbs(&devinfo->data_urbs);
 	uas_zap_pending(devinfo, DID_NO_CONNECT);
 
-	/*
-	 * Prevent SCSI scanning (if it hasn't started yet)
-	 * or wait for the SCSI-scanning routine to stop.
-	 */
-	cancel_work_sync(&devinfo->scan_work);
-
-	scsi_remove_host(shost);
 	uas_free_streams(devinfo);
 	scsi_host_put(shost);
 }
@@ -1267,6 +1268,7 @@ static struct usb_driver uas_driver = {
 	.suspend = uas_suspend,
 	.resume = uas_resume,
 	.reset_resume = uas_reset_resume,
+	.soft_unbind = 1,
 	.shutdown = uas_shutdown,
 	.id_table = uas_usb_ids,
 };

Re: [PATCH v2] usb: uas: quiesce SCSI before stopping endpoints on unbind

[Oliver Neukum]

On 22.09.26 05:11, Jiayi Li wrote:
> Unbinding uas while READ commands are in flight can leave the storage
> device unusable after the driver is rebound.  The first post-bind
> INQUIRY Data-In transfer completes with -EOVERFLOW, and SCSI error
> handling eventually offlines the device:
> 
>    scsi host7: uas
>    scsi 7:0:0:0: tag#4 data cmplt err -75 uas-tag 1 inflight: CMD
>    scsi 7:0:0:0: tag#4 CDB: Inquiry 12 00 00 00 24 00
>    scsi 7:0:0:0: tag#4 uas_eh_abort_handler 0 uas-tag 1 inflight: CMD
>    usb 2-2: reset SuperSpeed USB device number 2 using xhci_hcd
>    scsi host7: uas_eh_device_reset_handler success
>    ...
>    scsi 7:0:0:0: tag#7 CDB: Test Unit Ready 00 00 00 00 00 00
>    scsi host7: uas_eh_device_reset_handler success
>    sd 7:0:0:0: Device offlined - not ready after error recovery
> 
> A command URB may already have delivered a SCSI command when usbcore
> disables the interface endpoints and kills the data and status URBs
> before ->disconnect.  uas_disconnect() then removes the SCSI host only
> after killing its anchored URBs, so SCSI teardown cannot first quiesce
> the accepted commands.  A newly bound UAS instance can encounter the
> residual transport state.
> 
> The failure reproduced with a VIA Labs 2109:0715 storage bridge on both
> Zhaoxin 1d17:9204 and Intel 8086:a2af xHCI controllers.  USB device reset
> and xHCI unbind/rebind did not recover the device; physical reconnection
> did.
> 
> Set soft_unbind so the endpoints remain available during driver unbind.
> Cancel pending scanning and remove the SCSI host before setting resetting
> and killing the anchored URBs.  Use the same teardown order for every
> disconnect path and rely on SCSI host removal to handle a device that can
> no longer communicate.
> 
> With the change, 10 of 10 zero-delay unbind/rebind iterations with 30
> READ commands in flight reattached the disk and completed a post-bind
> O_DIRECT read.  Unbind took 82 to 109 ms, with no UAS completion error or
> command timeout after rebind.
> 
> Signed-off-by: Jiayi Li <lijiayi@kylinos.cn>
Acked-by: Oliver Neukum <oneukum@suse.com>
> ---
> Changes in v2:
> - Remove the USB_STATE_NOTATTACHED-based disconnect classification.
> - Use the same teardown ordering for all disconnect paths.
> - Reword the commit message.
> 
> Link: https://lore.kernel.org/lkml/20260920012358.3362053-1-lijiayi@kylinos.cn/
> 
>   drivers/usb/storage/uas.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 8655edbd66b16..8752cecb45915 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -1217,6 +1217,14 @@ static void uas_disconnect(struct usb_interface *intf)
>   	struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata;
>   	unsigned long flags;
>   
> +	/*
> +	 * Prevent SCSI scanning (if it hasn't started yet)
> +	 * or wait for the SCSI-scanning routine to stop.
> +	 */
> +	cancel_work_sync(&devinfo->scan_work);
> +
> +	scsi_remove_host(shost);
> +
>   	spin_lock_irqsave(&devinfo->lock, flags);
>   	devinfo->resetting = 1;
>   	spin_unlock_irqrestore(&devinfo->lock, flags);
> @@ -1227,13 +1235,6 @@ static void uas_disconnect(struct usb_interface *intf)
>   	usb_kill_anchored_urbs(&devinfo->data_urbs);
>   	uas_zap_pending(devinfo, DID_NO_CONNECT);
>   
> -	/*
> -	 * Prevent SCSI scanning (if it hasn't started yet)
> -	 * or wait for the SCSI-scanning routine to stop.
> -	 */
> -	cancel_work_sync(&devinfo->scan_work);
> -
> -	scsi_remove_host(shost);
>   	uas_free_streams(devinfo);
>   	scsi_host_put(shost);
>   }
> @@ -1267,6 +1268,7 @@ static struct usb_driver uas_driver = {
>   	.suspend = uas_suspend,
>   	.resume = uas_resume,
>   	.reset_resume = uas_reset_resume,
> +	.soft_unbind = 1,
>   	.shutdown = uas_shutdown,
>   	.id_table = uas_usb_ids,
>   };

Re: [PATCH v2] usb: uas: quiesce SCSI before stopping endpoints on unbind

[Jiayi Li]

Hi Oliver,

> Acked-by: Oliver Neukum <oneukum@suse.com>

Thank you for the Acked-by. Subsequent testing found a regression in
v2: physically unplugging the device with UAS commands in flight can
cause an approximately 30-second SCSI timeout. I described the failure
in my reply to the Sashiko review:

https://lore.kernel.org/linux-scsi/20260922100542.164047-1-lijiayi@kylinos.cn/

I have a local candidate that keeps the unified teardown order.
When URB submission or completion returns -ENODEV or -ESHUTDOWN,
it marks the transport dead, stops further submissions, clears
COMMAND_INFLIGHT and sets DID_NO_CONNECT for pending commands.
It drains the remaining URBs in work context, allowing the commands
to complete through uas_try_complete().

Does this approach seem reasonable, or would you suggest a simpler
way to handle the physical-disconnect case?

Thanks,
Jiayi

Re: [PATCH v2] usb: uas: quiesce SCSI before stopping endpoints on unbind

[Oliver Neukum]

Hi,

thank you for pursuing this issue.

On 30.09.26 03:33, Jiayi Li wrote:
> Hi Oliver,
> 
>> Acked-by: Oliver Neukum <oneukum@suse.com>
> 
> Thank you for the Acked-by. Subsequent testing found a regression in
> v2: physically unplugging the device with UAS commands in flight can
> cause an approximately 30-second SCSI timeout. I described the failure

This can happen. Don't be discouraged.

> in my reply to the Sashiko review:
> 
> https://lore.kernel.org/linux-scsi/20260922100542.164047-1-lijiayi@kylinos.cn/
> 
> I have a local candidate that keeps the unified teardown order.

Highly important.

> When URB submission or completion returns -ENODEV or -ESHUTDOWN,
> it marks the transport dead, stops further submissions, clears

Why this specific trigger?
I am asking because -ENODEV comes relatively late in the process.
URBs tend to fail long before that.

> COMMAND_INFLIGHT and sets DID_NO_CONNECT for pending commands.
> It drains the remaining URBs in work context, allowing the commands
> to complete through uas_try_complete().

In the context of which work?

> Does this approach seem reasonable, or would you suggest a simpler
> way to handle the physical-disconnect case?

I definitely have no simpler way to handle physical disconnect.
With what you describe, while the approach seems entirely reasonable,
I wonder what is to be done if error handling has already started
on the SCSI level.

	Regards
		Oliver

Re: [PATCH v2] usb: uas: quiesce SCSI before stopping endpoints on unbind

[Jiayi Li]

> [Severity: High]
> Will moving scsi_remove_host() before devinfo->resetting = 1 and
> uas_zap_pending() cause a 30-second hang and 100% CPU livelock on
> physical unplug?
>
> On physical unplug, usbcore detects the disconnect and calls
> usb_disable_device(), which kills all active URBs with status -ESHUTDOWN
> before calling uas_disconnect().
>
> scsi_remove_host() synchronously waits for all in-flight SCSI commands to
> complete. Because the commands are stuck in-flight until uas_zap_pending()
> forcefully completes them, scsi_remove_host() will block indefinitely.

Thanks for the review.

I tested this case with v2 and can reproduce the approximately
30-second timeout by physically unplugging the device while UAS
commands are still in flight. I have not confirmed the reported
100% CPU livelock.

For a driver-only unbind, soft_unbind keeps the endpoints available
and the new ordering works as intended.

For a physical disconnect, however, the status URB completes with
-ESHUTDOWN before uas_disconnect() gets to drain the SCSI host.
uas_stat_cmplt() takes the error path in this case and does not clear
COMMAND_INFLIGHT.

With the v2 ordering, the sequence is therefore roughly:

    status URB -> -ESHUTDOWN
        -> COMMAND_INFLIGHT remains set
        -> scsi_remove_host()
        -> outstanding command cannot reach scsi_done()
        -> SCSI timeout

The existing disconnect ordering avoids this because
uas_zap_pending() runs before scsi_remove_host() and explicitly clears
COMMAND_INFLIGHT after the status URBs have been killed.

Would it make more sense to retain the original kill/zap-first ordering
for a physical disconnect, while keeping scsi_remove_host()-first for
a soft unbind?

I understand Alan's earlier point that USB_STATE_NOTATTACHED alone has
a race window, so if separate handling is appropriate, the remaining
question would be how to distinguish these cases reliably.

Thanks,
Jiayi