[PATCH 3/5] uas: rename work list lock + list field

[PATCH 3/5] uas: rename work list lock + list field

[Gerd Hoffmann]

This patch prepares for the addition of another list and renames the
work list lock and the list_head field in struct uas_cmd_info.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/usb/storage/uas.c | 50 +++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index f89202f..a63972a 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -77,7 +77,7 @@ struct uas_cmd_info {
 	struct urb *cmd_urb;
 	struct urb *data_in_urb;
 	struct urb *data_out_urb;
-	struct list_head list;
+	struct list_head work;
 };
 
 /* I hate forward declarations, but I actually have a loop */
@@ -89,7 +89,7 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo);
 static void uas_free_streams(struct uas_dev_info *devinfo);
 
 static DECLARE_WORK(uas_work, uas_do_work);
-static DEFINE_SPINLOCK(uas_work_lock);
+static DEFINE_SPINLOCK(uas_lists_lock);
 static LIST_HEAD(uas_work_list);
 
 static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
@@ -124,11 +124,11 @@ static void uas_do_work(struct work_struct *work)
 	unsigned long flags;
 	int err;
 
-	spin_lock_irq(&uas_work_lock);
+	spin_lock_irq(&uas_lists_lock);
 	list_replace_init(&uas_work_list, &list);
-	spin_unlock_irq(&uas_work_lock);
+	spin_unlock_irq(&uas_lists_lock);
 
-	list_for_each_entry_safe(cmdinfo, temp, &list, list) {
+	list_for_each_entry_safe(cmdinfo, temp, &list, work) {
 		struct scsi_pointer *scp = (void *)cmdinfo;
 		struct scsi_cmnd *cmnd = container_of(scp,
 							struct scsi_cmnd, SCp);
@@ -139,10 +139,10 @@ static void uas_do_work(struct work_struct *work)
 			cmdinfo->state &= ~IS_IN_WORK_LIST;
 		spin_unlock_irqrestore(&devinfo->lock, flags);
 		if (err) {
-			list_del(&cmdinfo->list);
-			spin_lock_irq(&uas_work_lock);
-			list_add_tail(&cmdinfo->list, &uas_work_list);
-			spin_unlock_irq(&uas_work_lock);
+			list_del(&cmdinfo->work);
+			spin_lock_irq(&uas_lists_lock);
+			list_add_tail(&cmdinfo->work, &uas_work_list);
+			spin_unlock_irq(&uas_lists_lock);
 			schedule_work(&uas_work);
 		}
 	}
@@ -155,12 +155,12 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
 	struct list_head list;
 	unsigned long flags;
 
-	spin_lock_irq(&uas_work_lock);
+	spin_lock_irq(&uas_lists_lock);
 	list_replace_init(&uas_work_list, &list);
-	spin_unlock_irq(&uas_work_lock);
+	spin_unlock_irq(&uas_lists_lock);
 
 	spin_lock_irqsave(&devinfo->lock, flags);
-	list_for_each_entry_safe(cmdinfo, temp, &list, list) {
+	list_for_each_entry_safe(cmdinfo, temp, &list, work) {
 		struct scsi_pointer *scp = (void *)cmdinfo;
 		struct scsi_cmnd *cmnd = container_of(scp,
 							struct scsi_cmnd, SCp);
@@ -178,10 +178,10 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
 			uas_try_complete(cmnd, __func__);
 		} else {
 			/* not our uas device, relink into list */
-			list_del(&cmdinfo->list);
-			spin_lock_irq(&uas_work_lock);
-			list_add_tail(&cmdinfo->list, &uas_work_list);
-			spin_unlock_irq(&uas_work_lock);
+			list_del(&cmdinfo->work);
+			spin_lock_irq(&uas_lists_lock);
+			list_add_tail(&cmdinfo->work, &uas_work_list);
+			spin_unlock_irq(&uas_lists_lock);
 		}
 	}
 	spin_unlock_irqrestore(&devinfo->lock, flags);
@@ -288,10 +288,10 @@ static void uas_xfer_data(struct urb *urb, struct scsi_cmnd *cmnd,
 	cmdinfo->state |= direction | SUBMIT_STATUS_URB;
 	err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC);
 	if (err) {
-		spin_lock(&uas_work_lock);
-		list_add_tail(&cmdinfo->list, &uas_work_list);
+		spin_lock(&uas_lists_lock);
+		list_add_tail(&cmdinfo->work, &uas_work_list);
 		cmdinfo->state |= IS_IN_WORK_LIST;
-		spin_unlock(&uas_work_lock);
+		spin_unlock(&uas_lists_lock);
 		schedule_work(&uas_work);
 	}
 }
