* [PATCH] updates to UAS
@ 2020-09-15 13:44 Oliver Neukum
2020-09-15 13:45 ` [PATCH 1/2] UAS: use macro for reporting results Oliver Neukum
2020-09-15 13:45 ` [PATCH 2/2] UAS: fix disconnect by unplugging a hub Oliver Neukum
0 siblings, 2 replies; 6+ messages in thread
From: Oliver Neukum @ 2020-09-15 13:44 UTC (permalink / raw)
To: gregKH, linux-usb; +Cc: 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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] UAS: use macro for reporting results
2020-09-15 13:44 [PATCH] updates to UAS Oliver Neukum
@ 2020-09-15 13:45 ` Oliver Neukum
2020-09-15 13:45 ` [PATCH 2/2] UAS: fix disconnect by unplugging a hub Oliver Neukum
1 sibling, 0 replies; 6+ messages in thread
From: Oliver Neukum @ 2020-09-15 13:45 UTC (permalink / raw)
To: gregKH, linux-usb; +Cc: 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] UAS: fix disconnect by unplugging a hub
2020-09-15 13:44 [PATCH] updates to UAS Oliver Neukum
2020-09-15 13:45 ` [PATCH 1/2] UAS: use macro for reporting results Oliver Neukum
@ 2020-09-15 13:45 ` Oliver Neukum
2020-09-15 14:00 ` Greg KH
1 sibling, 1 reply; 6+ messages in thread
From: Oliver Neukum @ 2020-09-15 13:45 UTC (permalink / raw)
To: gregKH, linux-usb; +Cc: 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] UAS: fix disconnect by unplugging a hub
2020-09-15 13:45 ` [PATCH 2/2] UAS: fix disconnect by unplugging a hub Oliver Neukum
@ 2020-09-15 14:00 ` Greg KH
2020-09-16 8:11 ` Oliver Neukum
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2020-09-15 14:00 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-usb
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] UAS: fix disconnect by unplugging a hub
2020-09-15 14:00 ` Greg KH
@ 2020-09-16 8:11 ` Oliver Neukum
2020-09-16 8:37 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Oliver Neukum @ 2020-09-16 8:11 UTC (permalink / raw)
To: Greg KH; +Cc: linux-usb
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] UAS: fix disconnect by unplugging a hub
2020-09-16 8:11 ` Oliver Neukum
@ 2020-09-16 8:37 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2020-09-16 8:37 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-usb
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-16 8:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-15 13:44 [PATCH] updates to UAS Oliver Neukum
2020-09-15 13:45 ` [PATCH 1/2] UAS: use macro for reporting results Oliver Neukum
2020-09-15 13:45 ` [PATCH 2/2] UAS: fix disconnect by unplugging a hub Oliver Neukum
2020-09-15 14:00 ` Greg KH
2020-09-16 8:11 ` Oliver Neukum
2020-09-16 8:37 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.