[PATCH v2 1/3] usb: uas: complete command on status URB error

[Sarthak Kukreti <sarthakkukreti@google.com>]

When a UAS device is unplugged while hub_wq is frozen during system
suspend, usb_disconnect() is deferred and sd_sync_cache() issues
SYNCHRONIZE_CACHE to the disconnected target. If the status URB fails in
uas_stat_cmplt(), uas_stat_cmplt() logs the error and frees the URB
without completing the command, leaving SYNCHRONIZE_CACHE pending until
the 60s SCSI timeout and triggering the 25s DPM watchdog.

Track each command's active sense_urb in uas_cmd_info. When a status URB
fails in uas_stat_cmplt() while not resetting, complete the command that
owns the failed status URB (with DID_NO_CONNECT on -ENODEV/-ESHUTDOWN
and DID_ERROR otherwise), free unsubmitted URBs, and unlink any inflight
data URBs outside devinfo->lock. Set DID_NO_CONNECT on -ENODEV/-ESHUTDOWN
and preserve non-DID_OK host bytes in uas_data_cmplt().

Fixes: 115bb1ffa54c ("USB: Add UAS driver")
Signed-off-by: Sarthak Kukreti <sarthakkukreti@google.com>
---
 drivers/usb/storage/uas.c | 76 +++++++++++++++++++++++++++++++-------
 1 file changed, 64 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 265162981269..904b4ce04fc6 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -71,6 +71,7 @@ struct uas_cmd_info {
 	struct urb *cmd_urb;
 	struct urb *data_in_urb;
 	struct urb *data_out_urb;
+	struct urb *sense_urb;
 };
 
 /* I hate forward declarations, but I actually have a loop */
@@ -232,14 +233,20 @@ static void uas_free_unsubmitted_urbs(struct scsi_cmnd *cmnd)
 
 	cmdinfo = scsi_cmd_priv(cmnd);
 
-	if (cmdinfo->state & SUBMIT_CMD_URB)
+	if (cmdinfo->state & SUBMIT_CMD_URB) {
 		usb_free_urb(cmdinfo->cmd_urb);
+		cmdinfo->cmd_urb = NULL;
+	}
 
 	/* data urbs may have never gotten their submit flag set */
-	if (!(cmdinfo->state & DATA_IN_URB_INFLIGHT))
+	if (!(cmdinfo->state & DATA_IN_URB_INFLIGHT)) {
 		usb_free_urb(cmdinfo->data_in_urb);
-	if (!(cmdinfo->state & DATA_OUT_URB_INFLIGHT))
+		cmdinfo->data_in_urb = NULL;
+	}
+	if (!(cmdinfo->state & DATA_OUT_URB_INFLIGHT)) {
 		usb_free_urb(cmdinfo->data_out_urb);
+		cmdinfo->data_out_urb = NULL;
+	}
 }
 
 static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
@@ -304,23 +311,58 @@ static void uas_stat_cmplt(struct urb *urb)
 	struct urb *data_out_urb = NULL;
 	struct scsi_cmnd *cmnd;
 	struct uas_cmd_info *cmdinfo;
+	struct uas_cmd_info *s_cmdinfo = NULL;
 	unsigned long flags;
 	unsigned int idx;
+	int s_idx;
 	int status = urb->status;
 	bool success;
 