@@ -694,10 +694,10 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
 			spin_unlock_irqrestore(&devinfo->lock, flags);
 			return SCSI_MLQUEUE_DEVICE_BUSY;
 		}
-		spin_lock(&uas_work_lock);
-		list_add_tail(&cmdinfo->list, &uas_work_list);
+		spin_lock(&uas_lists_lock);
+		list_add_tail(&cmdinfo->work, &uas_work_list);
 		cmdinfo->state |= IS_IN_WORK_LIST;
-		spin_unlock(&uas_work_lock);
+		spin_unlock(&uas_lists_lock);
 		schedule_work(&uas_work);
 	}
 
@@ -764,10 +764,10 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
 	spin_lock_irqsave(&devinfo->lock, flags);
 	cmdinfo->state |= COMMAND_ABORTED;
 	if (cmdinfo->state & IS_IN_WORK_LIST) {
-		spin_lock(&uas_work_lock);
-		list_del(&cmdinfo->list);
+		spin_lock(&uas_lists_lock);
+		list_del(&cmdinfo->work);
 		cmdinfo->state &= ~IS_IN_WORK_LIST;
-		spin_unlock(&uas_work_lock);
+		spin_unlock(&uas_lists_lock);
 	}
 	if (cmdinfo->state & COMMAND_INFLIGHT) {
 		spin_unlock_irqrestore(&devinfo->lock, flags);
-- 
1.8.3.1

[PATCH 2/5] uas: properly reinitialize in uas_eh_bus_reset_handler

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/usb/storage/uas.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index d966b59..fc08ee9 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -85,6 +85,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
 				struct uas_dev_info *devinfo, gfp_t gfp);
 static void uas_do_work(struct work_struct *work);
 static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller);
+static void uas_configure_endpoints(struct uas_dev_info *devinfo);
+static void uas_free_streams(struct uas_dev_info *devinfo);
 
 static DECLARE_WORK(uas_work, uas_do_work);
 static DEFINE_SPINLOCK(uas_work_lock);
@@ -800,7 +802,10 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
 	usb_kill_anchored_urbs(&devinfo->cmd_urbs);
 	usb_kill_anchored_urbs(&devinfo->sense_urbs);
 	usb_kill_anchored_urbs(&devinfo->data_urbs);
+	uas_free_streams(devinfo);
 	err = usb_reset_device(udev);
+	if (!err)
+		uas_configure_endpoints(devinfo);
 	devinfo->resetting = 0;
 
 	if (err) {
-- 
1.8.3.1

[PATCH 3/5] uas: rename work list lock + list field

[Gerd Hoffmann]

This patch prepares for the addition of another list and renames the
work list lock and the list_head field in struct uas_cmd_info.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/usb/storage/uas.c | 50 +++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index fc08ee9..db09bda 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -77,7 +77,7 @@ struct uas_cmd_info {
 	struct urb *cmd_urb;
 	struct urb *data_in_urb;
 	struct urb *data_out_urb;
-	struct list_head list;
+	struct list_head work;
 };
 
 /* I hate forward declarations, but I actually have a loop */
@@ -89,7 +89,7 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo);
 static void uas_free_streams(struct uas_dev_info *devinfo);
 
 static DECLARE_WORK(uas_work, uas_do_work);
-static DEFINE_SPINLOCK(uas_work_lock);
+static DEFINE_SPINLOCK(uas_lists_lock);
 static LIST_HEAD(uas_work_list);
 
 static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
