[PATCH] updates to UAS

[PATCH] updates to UAS

[Oliver Neukum]

The first patch just modernizes to new helpers. No functional change
intended

The second patch fixes a corner case where a whole tree of devices
is removed and I got a report of a live lock 

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

[PATCH 1/2] UAS: use macro for reporting results

[Oliver Neukum]

The SCSI layer has introduced a new macro for recording the result
of a command. Use it.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/storage/uas.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index d592071119ba..842ca5c82091 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -279,17 +279,17 @@ static bool uas_evaluate_response_iu(struct response_iu *riu, struct scsi_cmnd *
 
 	switch (response_code) {
 	case RC_INCORRECT_LUN:
-		cmnd->result = DID_BAD_TARGET << 16;
+		set_host_byte(cmnd, DID_BAD_TARGET);
 		break;
 	case RC_TMF_SUCCEEDED:
-		cmnd->result = DID_OK << 16;
+		set_host_byte(cmnd, DID_OK);
 		break;
 	case RC_TMF_NOT_SUPPORTED:
-		cmnd->result = DID_TARGET_FAILURE << 16;
+		set_host_byte(cmnd, DID_TARGET_FAILURE);
 		break;
 	default:
 		uas_log_cmd_state(cmnd, "response iu", response_code);
-		cmnd->result = DID_ERROR << 16;
+		set_host_byte(cmnd, DID_ERROR);
 		break;
 	}
 
@@ -660,7 +660,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
 	spin_lock_irqsave(&devinfo->lock, flags);
 
 	if (devinfo->resetting) {
-		cmnd->result = DID_ERROR << 16;
+		set_host_byte(cmnd, DID_ERROR);
 		cmnd->scsi_done(cmnd);
 		spin_unlock_irqrestore(&devinfo->lock, flags);
 		return 0;
-- 
2.16.4

[PATCH 2/2] UAS: fix disconnect by unplugging a hub

[Oliver Neukum]

The SCSI layer can go into an ugly loop if you ignore that a device
is gone. You need to report an error in the command rather than
in the return value of the queue method.
We need to specifically check for ENODEV..

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/storage/uas.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 842ca5c82091..19e730f9ad19 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -662,8 +662,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
 	if (devinfo->resetting) {
 		set_host_byte(cmnd, DID_ERROR);
 		cmnd->scsi_done(cmnd);
-		spin_unlock_irqrestore(&devinfo->lock, flags);
-		return 0;
+		goto zombie;
 	}
 
 	/* Find a free uas-tag */
@@ -699,6 +698,16 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
 		cmdinfo->state &= ~(SUBMIT_DATA_IN_URB | SUBMIT_DATA_OUT_URB);
 
 	err = uas_submit_urbs(cmnd, devinfo);
+	/*
+	 * in case of fatal errors the SCSI layer is peculiar
+	 * a command that has finished is a success for the purpose
+	 * of queueing, no matter how fatal the error
+	 */
+	if (err == -ENODEV) {
+		set_host_byte(cmnd, DID_ERROR);
+		cmnd->scsi_done(cmnd);
+		goto zombie;
+	}
 	if (err) {
 		/* If we did nothing, give up now */
 		if (cmdinfo->state & SUBMIT_STATUS_URB) {
@@ -709,6 +718,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
 	}
 
 	devinfo->cmnd[idx] = cmnd;
+zombie:
 	spin_unlock_irqrestore(&devinfo->lock, flags);
 	return 0;
 }
-- 
2.16.4

Re: [PATCH 2/2] UAS: fix disconnect by unplugging a hub

[Greg KH]

On Tue, Sep 15, 2020 at 03:45:01PM +0200, Oliver Neukum wrote:
> The SCSI layer can go into an ugly loop if you ignore that a device
> is gone. You need to report an error in the command rather than
> in the return value of the queue method.
> We need to specifically check for ENODEV..
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
>  drivers/usb/storage/uas.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

Should this one go to the stable kernels?

thanks,

greg k-h

Re: [PATCH 2/2] UAS: fix disconnect by unplugging a hub

[Oliver Neukum]

Am Dienstag, den 15.09.2020, 16:00 +0200 schrieb Greg KH:
> On Tue, Sep 15, 2020 at 03:45:01PM +0200, Oliver Neukum wrote:
> > The SCSI layer can go into an ugly loop if you ignore that a device
> > is gone. You need to report an error in the command rather than
> > in the return value of the queue method.
> > We need to specifically check for ENODEV..
> > 
> > Signed-off-by: Oliver Neukum <oneukum@suse.com>
> > ---
> >  drivers/usb/storage/uas.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> Should this one go to the stable kernels?

Hi,

in theory yes, but it depends on the previous patch.
Should I submit a separate patch?

	Regards
		Oliver

Re: [PATCH 2/2] UAS: fix disconnect by unplugging a hub

[Greg KH]

On Wed, Sep 16, 2020 at 10:11:25AM +0200, Oliver Neukum wrote:
> Am Dienstag, den 15.09.2020, 16:00 +0200 schrieb Greg KH:
> > On Tue, Sep 15, 2020 at 03:45:01PM +0200, Oliver Neukum wrote:
> > > The SCSI layer can go into an ugly loop if you ignore that a device
> > > is gone. You need to report an error in the command rather than
> > > in the return value of the queue method.
> > > We need to specifically check for ENODEV..
> > > 
> > > Signed-off-by: Oliver Neukum <oneukum@suse.com>
> > > ---
> > >  drivers/usb/storage/uas.c | 14 ++++++++++++--
> > >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > Should this one go to the stable kernels?
> 
> Hi,
> 
> in theory yes, but it depends on the previous patch.
> Should I submit a separate patch?

Reordering these would be best, thanks!

greg k-h