uas: Properly complete inflight commands on bus-reset or disconnect
Before this commit the uas driver would keep track of scsi commands which still
need to have some urbs submitted to the device, and complete this with an
ABORT result code on bus-reset or disconnect, but in flight scsi commands
which have all their urbs submitted, and thus are not part of the work list,
would never get their done callback called.
The problem is killed sense urbs don't have any tag info, so it is impossible
to tell which scsi cmd they belong to, so merely making sure all the urbs
have completed one way or the other is not enough.
This commit fixes this by changing the work list to an inflight list, which
keeps tracks of all inflight scsi cmnds, using the IS_IN_WORK_LIST flag to
determine if actual work needs to be done in uas_do_work(), and by moving
marking all inflight scsi commands as aborted and moving them to the dead list
on bus-reset or disconnect.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index e06505c..1a18839 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -58,7 +58,7 @@
struct scsi_cmnd *cmnd;
spinlock_t lock;
struct work_struct work;
- struct list_head work_list;
+ struct list_head inflight_list;
struct list_head dead_list;
};
@@ -86,7 +86,7 @@
struct urb *cmd_urb;
struct urb *data_in_urb;
struct urb *data_out_urb;
- struct list_head work;
+ struct list_head inflight;
struct list_head dead;
};
@@ -125,34 +125,36 @@
struct uas_dev_info *devinfo =
container_of(work, struct uas_dev_info, work);
struct uas_cmd_info *cmdinfo;
- struct uas_cmd_info *temp;
unsigned long flags;
int err;
spin_lock_irqsave(&devinfo->lock, flags);
- list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) {
+ list_for_each_entry(cmdinfo, &devinfo->inflight_list, inflight) {
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
SCp);
+
+ if (!(cmdinfo->state & IS_IN_WORK_LIST))
+ continue;
+
err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO);
- if (!err) {
+ if (!err)
cmdinfo->state &= ~IS_IN_WORK_LIST;
- list_del(&cmdinfo->work);
- } else {
+ else
schedule_work(&devinfo->work);
- }
}
spin_unlock_irqrestore(&devinfo->lock, flags);
}
-static void uas_abort_work(struct uas_dev_info *devinfo)
+static void uas_abort_inflight(struct uas_dev_info *devinfo)
{
struct uas_cmd_info *cmdinfo;
struct uas_cmd_info *temp;
unsigned long flags;
spin_lock_irqsave(&devinfo->lock, flags);
- list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) {
+ list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list,
+ inflight) {
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
SCp);
@@ -160,7 +162,7 @@
WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED);
cmdinfo->state |= COMMAND_ABORTED;
cmdinfo->state &= ~IS_IN_WORK_LIST;
- list_del(&cmdinfo->work);
+ list_del(&cmdinfo->inflight);
list_add_tail(&cmdinfo->dead, &devinfo->dead_list);
}
spin_unlock_irqrestore(&devinfo->lock, flags);
@@ -173,7 +175,6 @@
struct uas_dev_info *devinfo = cmnd->device->hostdata;
WARN_ON_ONCE(!spin_is_locked(&devinfo->lock));
- list_add_tail(&cmdinfo->work, &devinfo->work_list);
cmdinfo->state |= IS_IN_WORK_LIST;
schedule_work(&devinfo->work);
}
@@ -289,7 +290,8 @@
scmd_printk(KERN_INFO, cmnd, "abort completed\n");
cmnd->result = DID_ABORT << 16;
list_del(&cmdinfo->dead);
- }
+ } else
+ list_del(&cmdinfo->inflight);
cmnd->scsi_done(cmnd);
return 0;
}
@@ -717,6 +719,7 @@
uas_add_work(cmdinfo);
}
+ list_add_tail(&cmdinfo->inflight, &devinfo->inflight_list);
spin_unlock_irqrestore(&devinfo->lock, flags);
return 0;
}
@@ -807,11 +810,9 @@
spin_lock_irqsave(&devinfo->lock, flags);
WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED);
cmdinfo->state |= COMMAND_ABORTED;
+ cmdinfo->state &= ~IS_IN_WORK_LIST;
+ list_del(&cmdinfo->inflight);
list_add_tail(&cmdinfo->dead, &devinfo->dead_list);
- if (cmdinfo->state & IS_IN_WORK_LIST) {
- list_del(&cmdinfo->work);
- cmdinfo->state &= ~IS_IN_WORK_LIST;
- }
if (cmdinfo->state & COMMAND_INFLIGHT) {
spin_unlock_irqrestore(&devinfo->lock, flags);
ret = uas_eh_task_mgmt(cmnd, "ABORT TASK", TMF_ABORT_TASK);
@@ -847,7 +848,7 @@
shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__);
devinfo->resetting = 1;
- uas_abort_work(devinfo);
+ uas_abort_inflight(devinfo);
usb_kill_anchored_urbs(&devinfo->cmd_urbs);
usb_kill_anchored_urbs(&devinfo->sense_urbs);
usb_kill_anchored_urbs(&devinfo->data_urbs);
@@ -1018,7 +1019,7 @@
init_usb_anchor(&devinfo->data_urbs);
spin_lock_init(&devinfo->lock);
INIT_WORK(&devinfo->work, uas_do_work);
- INIT_LIST_HEAD(&devinfo->work_list);
+ INIT_LIST_HEAD(&devinfo->inflight_list);
INIT_LIST_HEAD(&devinfo->dead_list);
result = uas_configure_endpoints(devinfo);
@@ -1145,7 +1146,7 @@
devinfo->resetting = 1;
cancel_work_sync(&devinfo->work);
- uas_abort_work(devinfo);
+ uas_abort_inflight(devinfo);
usb_kill_anchored_urbs(&devinfo->cmd_urbs);
usb_kill_anchored_urbs(&devinfo->sense_urbs);
usb_kill_anchored_urbs(&devinfo->data_urbs);