@@ -124,11 +124,11 @@ static void uas_do_work(struct work_struct *work)
 	unsigned long flags;
 	int err;
 
-	spin_lock_irq(&uas_work_lock);
+	spin_lock_irq(&uas_lists_lock);
 	list_replace_init(&uas_work_list, &list);
-	spin_unlock_irq(&uas_work_lock);
+	spin_unlock_irq(&uas_lists_lock);
 
-	list_for_each_entry_safe(cmdinfo, temp, &list, list) {
+	list_for_each_entry_safe(cmdinfo, temp, &list, work) {
 		struct scsi_pointer *scp = (void *)cmdinfo;
 		struct scsi_cmnd *cmnd = container_of(scp,
 							struct scsi_cmnd, SCp);
@@ -139,10 +139,10 @@ static void uas_do_work(struct work_struct *work)
 			cmdinfo->state &= ~IS_IN_WORK_LIST;
 		spin_unlock_irqrestore(&devinfo->lock, flags);
 		if (err) {
-			list_del(&cmdinfo->list);
-			spin_lock_irq(&uas_work_lock);
-			list_add_tail(&cmdinfo->list, &uas_work_list);
-			spin_unlock_irq(&uas_work_lock);
+			list_del(&cmdinfo->work);
+			spin_lock_irq(&uas_lists_lock);
+			list_add_tail(&cmdinfo->work, &uas_work_list);
+			spin_unlock_irq(&uas_lists_lock);
 			schedule_work(&uas_work);
 		}
 	}
@@ -155,12 +155,12 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
 	struct list_head list;
 	unsigned long flags;
 
-	spin_lock_irq(&uas_work_lock);
+	spin_lock_irq(&uas_lists_lock);
 	list_replace_init(&uas_work_list, &list);
-	spin_unlock_irq(&uas_work_lock);
+	spin_unlock_irq(&uas_lists_lock);
 
 	spin_lock_irqsave(&devinfo->lock, flags);
-	list_for_each_entry_safe(cmdinfo, temp, &list, list) {
+	list_for_each_entry_safe(cmdinfo, temp, &list, work) {
 		struct scsi_pointer *scp = (void *)cmdinfo;
 		struct scsi_cmnd *cmnd = container_of(scp,
 							struct scsi_cmnd, SCp);
@@ -178,10 +178,10 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
 			uas_try_complete(cmnd, __func__);
 		} else {
 			/* not our uas device, relink into list */
-			list_del(&cmdinfo->list);
-			spin_lock_irq(&uas_work_lock);
-			list_add_tail(&cmdinfo->list, &uas_work_list);
-			spin_unlock_irq(&uas_work_lock);
+			list_del(&cmdinfo->work);
+			spin_lock_irq(&uas_lists_lock);
+			list_add_tail(&cmdinfo->work, &uas_work_list);
+			spin_unlock_irq(&uas_lists_lock);
 		}
 	}
 	spin_unlock_irqrestore(&devinfo->lock, flags);
@@ -288,10 +288,10 @@ static void uas_xfer_data(struct urb *urb, struct scsi_cmnd *cmnd,
 	cmdinfo->state |= direction | SUBMIT_STATUS_URB;
 	err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC);
 	if (err) {
-		spin_lock(&uas_work_lock);
-		list_add_tail(&cmdinfo->list, &uas_work_list);
+		spin_lock(&uas_lists_lock);
+		list_add_tail(&cmdinfo->work, &uas_work_list);
 		cmdinfo->state |= IS_IN_WORK_LIST;
-		spin_unlock(&uas_work_lock);
+		spin_unlock(&uas_lists_lock);
 		schedule_work(&uas_work);
 	}
 }
@@ -694,10 +694,10 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
 			spin_unlock_irqrestore(&devinfo->lock, flags);
 			return SCSI_MLQUEUE_DEVICE_BUSY;
 		}
