diff options
author | Hans de Goede <hdegoede@redhat.com> | 2014-09-13 06:26:42 -0400 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2014-09-24 00:42:11 -0400 |
commit | f9dc024a2da1fe6b0ce180b89fac085e1255a932 (patch) | |
tree | 839617e87a348768dd0eb94922968831a5919f9d /drivers/usb | |
parent | 4c5481efb4346948ba7034432f86235a16ac9180 (diff) |
uas: pre_reset and suspend: Fix a few races
The purpose of uas_pre_reset is to:
1) Stop any new commands from being submitted while an externally triggered
usb-device-reset is running
2) Wait for any pending commands to finish before allowing the usb-device-reset
to continue
The purpose of uas_suspend is to:
2) Wait for any pending commands to finish before suspending
This commit fixes races in both paths:
1) For 1) we use scsi_block_requests, but the scsi midlayer calls queuecommand
without holding any locks, so a queuecommand may already past the midlayer
scsi_block_requests checks when we call it, add a check to uas_queuecommand
to fix this
2) For 2) we were waiting for all sense-urbs to complete, there are 2 problems
with this approach:
a) data-urbs may complete after the sense urb, so we need to check for those
too
b) if a sense-urb completes with a iu id of READ/WRITE_READY a command is not
yet done. We submit a new sense-urb immediately in this case, but that
submit may fail (in which case it will get retried by uas_do_work), if this
happens the sense_urbs anchor may become empty while the cmnd is not yet
done
Also unblock requests on timeout, to avoid things getting stuck in that case.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/usb')
-rw-r--r-- | drivers/usb/storage/uas.c | 61 |
1 files changed, 55 insertions, 6 deletions
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 10a3dea041ed..8d2e5450de91 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c | |||
@@ -667,6 +667,10 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, | |||
667 | 667 | ||
668 | BUILD_BUG_ON(sizeof(struct uas_cmd_info) > sizeof(struct scsi_pointer)); | 668 | BUILD_BUG_ON(sizeof(struct uas_cmd_info) > sizeof(struct scsi_pointer)); |
669 | 669 | ||
670 | /* Re-check scsi_block_requests now that we've the host-lock */ | ||
671 | if (cmnd->device->host->host_self_blocked) | ||
672 | return SCSI_MLQUEUE_DEVICE_BUSY; | ||
673 | |||
670 | if ((devinfo->flags & US_FL_NO_ATA_1X) && | 674 | if ((devinfo->flags & US_FL_NO_ATA_1X) && |
671 | (cmnd->cmnd[0] == ATA_12 || cmnd->cmnd[0] == ATA_16)) { | 675 | (cmnd->cmnd[0] == ATA_12 || cmnd->cmnd[0] == ATA_16)) { |
672 | memcpy(cmnd->sense_buffer, usb_stor_sense_invalidCDB, | 676 | memcpy(cmnd->sense_buffer, usb_stor_sense_invalidCDB, |
@@ -1009,6 +1013,54 @@ set_alt0: | |||
1009 | return result; | 1013 | return result; |
1010 | } | 1014 | } |
1011 | 1015 | ||
1016 | static int uas_cmnd_list_empty(struct uas_dev_info *devinfo) | ||
1017 | { | ||
1018 | unsigned long flags; | ||
1019 | int i, r = 1; | ||
1020 | |||
1021 | spin_lock_irqsave(&devinfo->lock, flags); | ||
1022 | |||
1023 | for (i = 0; i < devinfo->qdepth; i++) { | ||
1024 | if (devinfo->cmnd[i]) { | ||
1025 | r = 0; /* Not empty */ | ||
1026 | break; | ||
1027 | } | ||
1028 | } | ||
1029 | |||
1030 | spin_unlock_irqrestore(&devinfo->lock, flags); | ||
1031 | |||
1032 | return r; | ||
1033 | } | ||
1034 | |||
1035 | /* | ||
1036 | * Wait for any pending cmnds to complete, on usb-2 sense_urbs may temporarily | ||
1037 | * get empty while there still is more work to do due to sense-urbs completing | ||
1038 | * with a READ/WRITE_READY iu code, so keep waiting until the list gets empty. | ||
1039 | */ | ||
1040 | static int uas_wait_for_pending_cmnds(struct uas_dev_info *devinfo) | ||
1041 | { | ||
1042 | unsigned long start_time; | ||
1043 | int r; | ||
1044 | |||
1045 | start_time = jiffies; | ||
1046 | do { | ||
1047 | flush_work(&devinfo->work); | ||
1048 | |||
1049 | r = usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000); | ||
1050 | if (r == 0) | ||
1051 | return -ETIME; | ||
1052 | |||
1053 | r = usb_wait_anchor_empty_timeout(&devinfo->data_urbs, 500); | ||
1054 | if (r == 0) | ||
1055 | return -ETIME; | ||
1056 | |||
1057 | if (time_after(jiffies, start_time + 5 * HZ)) | ||
1058 | return -ETIME; | ||
1059 | } while (!uas_cmnd_list_empty(devinfo)); | ||
1060 | |||
1061 | return 0; | ||
1062 | } | ||
1063 | |||
1012 | static int uas_pre_reset(struct usb_interface *intf) | 1064 | static int uas_pre_reset(struct usb_interface *intf) |
1013 | { | 1065 | { |
1014 | struct Scsi_Host *shost = usb_get_intfdata(intf); | 1066 | struct Scsi_Host *shost = usb_get_intfdata(intf); |
@@ -1023,10 +1075,9 @@ static int uas_pre_reset(struct usb_interface *intf) | |||
1023 | scsi_block_requests(shost); | 1075 | scsi_block_requests(shost); |
1024 | spin_unlock_irqrestore(shost->host_lock, flags); | 1076 | spin_unlock_irqrestore(shost->host_lock, flags); |
1025 | 1077 | ||
1026 | /* Wait for any pending requests to complete */ | 1078 | if (uas_wait_for_pending_cmnds(devinfo) != 0) { |
1027 | flush_work(&devinfo->work); | ||
1028 | if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000) == 0) { | ||
1029 | shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__); | 1079 | shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__); |
1080 | scsi_unblock_requests(shost); | ||
1030 | return 1; | 1081 | return 1; |
1031 | } | 1082 | } |
1032 | 1083 | ||
@@ -1064,9 +1115,7 @@ static int uas_suspend(struct usb_interface *intf, pm_message_t message) | |||
1064 | struct Scsi_Host *shost = usb_get_intfdata(intf); | 1115 | struct Scsi_Host *shost = usb_get_intfdata(intf); |
1065 | struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; | 1116 | struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; |
1066 | 1117 | ||
1067 | /* Wait for any pending requests to complete */ | 1118 | if (uas_wait_for_pending_cmnds(devinfo) != 0) { |
1068 | flush_work(&devinfo->work); | ||
1069 | if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000) == 0) { | ||
1070 | shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__); | 1119 | shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__); |
1071 | return -ETIME; | 1120 | return -ETIME; |
1072 | } | 1121 | } |