| Message ID | 20200915134501.13947-3-oneukum@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] UAS: use macro for reporting results | expand |
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
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
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; }
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(-)