-		spin_lock(&uas_work_lock);
-		list_add_tail(&cmdinfo->list, &uas_work_list);
+		spin_lock(&uas_lists_lock);
+		list_add_tail(&cmdinfo->work, &uas_work_list);
 		cmdinfo->state |= IS_IN_WORK_LIST;
-		spin_unlock(&uas_work_lock);
+		spin_unlock(&uas_lists_lock);
 		schedule_work(&uas_work);
 	}
 
@@ -764,10 +764,10 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
 	spin_lock_irqsave(&devinfo->lock, flags);
 	cmdinfo->state |= COMMAND_ABORTED;
 	if (cmdinfo->state & IS_IN_WORK_LIST) {
-		spin_lock(&uas_work_lock);
-		list_del(&cmdinfo->list);
+		spin_lock(&uas_lists_lock);
+		list_del(&cmdinfo->work);
 		cmdinfo->state &= ~IS_IN_WORK_LIST;
-		spin_unlock(&uas_work_lock);
+		spin_unlock(&uas_lists_lock);
 	}
 	if (cmdinfo->state & COMMAND_INFLIGHT) {
 		spin_unlock_irqrestore(&devinfo->lock, flags);
-- 
1.8.3.1

[PATCH 4/5] uas: add dead request list

[Gerd Hoffmann]

This patch adds a new list where all requests which are canceled are
added to, so we don't loose them.  Then, after killing all inflight
urbs on bus reset (and disconnect) we'll walk over the list and clean
them up.

Without this we can end up with aborted requests lingering around in
case of status pipe transfer errors.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/usb/storage/uas.c | 70 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index db09bda..2b3ca29 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -78,6 +78,7 @@ struct uas_cmd_info {
 	struct urb *data_in_urb;
 	struct urb *data_out_urb;
 	struct list_head work;
+	struct list_head dead;
 };
 
 /* I hate forward declarations, but I actually have a loop */
@@ -87,10 +88,12 @@ static void uas_do_work(struct work_struct *work);
 static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller);
 static void uas_configure_endpoints(struct uas_dev_info *devinfo);
 static void uas_free_streams(struct uas_dev_info *devinfo);
+static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller);
 
 static DECLARE_WORK(uas_work, uas_do_work);
 static DEFINE_SPINLOCK(uas_lists_lock);
 static LIST_HEAD(uas_work_list);
+static LIST_HEAD(uas_dead_list);
 
 static void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
 				 struct uas_cmd_info *cmdinfo)
@@ -167,15 +170,13 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
 		struct uas_dev_info *di = (void *)cmnd->device->hostdata;
 
 		if (di == devinfo) {
+			uas_log_cmd_state(cmnd, __func__);
+			BUG_ON(cmdinfo->state & COMMAND_ABORTED);
 			cmdinfo->state |= COMMAND_ABORTED;
+			spin_lock_irq(&uas_lists_lock);
+			list_add_tail(&cmdinfo->dead, &uas_dead_list);
+			spin_unlock_irq(&uas_lists_lock);
 			cmdinfo->state &= ~IS_IN_WORK_LIST;
-			if (devinfo->resetting) {
-				/* uas_stat_cmplt() will not do that
-				 * when a device reset is in
-				 * progress */
-				cmdinfo->state &= ~COMMAND_INFLIGHT;
-			}
-			uas_try_complete(cmnd, __func__);
 		} else {
 			/* not our uas device, relink into list */
 			list_del(&cmdinfo->work);
@@ -187,6 +188,43 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
 	spin_unlock_irqrestore(&devinfo->lock, flags);
 }
 
