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@xxxxxxxxxx>
Acked-by: Oliver Neukum <oneukum@xxxxxxxx>
> ---
> 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@xxxxxxxxxx/
>
>   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,
>   };