linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: linux-usb@vger.kernel.org
Cc: Gerd Hoffmann <kraxel@redhat.com>,
	Matthew Wilcox <willy@linux.intel.com>,
	Sarah Sharp <sarah.a.sharp@linux.intel.com>,
	Matthew Dharm <mdharm-usb@one-eyed-alien.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"open list:USB ATTACHED SCSI" <linux-scsi@vger.kernel.org>,
	"open list:USB MASS STORAGE..."
	<usb-storage@lists.one-eyed-alien.net>,
	open list <linux-kernel@vger.kernel.org>
Subject: [PATCH v3 4/6] uas: add dead request list
Date: Fri, 13 Sep 2013 13:27:13 +0200	[thread overview]
Message-ID: <1379071635-30701-5-git-send-email-kraxel@redhat.com> (raw)
In-Reply-To: <1379071635-30701-1-git-send-email-kraxel@redhat.com>

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 | 50 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 3cf5a5f..f049038 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -53,6 +53,7 @@ struct uas_dev_info {
 	spinlock_t lock;
 	struct work_struct work;
 	struct list_head work_list;
+	struct list_head dead_list;
 };
 
 enum {
@@ -80,6 +81,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 */
@@ -89,6 +91,7 @@ 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 void uas_unlink_data_urbs(struct uas_dev_info *devinfo,
 				 struct uas_cmd_info *cmdinfo)
@@ -150,16 +153,12 @@ static void uas_abort_work(struct uas_dev_info *devinfo)
 		struct scsi_pointer *scp = (void *)cmdinfo;
 		struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
 						      SCp);
+		uas_log_cmd_state(cmnd, __func__);
+		WARN_ON(cmdinfo->state & COMMAND_ABORTED);
 		cmdinfo->state |= COMMAND_ABORTED;
 		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__);
 		list_del(&cmdinfo->work);
+		list_add_tail(&cmdinfo->dead, &devinfo->dead_list);
 	}
 	spin_unlock_irqrestore(&devinfo->lock, flags);
 }
@@ -176,6 +175,28 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo)
 	schedule_work(&devinfo->work);
 }
 
+static void uas_zap_dead(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->dead_list, dead) {
+		struct scsi_pointer *scp = (void *)cmdinfo;
+		struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
+						      SCp);
+		uas_log_cmd_state(cmnd, __func__);
+		WARN_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__);
+	}
+	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;
@@ -263,6 +284,7 @@ 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;
+		list_del(&cmdinfo->dead);
 	}
 	cmnd->scsi_done(cmnd);
 	return 0;
@@ -292,7 +314,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;
 	}
@@ -743,7 +771,9 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
 
 	uas_log_cmd_state(cmnd, __func__);
 	spin_lock_irqsave(&devinfo->lock, flags);
+	WARN_ON(cmdinfo->state & COMMAND_ABORTED);
 	cmdinfo->state |= COMMAND_ABORTED;
+	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;
@@ -776,11 +806,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)
@@ -988,6 +1020,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	spin_lock_init(&devinfo->lock);
 	INIT_WORK(&devinfo->work, uas_do_work);
 	INIT_LIST_HEAD(&devinfo->work_list);
+	INIT_LIST_HEAD(&devinfo->dead_list);
 	uas_configure_endpoints(devinfo);
 
 	result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 3);
@@ -1036,6 +1069,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

  parent reply	other threads:[~2013-09-13 11:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1379071635-30701-1-git-send-email-kraxel@redhat.com>
2013-09-13 11:27 ` [PATCH v3 2/6] uas: properly reinitialize in uas_eh_bus_reset_handler Gerd Hoffmann
     [not found] ` <1379071635-30701-1-git-send-email-kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-09-13 11:27   ` [PATCH v3 3/6] uas: make work list per-device Gerd Hoffmann
2013-09-17 20:30     ` Christoph Hellwig
2013-09-18  7:33       ` Gerd Hoffmann
2013-09-24  8:52         ` Christoph Hellwig
2013-09-13 11:27 ` Gerd Hoffmann [this message]
2013-09-13 11:27 ` [PATCH v3 5/6] uas: replace BUG_ON() + WARN_ON() with WARN_ON_ONCE() Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1379071635-30701-5-git-send-email-kraxel@redhat.com \
    --to=kraxel@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mdharm-usb@one-eyed-alien.net \
    --cc=sarah.a.sharp@linux.intel.com \
    --cc=usb-storage@lists.one-eyed-alien.net \
    --cc=willy@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).