+static void uas_zap_dead(struct uas_dev_info *devinfo)
+{
+	struct uas_cmd_info *cmdinfo;
+	struct uas_cmd_info *temp;
+	struct list_head list;
+	unsigned long flags;
+
+	spin_lock_irq(&uas_lists_lock);
+	list_replace_init(&uas_dead_list, &list);
+	spin_unlock_irq(&uas_lists_lock);
+
+	spin_lock_irqsave(&devinfo->lock, flags);
+	list_for_each_entry_safe(cmdinfo, temp, &list, dead) {
+		struct scsi_pointer *scp = (void *)cmdinfo;
+		struct scsi_cmnd *cmnd = container_of(scp,
+							struct scsi_cmnd, SCp);
+		struct uas_dev_info *di = (void *)cmnd->device->hostdata;
+
+		if (di == devinfo) {
+			uas_log_cmd_state(cmnd, __func__);
+			BUG_ON(!(cmdinfo->state & COMMAND_ABORTED));
+			/* all urbs are killed, clear inflight bits */
+			cmdinfo->state &= ~(COMMAND_INFLIGHT |
+					    DATA_IN_URB_INFLIGHT |
+					    DATA_OUT_URB_INFLIGHT);
+			uas_try_complete(cmnd, __func__);
+		} else {
+			/* not our uas device, relink into list */
+			list_del(&cmdinfo->dead);
+			spin_lock_irq(&uas_lists_lock);
+			list_add_tail(&cmdinfo->dead, &uas_dead_list);
+			spin_unlock_irq(&uas_lists_lock);
+		}
+	}
+	spin_unlock_irqrestore(&devinfo->lock, flags);
+}
+
 static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd)
 {
 	struct sense_iu *sense_iu = urb->transfer_buffer;
@@ -274,6 +312,9 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
 	if (cmdinfo->state & COMMAND_ABORTED) {
 		scmd_printk(KERN_INFO, cmnd, "abort completed\n");
 		cmnd->result = DID_ABORT << 16;
+		spin_lock_irq(&uas_lists_lock);
+		list_del(&cmdinfo->dead);
+		spin_unlock_irq(&uas_lists_lock);
 	}
 	cmnd->scsi_done(cmnd);
 	return 0;
@@ -307,7 +348,13 @@ static void uas_stat_cmplt(struct urb *urb)
 	u16 tag;
 
 	if (urb->status) {
-		dev_err(&urb->dev->dev, "URB BAD STATUS %d\n", urb->status);
+		if (urb->status == -ENOENT) {
+			dev_err(&urb->dev->dev, "stat urb: killed, stream %d\n",
+				urb->stream_id);
+		} else {
+			dev_err(&urb->dev->dev, "stat urb: status %d\n",
+				urb->status);
+		}
 		usb_free_urb(urb);
 		return;
 	}
@@ -762,7 +809,11 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
 
 	uas_log_cmd_state(cmnd, __func__);
 	spin_lock_irqsave(&devinfo->lock, flags);
+	BUG_ON(cmdinfo->state & COMMAND_ABORTED);
 	cmdinfo->state |= COMMAND_ABORTED;
+	spin_lock_irq(&uas_lists_lock);
+	list_add_tail(&cmdinfo->dead, &uas_dead_list);
+	spin_unlock_irq(&uas_lists_lock);
 	if (cmdinfo->state & IS_IN_WORK_LIST) {
 		spin_lock(&uas_lists_lock);
 		list_del(&cmdinfo->work);
@@ -797,11 +848,13 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
 	struct usb_device *udev = devinfo->udev;
 	int err;
 
+	shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__);
 	devinfo->resetting = 1;
 	uas_abort_work(devinfo);
 	usb_kill_anchored_urbs(&devinfo->cmd_urbs);
 	usb_kill_anchored_urbs(&devinfo->sense_urbs);
 	usb_kill_anchored_urbs(&devinfo->data_urbs);
+	uas_zap_dead(devinfo);
 	uas_free_streams(devinfo);
 	err = usb_reset_device(udev);
 	if (!err)
@@ -1054,6 +1107,7 @@ static void uas_disconnect(struct usb_interface *intf)
 	usb_kill_anchored_urbs(&devinfo->cmd_urbs);
 	usb_kill_anchored_urbs(&devinfo->sense_urbs);
 	usb_kill_anchored_urbs(&devinfo->data_urbs);
+	uas_zap_dead(devinfo);
 	scsi_remove_host(shost);
 	uas_free_streams(devinfo);
 	kfree(devinfo);
-- 
1.8.3.1