uas: Unblock scsi-requests on failure to alloc streams in post_reset

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 10-01-18 16:23, Oliver Neukum wrote:
> Am Mittwoch, den 10.01.2018, 08:13 +0100 schrieb Hans de Goede:
>> If we return 1 from our post_reset handler, then our disconnect handler
>> will be called immediately afterwards. Since pre_reset blocks all scsi
>> requests our disconnect handler will then hang in the scsi_remove_host
>> call.
> 
> Hi Hans,
> 
> it seems to me that the diagnosis is spot on. But why do we
> keep different code paths at all in this case? I do not see
> the point of not reporting the reset to the SCSI subsystem,
> even if we are not operational afterwards.
> So how about something like this?

Sure, works for me :)

Regards,

Hans


> 
>  From 4d1e26154bc5d09913bfba34d7adc39cce98d20a Mon Sep 17 00:00:00 2001
> From: Oliver Neukum <oneukum@suse.com>
> Date: Wed, 10 Jan 2018 16:16:03 +0100
> Subject: [PATCH] usb: uas: unconditionally bring back host after reset
> 
> Quoting Hans:
> 
> If we return 1 from our post_reset handler, then our disconnect handler
> will be called immediately afterwards. Since pre_reset blocks all scsi
> requests our disconnect handler will then hang in the scsi_remove_host
> call.
> 
> This is esp. bad because our disconnect handler hanging for ever also
> stops the USB subsys from enumerating any new USB devices, causes
> commands
> like lsusb to hang, etc.
> 
> In practice this happens when unplugging some uas devices because the
> hub
> code may see the device as needing a warm-reset and calls
> usb_reset_device
> before seeing the disconnect. In this case uas_configure_endpoints
> fails
> with -ENODEV. We do not want to print an error for this, so this commit
> also silences the shost_printk for -ENODEV.
> 
> ENDQUOTE
> 
> However, if we do that we better drop any unconditional execution
> and report to the SCSI subsystem that we have undergone a reset
> but we are not operational now.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> Reported-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   Makefile                  | 2 +-
>   drivers/usb/storage/uas.c | 7 +++----
>   2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index 5d04c40ee40a..3b1b9695177a 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -1076,20 +1076,19 @@ static int uas_post_reset(struct usb_interface
> *intf)
>   		return 0;
>   
>   	err = uas_configure_endpoints(devinfo);
> -	if (err) {
> +	if (err && err != ENODEV)
>   		shost_printk(KERN_ERR, shost,
>   			     "%s: alloc streams error %d after reset",
>   			     __func__, err);
> -		return 1;
> -	}
>   
> +	/* we must unblock the host in every case lest we deadlock */
>   	spin_lock_irqsave(shost->host_lock, flags);
>   	scsi_report_bus_reset(shost, 0);
>   	spin_unlock_irqrestore(shost->host_lock, flags);
>   
>   	scsi_unblock_requests(shost);
>   
> -	return 0;
> +	return err ? 1 : 0;
>   }
>   
>   static int uas_suspend(struct usb_interface *intf, pm_message_t
> message)
> -- 
> 2.13.6
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html