+	spin_lock_irqsave(&devinfo->lock, flags);
+
+	for (s_idx = 0; s_idx < devinfo->qdepth; s_idx++) {
+		if (!devinfo->cmnd[s_idx])
+			continue;
+		cmdinfo = scsi_cmd_priv(devinfo->cmnd[s_idx]);
+		if (cmdinfo->sense_urb == urb) {
+			s_cmdinfo = cmdinfo;
+			s_cmdinfo->sense_urb = NULL;
+			break;
+		}
+	}
+
+	if (devinfo->resetting)
+		goto out;
+
 	if (status) {
 		if (status != -ENOENT && status != -ECONNRESET && status != -ESHUTDOWN)
 			dev_err(&urb->dev->dev, "stat urb: status %d\n", status);
-		goto bail;
+		if (!s_cmdinfo)
+			goto out;
+		cmnd = devinfo->cmnd[s_idx];
+		if (s_cmdinfo->state & DATA_IN_URB_INFLIGHT)
+			data_in_urb = usb_get_urb(s_cmdinfo->data_in_urb);
+		if (s_cmdinfo->state & DATA_OUT_URB_INFLIGHT)
+			data_out_urb = usb_get_urb(s_cmdinfo->data_out_urb);
+		uas_free_unsubmitted_urbs(cmnd);
+		s_cmdinfo->state &= ~(COMMAND_INFLIGHT |
+				      SUBMIT_STATUS_URB |
+				      ALLOC_DATA_IN_URB |
+				      SUBMIT_DATA_IN_URB |
+				      ALLOC_DATA_OUT_URB |
+				      SUBMIT_DATA_OUT_URB |
+				      ALLOC_CMD_URB |
+				      SUBMIT_CMD_URB |
+				      IS_IN_WORK_LIST);
+		if (status == -ENODEV || status == -ESHUTDOWN)
+			set_host_byte(cmnd, DID_NO_CONNECT);
+		else if (host_byte(cmnd->result) == DID_OK)
+			set_host_byte(cmnd, DID_ERROR);
+		uas_try_complete(cmnd, __func__);
+		goto out_unlink;
 	}
 
 	idx = be16_to_cpup(&iu->tag) - 1;
-
-	spin_lock_irqsave(&devinfo->lock, flags);
-
-	if (devinfo->resetting)
-		goto out;
 	if (idx >= MAX_CMNDS || !devinfo->cmnd[idx]) {
 		dev_err(&urb->dev->dev,
 			"stat urb: no pending cmd for uas-tag %d\n", idx + 1);
@@ -335,6 +377,10 @@ static void uas_stat_cmplt(struct urb *urb)
 		goto out;
 	}
 
+	if (s_cmdinfo)
+		s_cmdinfo->sense_urb = cmdinfo->sense_urb;
+	cmdinfo->sense_urb = NULL;
+
 	switch (iu->iu_id) {
 	case IU_ID_STATUS:
 		uas_sense(urb, cmnd);
@@ -375,6 +421,7 @@ static void uas_stat_cmplt(struct urb *urb)
 	default:
 		uas_log_cmd_state(cmnd, "bogus IU", iu->iu_id);
 	}
+out_unlink:
 	spin_unlock_irqrestore(&devinfo->lock, flags);
 	usb_free_urb(urb);
 
@@ -391,7 +438,6 @@ static void uas_stat_cmplt(struct urb *urb)
 
 out:
 	spin_unlock_irqrestore(&devinfo->lock, flags);
-bail:
 	usb_free_urb(urb);
 }
 
@@ -428,7 +474,10 @@ static void uas_data_cmplt(struct urb *urb)
 			uas_log_cmd_state(cmnd, "data cmplt err", status);
 		/* error: no data transfered */
 		scsi_set_resid(cmnd, sdb->length);
-		set_host_byte(cmnd, DID_ERROR);
+		if (status == -ENODEV || status == -ESHUTDOWN)
+			set_host_byte(cmnd, DID_NO_CONNECT);
+		else if (host_byte(cmnd->result) == DID_OK)
+			set_host_byte(cmnd, DID_ERROR);
 	} else {
 		scsi_set_resid(cmnd, sdb->length - urb->actual_length);
 	}
@@ -543,6 +592,7 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp,
 static int uas_submit_sense_urb(struct scsi_cmnd *cmnd, gfp_t gfp)
 {
 	struct uas_dev_info *devinfo = cmnd->device->hostdata;
+	struct uas_cmd_info *cmdinfo = scsi_cmd_priv(cmnd);
 	struct urb *urb;
 	int err;
 
@@ -555,8 +605,10 @@ static int uas_submit_sense_urb(struct scsi_cmnd *cmnd, gfp_t gfp)
 		usb_unanchor_urb(urb);
 		uas_log_cmd_state(cmnd, "sense submit err", err);
 		usb_free_urb(urb);
+		return err;
 	}
-	return err;
+	cmdinfo->sense_urb = urb;
+	return 0;
 }
 
 static int uas_submit_urbs(struct scsi_cmnd *cmnd,
-- 
2.56.0.611.g1030f0b940-goog