* [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.