All of lore.kernel.org
 help / color / mirror / Atom feed
From: Finn Thain <fthain@telegraphics.com.au>
To: "James E.J. Bottomley" <JBottomley@odin.com>,
	Michael Schmitz <schmitzmic@gmail.com>,
	<linux-m68k@vger.kernel.org>, <linux-scsi@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: [PATCH v2 59/72] ncr5380: Fix autosense bugs
Date: Sun, 06 Dec 2015 12:32:25 +1100	[thread overview]
Message-ID: <20151206013141.595517451@telegraphics.com.au> (raw)
In-Reply-To: 20151206013126.995379403@telegraphics.com.au

[-- Attachment #1: ncr5380-autosense-queue --]
[-- Type: text/plain, Size: 21555 bytes --]

NCR5380_information_transfer() may re-queue a command for autosense,
after calling scsi_eh_prep_cmnd(). This creates several possibilities:

1. Reselection may intervene before the re-queued command gets processed.
   If the reconnected command then undergoes autosense, this causes the
   scsi_eh_save data from the previous command to be overwritten.

2. After NCR5380_information_transfer() calls scsi_eh_prep_cmnd(),
   a new REQUEST SENSE command may arrive. This would be queued ahead
   of any command already undergoing autosense, which means the
   scsi_eh_save data might be restored to the wrong command.

3. After NCR5380_information_transfer() calls scsi_eh_prep_cmnd(),
   eh_abort_handler() may abort the command. But the scsi_eh_save data is
   not discarded, which means the scsi_eh_save data might be incorrectly
   restored to the next REQUEST SENSE command issued.

This patch adds a new autosense list so that commands that are re-queued
because of a CHECK CONDITION result can be kept apart from the REQUEST
SENSE commands that arrive via queuecommand.

This patch also adds a function dedicated to dequeueing and preparing the
next command for processing. By refactoring the main loop in this way,
scsi_eh_save takes place when an autosense command is dequeued rather
than when re-queued.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 drivers/scsi/NCR5380.c       |  194 +++++++++++++++++++---------------
 drivers/scsi/NCR5380.h       |    2 
 drivers/scsi/atari_NCR5380.c |  239 ++++++++++++++++++++++++-------------------
 3 files changed, 249 insertions(+), 186 deletions(-)

Index: linux/drivers/scsi/NCR5380.h
===================================================================
--- linux.orig/drivers/scsi/NCR5380.h	2015-12-06 12:30:58.000000000 +1100
+++ linux/drivers/scsi/NCR5380.h	2015-12-06 12:31:02.000000000 +1100
@@ -256,10 +256,12 @@ struct NCR5380_hostdata {
 	unsigned char last_message;		/* last message OUT */
 	struct scsi_cmnd *connected;		/* currently connected cmnd */
 	struct list_head unissued;		/* waiting to be issued */
+	struct list_head autosense;		/* priority issue queue */
 	struct list_head disconnected;		/* waiting for reconnect */
 	spinlock_t lock;			/* protects this struct */
 	int flags;
 	struct scsi_eh_save ses;
+	struct scsi_cmnd *sensing;
 	char info[256];
 	int read_overruns;                /* number of bytes to cut from a
 	                                   * transfer to handle chip overruns */
Index: linux/drivers/scsi/atari_NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/atari_NCR5380.c	2015-12-06 12:31:01.000000000 +1100
+++ linux/drivers/scsi/atari_NCR5380.c	2015-12-06 12:31:02.000000000 +1100
@@ -418,6 +418,9 @@ static inline void initialize_SCp(struct
 		cmd->SCp.ptr = NULL;
 		cmd->SCp.this_residual = 0;
 	}
+
+	cmd->SCp.Status = 0;
+	cmd->SCp.Message = 0;
 }
 
 /**
@@ -661,6 +664,8 @@ static int __init NCR5380_init(struct Sc
 #endif
 	spin_lock_init(&hostdata->lock);
 	hostdata->connected = NULL;
+	hostdata->sensing = NULL;
+	INIT_LIST_HEAD(&hostdata->autosense);
 	INIT_LIST_HEAD(&hostdata->unissued);
 	INIT_LIST_HEAD(&hostdata->disconnected);
 
@@ -772,6 +777,16 @@ static void complete_cmd(struct Scsi_Hos
 
 	dsprintk(NDEBUG_QUEUES, instance, "complete_cmd: cmd %p\n", cmd);
 
+	if (hostdata->sensing == cmd) {
+		/* Autosense processing ends here */
+		if ((cmd->result & 0xff) != SAM_STAT_GOOD) {
+			scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+			set_host_byte(cmd, DID_ERROR);
+		} else
+			scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+		hostdata->sensing = NULL;
+	}
+
 #ifdef SUPPORT_TAGS
 	cmd_free_tag(cmd);
 #else
@@ -863,12 +878,77 @@ static inline void maybe_release_dma_irq
 	/* Caller does the locking needed to set & test these data atomically */
 	if (list_empty(&hostdata->disconnected) &&
 	    list_empty(&hostdata->unissued) &&
+	    list_empty(&hostdata->autosense) &&
 	    !hostdata->connected &&
 	    !hostdata->retain_dma_intr)
 		NCR5380_release_dma_irq(instance);
 }
 
 /**
+ * dequeue_next_cmd - dequeue a command for processing
+ * @instance: the scsi host instance
+ *
+ * Priority is given to commands on the autosense queue. These commands
+ * need autosense because of a CHECK CONDITION result.
+ *
+ * Returns a command pointer if a command is found for a target that is
+ * not already busy. Otherwise returns NULL.
+ */
+
+static struct scsi_cmnd *dequeue_next_cmd(struct Scsi_Host *instance)
+{
+	struct NCR5380_hostdata *hostdata = shost_priv(instance);
+	struct NCR5380_cmd *ncmd;
+	struct scsi_cmnd *cmd;
+
+	if (list_empty(&hostdata->autosense)) {
+		list_for_each_entry(ncmd, &hostdata->unissued, list) {
+			cmd = NCR5380_to_scmd(ncmd);
+			dsprintk(NDEBUG_QUEUES, instance, "dequeue: cmd=%p target=%d busy=0x%02x lun=%llu\n",
+			         cmd, scmd_id(cmd), hostdata->busy[scmd_id(cmd)], cmd->device->lun);
+
+			if (
+#ifdef SUPPORT_TAGS
+			    !is_lun_busy(cmd, 1)
+#else
+			    !(hostdata->busy[scmd_id(cmd)] & (1 << cmd->device->lun))
+#endif
+			) {
+				list_del(&ncmd->list);
+				dsprintk(NDEBUG_QUEUES, instance,
+				         "dequeue: removed %p from issue queue\n", cmd);
+				return cmd;
+			}
+		}
+	} else {
+		/* Autosense processing begins here */
+		ncmd = list_first_entry(&hostdata->autosense,
+		                        struct NCR5380_cmd, list);
+		list_del(&ncmd->list);
+		cmd = NCR5380_to_scmd(ncmd);
+		dsprintk(NDEBUG_QUEUES, instance,
+		         "dequeue: removed %p from autosense queue\n", cmd);
+		scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
+		hostdata->sensing = cmd;
+		return cmd;
+	}
+	return NULL;
+}
+
+static void requeue_cmd(struct Scsi_Host *instance, struct scsi_cmnd *cmd)
+{
+	struct NCR5380_hostdata *hostdata = shost_priv(instance);
+	struct NCR5380_cmd *ncmd = scsi_cmd_priv(cmd);
+
+	if (hostdata->sensing) {
+		scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+		list_add(&ncmd->list, &hostdata->autosense);
+		hostdata->sensing = NULL;
+	} else
+		list_add(&ncmd->list, &hostdata->unissued);
+}
+
+/**
  * NCR5380_main - NCR state machines
  *
  * NCR5380_main is a coroutine that runs as long as more work can
@@ -884,7 +964,7 @@ static void NCR5380_main(struct work_str
 	struct NCR5380_hostdata *hostdata =
 		container_of(work, struct NCR5380_hostdata, main_task);
 	struct Scsi_Host *instance = hostdata->host;
-	struct NCR5380_cmd *ncmd, *n;
+	struct scsi_cmnd *cmd;
 	int done;
 
 	/*
@@ -897,75 +977,46 @@ static void NCR5380_main(struct work_str
 	do {
 		done = 1;
 
-		if (!hostdata->connected) {
-			dprintk(NDEBUG_MAIN, "scsi%d: not connected\n", HOSTNO);
-			/*
-			 * Search through the issue_queue for a command destined
-			 * for a target that's not busy.
-			 */
-			list_for_each_entry_safe(ncmd, n, &hostdata->unissued,
-			                         list) {
-				struct scsi_cmnd *tmp = NCR5380_to_scmd(ncmd);
-				u8 lun = tmp->device->lun;
-
-				dsprintk(NDEBUG_QUEUES, instance, "main: tmp=%p target=%d busy=%d lun=%d\n",
-				         tmp, scmd_id(tmp), hostdata->busy[scmd_id(tmp)], lun);
-				/*  When we find one, remove it from the issue queue. */
-				if (
-#ifdef SUPPORT_TAGS
-				    !is_lun_busy( tmp, tmp->cmnd[0] != REQUEST_SENSE)
-#else
-				    !(hostdata->busy[tmp->device->id] & (1 << lun))
-#endif
-				    ) {
-					list_del(&ncmd->list);
-					dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES,
-					         instance, "main: removed %p from issue queue\n",
-					         tmp);
+		while (!hostdata->connected &&
+		       (cmd = dequeue_next_cmd(instance))) {
 
-					hostdata->retain_dma_intr++;
+			dsprintk(NDEBUG_MAIN, instance, "main: dequeued %p\n", cmd);
 
-					/*
-					 * Attempt to establish an I_T_L nexus here.
-					 * On success, instance->hostdata->connected is set.
-					 * On failure, we must add the command back to the
-					 *   issue queue so we can keep trying.
-					 */
-					/*
-					 * REQUEST SENSE commands are issued without tagged
-					 * queueing, even on SCSI-II devices because the
-					 * contingent allegiance condition exists for the
-					 * entire unit.
-					 */
-					/* ++roman: ...and the standard also requires that
-					 * REQUEST SENSE command are untagged.
-					 */
+			/*
+			 * Attempt to establish an I_T_L nexus here.
+			 * On success, instance->hostdata->connected is set.
+			 * On failure, we must add the command back to the
+			 * issue queue so we can keep trying.
+			 */
+			/*
+			 * REQUEST SENSE commands are issued without tagged
+			 * queueing, even on SCSI-II devices because the
+			 * contingent allegiance condition exists for the
+			 * entire unit.
+			 */
+			/* ++roman: ...and the standard also requires that
+			 * REQUEST SENSE command are untagged.
+			 */
 
 #ifdef SUPPORT_TAGS
-					cmd_get_tag(tmp, tmp->cmnd[0] != REQUEST_SENSE);
+			cmd_get_tag(cmd, cmd->cmnd[0] != REQUEST_SENSE);
 #endif
-					if (!NCR5380_select(instance, tmp)) {
-						/* OK or bad target */
-						hostdata->retain_dma_intr--;
-						maybe_release_dma_irq(instance);
-					} else {
-						/* Need to retry */
-						list_add(&ncmd->list, &hostdata->unissued);
-						dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES,
-						         instance, "main: select() failed, %p returned to issue queue\n",
-						         tmp);
+			hostdata->retain_dma_intr++;
+			if (!NCR5380_select(instance, cmd)) {
+				dsprintk(NDEBUG_MAIN, instance, "main: selected target %d for command %p\n",
+				         scmd_id(cmd), cmd);
+				hostdata->retain_dma_intr--;
+				maybe_release_dma_irq(instance);
+			} else {
+				hostdata->retain_dma_intr--;
+				dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES, instance,
+				         "main: select failed, returning %p to queue\n", cmd);
+				requeue_cmd(instance, cmd);
 #ifdef SUPPORT_TAGS
-						cmd_free_tag(tmp);
+				cmd_free_tag(cmd);
 #endif
-						hostdata->retain_dma_intr--;
-						done = 0;
-					}
-					if (hostdata->connected)
-						break;
-				} /* if target/lun/target queue is not busy */
-			} /* for issue_queue */
-		} /* if (!hostdata->connected) */
-
+			}
+		}
 		if (hostdata->connected
 #ifdef REAL_DMA
 		    && !hostdata->dma_len
@@ -1997,48 +2048,21 @@ static void NCR5380_information_transfer
 					}
 #endif
 
-					/*
-					 * I'm not sure what the correct thing to do here is :
-					 *
-					 * If the command that just executed is NOT a request
-					 * sense, the obvious thing to do is to set the result
-					 * code to the values of the stored parameters.
-					 *
-					 * If it was a REQUEST SENSE command, we need some way to
-					 * differentiate between the failure code of the original
-					 * and the failure code of the REQUEST sense - the obvious
-					 * case is success, where we fall through and leave the
-					 * result code unchanged.
-					 *
-					 * The non-obvious place is where the REQUEST SENSE failed
-					 */
+					cmd->result &= ~0xffff;
+					cmd->result |= cmd->SCp.Status;
+					cmd->result |= cmd->SCp.Message << 8;
 
-					if (cmd->cmnd[0] != REQUEST_SENSE)
-						cmd->result = cmd->SCp.Status | (cmd->SCp.Message << 8);
-					else if (status_byte(cmd->SCp.Status) != GOOD)
-						cmd->result = (cmd->result & 0x00ffff) | (DID_ERROR << 16);
-
-					if ((cmd->cmnd[0] == REQUEST_SENSE) &&
-						hostdata->ses.cmd_len) {
-						scsi_eh_restore_cmnd(cmd, &hostdata->ses);
-						hostdata->ses.cmd_len = 0 ;
-					}
-
-					if ((cmd->cmnd[0] != REQUEST_SENSE) &&
-					    (status_byte(cmd->SCp.Status) == CHECK_CONDITION)) {
-						scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
-
-						list_add(&ncmd->list, &hostdata->unissued);
-						dsprintk(NDEBUG_AUTOSENSE | NDEBUG_QUEUES,
-						         instance, "REQUEST SENSE cmd %p added to head of issue queue\n",
-						         cmd);
-#ifdef SUPPORT_TAGS
-						cmd_free_tag(cmd);
-#else
-						hostdata->busy[cmd->device->id] &= ~(1 << cmd->device->lun);
-#endif
-					} else {
+					if (cmd->cmnd[0] == REQUEST_SENSE)
 						complete_cmd(instance, cmd);
+					else {
+						if (cmd->SCp.Status == SAM_STAT_CHECK_CONDITION ||
+						    cmd->SCp.Status == SAM_STAT_COMMAND_TERMINATED) {
+							dsprintk(NDEBUG_QUEUES, instance, "autosense: adding cmd %p to tail of autosense queue\n",
+							         cmd);
+							list_add_tail(&ncmd->list,
+							              &hostdata->autosense);
+						} else
+							complete_cmd(instance, cmd);
 					}
 
 					/*
@@ -2528,6 +2552,15 @@ static int NCR5380_bus_reset(struct scsi
 		dsprintk(NDEBUG_ABORT, instance, "reset aborted a connected command\n");
 	hostdata->connected = NULL;
 
+	if (hostdata->sensing) {
+		complete_cmd(instance, hostdata->sensing);
+		hostdata->sensing = NULL;
+	}
+
+	if (!list_empty(&hostdata->autosense))
+		dsprintk(NDEBUG_ABORT, instance, "reset aborted autosense list\n");
+	INIT_LIST_HEAD(&hostdata->autosense);
+
 	if (!list_empty(&hostdata->unissued))
 		dsprintk(NDEBUG_ABORT, instance, "reset aborted unissued list\n");
 	INIT_LIST_HEAD(&hostdata->unissued);
Index: linux/drivers/scsi/NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/NCR5380.c	2015-12-06 12:31:01.000000000 +1100
+++ linux/drivers/scsi/NCR5380.c	2015-12-06 12:31:02.000000000 +1100
@@ -244,6 +244,9 @@ static inline void initialize_SCp(struct
 		cmd->SCp.ptr = NULL;
 		cmd->SCp.this_residual = 0;
 	}
+
+	cmd->SCp.Status = 0;
+	cmd->SCp.Message = 0;
 }
 
 /**
@@ -622,6 +625,8 @@ static int NCR5380_init(struct Scsi_Host
 #endif
 	spin_lock_init(&hostdata->lock);
 	hostdata->connected = NULL;
+	hostdata->sensing = NULL;
+	INIT_LIST_HEAD(&hostdata->autosense);
 	INIT_LIST_HEAD(&hostdata->unissued);
 	INIT_LIST_HEAD(&hostdata->disconnected);
 
@@ -733,6 +738,16 @@ static void complete_cmd(struct Scsi_Hos
 
 	dsprintk(NDEBUG_QUEUES, instance, "complete_cmd: cmd %p\n", cmd);
 
+	if (hostdata->sensing == cmd) {
+		/* Autosense processing ends here */
+		if ((cmd->result & 0xff) != SAM_STAT_GOOD) {
+			scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+			set_host_byte(cmd, DID_ERROR);
+		} else
+			scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+		hostdata->sensing = NULL;
+	}
+
 	hostdata->busy[scmd_id(cmd)] &= ~(1 << cmd->device->lun);
 
 	cmd->scsi_done(cmd);
@@ -793,6 +808,64 @@ static int NCR5380_queue_command(struct
 }
 
 /**
+ * dequeue_next_cmd - dequeue a command for processing
+ * @instance: the scsi host instance
+ *
+ * Priority is given to commands on the autosense queue. These commands
+ * need autosense because of a CHECK CONDITION result.
+ *
+ * Returns a command pointer if a command is found for a target that is
+ * not already busy. Otherwise returns NULL.
+ */
+
+static struct scsi_cmnd *dequeue_next_cmd(struct Scsi_Host *instance)
+{
+	struct NCR5380_hostdata *hostdata = shost_priv(instance);
+	struct NCR5380_cmd *ncmd;
+	struct scsi_cmnd *cmd;
+
+	if (list_empty(&hostdata->autosense)) {
+		list_for_each_entry(ncmd, &hostdata->unissued, list) {
+			cmd = NCR5380_to_scmd(ncmd);
+			dsprintk(NDEBUG_QUEUES, instance, "dequeue: cmd=%p target=%d busy=0x%02x lun=%llu\n",
+			         cmd, scmd_id(cmd), hostdata->busy[scmd_id(cmd)], cmd->device->lun);
+
+			if (!(hostdata->busy[scmd_id(cmd)] & (1 << cmd->device->lun))) {
+				list_del(&ncmd->list);
+				dsprintk(NDEBUG_QUEUES, instance,
+				         "dequeue: removed %p from issue queue\n", cmd);
+				return cmd;
+			}
+		}
+	} else {
+		/* Autosense processing begins here */
+		ncmd = list_first_entry(&hostdata->autosense,
+		                        struct NCR5380_cmd, list);
+		list_del(&ncmd->list);
+		cmd = NCR5380_to_scmd(ncmd);
+		dsprintk(NDEBUG_QUEUES, instance,
+		         "dequeue: removed %p from autosense queue\n", cmd);
+		scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
+		hostdata->sensing = cmd;
+		return cmd;
+	}
+	return NULL;
+}
+
+static void requeue_cmd(struct Scsi_Host *instance, struct scsi_cmnd *cmd)
+{
+	struct NCR5380_hostdata *hostdata = shost_priv(instance);
+	struct NCR5380_cmd *ncmd = scsi_cmd_priv(cmd);
+
+	if (hostdata->sensing) {
+		scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+		list_add(&ncmd->list, &hostdata->autosense);
+		hostdata->sensing = NULL;
+	} else
+		list_add(&ncmd->list, &hostdata->unissued);
+}
+
+/**
  *	NCR5380_main	-	NCR state machines
  *
  *	NCR5380_main is a coroutine that runs as long as more work can 
@@ -809,63 +882,40 @@ static void NCR5380_main(struct work_str
 	struct NCR5380_hostdata *hostdata =
 		container_of(work, struct NCR5380_hostdata, main_task);
 	struct Scsi_Host *instance = hostdata->host;
-	struct NCR5380_cmd *ncmd, *n;
+	struct scsi_cmnd *cmd;
 	int done;
 	
 	spin_lock_irq(&hostdata->lock);
 	do {
 		done = 1;
 
-		if (!hostdata->connected) {
-			dprintk(NDEBUG_MAIN, "scsi%d : not connected\n", instance->host_no);
+		while (!hostdata->connected &&
+		       (cmd = dequeue_next_cmd(instance))) {
+
+			dsprintk(NDEBUG_MAIN, instance, "main: dequeued %p\n", cmd);
+
 			/*
-			 * Search through the issue_queue for a command destined
-			 * for a target that's not busy.
+			 * Attempt to establish an I_T_L nexus here.
+			 * On success, instance->hostdata->connected is set.
+			 * On failure, we must add the command back to the
+			 * issue queue so we can keep trying.
+			 */
+			/*
+			 * REQUEST SENSE commands are issued without tagged
+			 * queueing, even on SCSI-II devices because the
+			 * contingent allegiance condition exists for the
+			 * entire unit.
 			 */
-			list_for_each_entry_safe(ncmd, n, &hostdata->unissued,
-			                         list) {
-				struct scsi_cmnd *tmp = NCR5380_to_scmd(ncmd);
-
-				dsprintk(NDEBUG_QUEUES, instance, "main: tmp=%p target=%d busy=%d lun=%llu\n",
-				         tmp, scmd_id(tmp), hostdata->busy[scmd_id(tmp)],
-				         tmp->device->lun);
-				/*  When we find one, remove it from the issue queue. */
-				if (!(hostdata->busy[tmp->device->id] &
-				      (1 << (u8)(tmp->device->lun & 0xff)))) {
-					list_del(&ncmd->list);
-					dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES,
-					         instance, "main: removed %p from issue queue\n",
-					         tmp);
-
-					/* 
-					 * Attempt to establish an I_T_L nexus here. 
-					 * On success, instance->hostdata->connected is set.
-					 * On failure, we must add the command back to the
-					 *   issue queue so we can keep trying. 
-					 */
-					/*
-					 * REQUEST SENSE commands are issued without tagged
-					 * queueing, even on SCSI-II devices because the
-					 * contingent allegiance condition exists for the
-					 * entire unit.
-					 */
-
-					if (!NCR5380_select(instance, tmp)) {
-						/* OK or bad target */
-					} else {
-						/* Need to retry */
-						list_add(&ncmd->list, &hostdata->unissued);
-						dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES,
-						         instance, "main: select() failed, %p returned to issue queue\n",
-						         tmp);
-						done = 0;
-					}
-					if (hostdata->connected)
-						break;
-				}	/* if target/lun is not busy */
-			}	/* for */
-		}	/* if (!hostdata->connected) */
 
+			if (!NCR5380_select(instance, cmd)) {
+				dsprintk(NDEBUG_MAIN, instance, "main: selected target %d for command %p\n",
+				         scmd_id(cmd), cmd);
+			} else {
+				dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES, instance,
+				         "main: select failed, returning %p to queue\n", cmd);
+				requeue_cmd(instance, cmd);
+			}
+		}
 		if (hostdata->connected
 #ifdef REAL_DMA
 		    && !hostdata->dmalen
@@ -1848,43 +1898,21 @@ static void NCR5380_information_transfer
 
 					hostdata->connected = NULL;
 
-					/* 
-					 * I'm not sure what the correct thing to do here is : 
-					 * 
-					 * If the command that just executed is NOT a request 
-					 * sense, the obvious thing to do is to set the result
-					 * code to the values of the stored parameters.
-					 * 
-					 * If it was a REQUEST SENSE command, we need some way 
-					 * to differentiate between the failure code of the original
-					 * and the failure code of the REQUEST sense - the obvious
-					 * case is success, where we fall through and leave the result
-					 * code unchanged.
-					 * 
-					 * The non-obvious place is where the REQUEST SENSE failed 
-					 */
-
-					if (cmd->cmnd[0] != REQUEST_SENSE)
-						cmd->result = cmd->SCp.Status | (cmd->SCp.Message << 8);
-					else if (status_byte(cmd->SCp.Status) != GOOD)
-						cmd->result = (cmd->result & 0x00ffff) | (DID_ERROR << 16);
-
-					if ((cmd->cmnd[0] == REQUEST_SENSE) &&
-						hostdata->ses.cmd_len) {
-						scsi_eh_restore_cmnd(cmd, &hostdata->ses);
-						hostdata->ses.cmd_len = 0 ;
-					}
-
-					if ((cmd->cmnd[0] != REQUEST_SENSE) && (status_byte(cmd->SCp.Status) == CHECK_CONDITION)) {
-						scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
+					cmd->result &= ~0xffff;
+					cmd->result |= cmd->SCp.Status;
+					cmd->result |= cmd->SCp.Message << 8;
 
-						list_add(&ncmd->list, &hostdata->unissued);
-						dsprintk(NDEBUG_AUTOSENSE | NDEBUG_QUEUES,
-						         instance, "REQUEST SENSE cmd %p added to head of issue queue\n",
-						         cmd);
-						hostdata->busy[cmd->device->id] &= ~(1 << (cmd->device->lun & 0xFF));
-					} else {
+					if (cmd->cmnd[0] == REQUEST_SENSE)
 						complete_cmd(instance, cmd);
+					else {
+						if (cmd->SCp.Status == SAM_STAT_CHECK_CONDITION ||
+						    cmd->SCp.Status == SAM_STAT_COMMAND_TERMINATED) {
+							dsprintk(NDEBUG_QUEUES, instance, "autosense: adding cmd %p to tail of autosense queue\n",
+							         cmd);
+							list_add_tail(&ncmd->list,
+							              &hostdata->autosense);
+						} else
+							complete_cmd(instance, cmd);
 					}
 
 					/* 



WARNING: multiple messages have this Message-ID (diff)
From: Finn Thain <fthain@telegraphics.com.au>
To: "James E.J. Bottomley" <JBottomley@odin.com>,
	Michael Schmitz <schmitzmic@gmail.com>,
	linux-m68k@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 59/72] ncr5380: Fix autosense bugs
Date: Sun, 06 Dec 2015 12:32:25 +1100	[thread overview]
Message-ID: <20151206013141.595517451@telegraphics.com.au> (raw)
In-Reply-To: 20151206013126.995379403@telegraphics.com.au

[-- Attachment #1: ncr5380-autosense-queue --]
[-- Type: text/plain, Size: 21553 bytes --]

NCR5380_information_transfer() may re-queue a command for autosense,
after calling scsi_eh_prep_cmnd(). This creates several possibilities:

1. Reselection may intervene before the re-queued command gets processed.
   If the reconnected command then undergoes autosense, this causes the
   scsi_eh_save data from the previous command to be overwritten.

2. After NCR5380_information_transfer() calls scsi_eh_prep_cmnd(),
   a new REQUEST SENSE command may arrive. This would be queued ahead
   of any command already undergoing autosense, which means the
   scsi_eh_save data might be restored to the wrong command.

3. After NCR5380_information_transfer() calls scsi_eh_prep_cmnd(),
   eh_abort_handler() may abort the command. But the scsi_eh_save data is
   not discarded, which means the scsi_eh_save data might be incorrectly
   restored to the next REQUEST SENSE command issued.

This patch adds a new autosense list so that commands that are re-queued
because of a CHECK CONDITION result can be kept apart from the REQUEST
SENSE commands that arrive via queuecommand.

This patch also adds a function dedicated to dequeueing and preparing the
next command for processing. By refactoring the main loop in this way,
scsi_eh_save takes place when an autosense command is dequeued rather
than when re-queued.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 drivers/scsi/NCR5380.c       |  194 +++++++++++++++++++---------------
 drivers/scsi/NCR5380.h       |    2 
 drivers/scsi/atari_NCR5380.c |  239 ++++++++++++++++++++++++-------------------
 3 files changed, 249 insertions(+), 186 deletions(-)

Index: linux/drivers/scsi/NCR5380.h
===================================================================
--- linux.orig/drivers/scsi/NCR5380.h	2015-12-06 12:30:58.000000000 +1100
+++ linux/drivers/scsi/NCR5380.h	2015-12-06 12:31:02.000000000 +1100
@@ -256,10 +256,12 @@ struct NCR5380_hostdata {
 	unsigned char last_message;		/* last message OUT */
 	struct scsi_cmnd *connected;		/* currently connected cmnd */
 	struct list_head unissued;		/* waiting to be issued */
+	struct list_head autosense;		/* priority issue queue */
 	struct list_head disconnected;		/* waiting for reconnect */
 	spinlock_t lock;			/* protects this struct */
 	int flags;
 	struct scsi_eh_save ses;
+	struct scsi_cmnd *sensing;
 	char info[256];
 	int read_overruns;                /* number of bytes to cut from a
 	                                   * transfer to handle chip overruns */
Index: linux/drivers/scsi/atari_NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/atari_NCR5380.c	2015-12-06 12:31:01.000000000 +1100
+++ linux/drivers/scsi/atari_NCR5380.c	2015-12-06 12:31:02.000000000 +1100
@@ -418,6 +418,9 @@ static inline void initialize_SCp(struct
 		cmd->SCp.ptr = NULL;
 		cmd->SCp.this_residual = 0;
 	}
+
+	cmd->SCp.Status = 0;
+	cmd->SCp.Message = 0;
 }
 
 /**
@@ -661,6 +664,8 @@ static int __init NCR5380_init(struct Sc
 #endif
 	spin_lock_init(&hostdata->lock);
 	hostdata->connected = NULL;
+	hostdata->sensing = NULL;
+	INIT_LIST_HEAD(&hostdata->autosense);
 	INIT_LIST_HEAD(&hostdata->unissued);
 	INIT_LIST_HEAD(&hostdata->disconnected);
 
@@ -772,6 +777,16 @@ static void complete_cmd(struct Scsi_Hos
 
 	dsprintk(NDEBUG_QUEUES, instance, "complete_cmd: cmd %p\n", cmd);
 
+	if (hostdata->sensing == cmd) {
+		/* Autosense processing ends here */
+		if ((cmd->result & 0xff) != SAM_STAT_GOOD) {
+			scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+			set_host_byte(cmd, DID_ERROR);
+		} else
+			scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+		hostdata->sensing = NULL;
+	}
+
 #ifdef SUPPORT_TAGS
 	cmd_free_tag(cmd);
 #else
@@ -863,12 +878,77 @@ static inline void maybe_release_dma_irq
 	/* Caller does the locking needed to set & test these data atomically */
 	if (list_empty(&hostdata->disconnected) &&
 	    list_empty(&hostdata->unissued) &&
+	    list_empty(&hostdata->autosense) &&
 	    !hostdata->connected &&
 	    !hostdata->retain_dma_intr)
 		NCR5380_release_dma_irq(instance);
 }
 
 /**
+ * dequeue_next_cmd - dequeue a command for processing
+ * @instance: the scsi host instance
+ *
+ * Priority is given to commands on the autosense queue. These commands
+ * need autosense because of a CHECK CONDITION result.
+ *
+ * Returns a command pointer if a command is found for a target that is
+ * not already busy. Otherwise returns NULL.
+ */
+
+static struct scsi_cmnd *dequeue_next_cmd(struct Scsi_Host *instance)
+{
+	struct NCR5380_hostdata *hostdata = shost_priv(instance);
+	struct NCR5380_cmd *ncmd;
+	struct scsi_cmnd *cmd;
+
+	if (list_empty(&hostdata->autosense)) {
+		list_for_each_entry(ncmd, &hostdata->unissued, list) {
+			cmd = NCR5380_to_scmd(ncmd);
+			dsprintk(NDEBUG_QUEUES, instance, "dequeue: cmd=%p target=%d busy=0x%02x lun=%llu\n",
+			         cmd, scmd_id(cmd), hostdata->busy[scmd_id(cmd)], cmd->device->lun);
+
+			if (
+#ifdef SUPPORT_TAGS
+			    !is_lun_busy(cmd, 1)
+#else
+			    !(hostdata->busy[scmd_id(cmd)] & (1 << cmd->device->lun))
+#endif
+			) {
+				list_del(&ncmd->list);
+				dsprintk(NDEBUG_QUEUES, instance,
+				         "dequeue: removed %p from issue queue\n", cmd);
+				return cmd;
+			}
+		}
+	} else {
+		/* Autosense processing begins here */
+		ncmd = list_first_entry(&hostdata->autosense,
+		                        struct NCR5380_cmd, list);
+		list_del(&ncmd->list);
+		cmd = NCR5380_to_scmd(ncmd);
+		dsprintk(NDEBUG_QUEUES, instance,
+		         "dequeue: removed %p from autosense queue\n", cmd);
+		scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
+		hostdata->sensing = cmd;
+		return cmd;
+	}
+	return NULL;
+}
+
+static void requeue_cmd(struct Scsi_Host *instance, struct scsi_cmnd *cmd)
+{
+	struct NCR5380_hostdata *hostdata = shost_priv(instance);
+	struct NCR5380_cmd *ncmd = scsi_cmd_priv(cmd);
+
+	if (hostdata->sensing) {
+		scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+		list_add(&ncmd->list, &hostdata->autosense);
+		hostdata->sensing = NULL;
+	} else
+		list_add(&ncmd->list, &hostdata->unissued);
+}
+
+/**
  * NCR5380_main - NCR state machines
  *
  * NCR5380_main is a coroutine that runs as long as more work can
@@ -884,7 +964,7 @@ static void NCR5380_main(struct work_str
 	struct NCR5380_hostdata *hostdata =
 		container_of(work, struct NCR5380_hostdata, main_task);
 	struct Scsi_Host *instance = hostdata->host;
-	struct NCR5380_cmd *ncmd, *n;
+	struct scsi_cmnd *cmd;
 	int done;
 
 	/*
@@ -897,75 +977,46 @@ static void NCR5380_main(struct work_str
 	do {
 		done = 1;
 
-		if (!hostdata->connected) {
-			dprintk(NDEBUG_MAIN, "scsi%d: not connected\n", HOSTNO);
-			/*
-			 * Search through the issue_queue for a command destined
-			 * for a target that's not busy.
-			 */
-			list_for_each_entry_safe(ncmd, n, &hostdata->unissued,
-			                         list) {
-				struct scsi_cmnd *tmp = NCR5380_to_scmd(ncmd);
-				u8 lun = tmp->device->lun;
-
-				dsprintk(NDEBUG_QUEUES, instance, "main: tmp=%p target=%d busy=%d lun=%d\n",
-				         tmp, scmd_id(tmp), hostdata->busy[scmd_id(tmp)], lun);
-				/*  When we find one, remove it from the issue queue. */
-				if (
-#ifdef SUPPORT_TAGS
-				    !is_lun_busy( tmp, tmp->cmnd[0] != REQUEST_SENSE)
-#else
-				    !(hostdata->busy[tmp->device->id] & (1 << lun))
-#endif
-				    ) {
-					list_del(&ncmd->list);
-					dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES,
-					         instance, "main: removed %p from issue queue\n",
-					         tmp);
+		while (!hostdata->connected &&
+		       (cmd = dequeue_next_cmd(instance))) {
 
-					hostdata->retain_dma_intr++;
+			dsprintk(NDEBUG_MAIN, instance, "main: dequeued %p\n", cmd);
 
-					/*
-					 * Attempt to establish an I_T_L nexus here.
-					 * On success, instance->hostdata->connected is set.
-					 * On failure, we must add the command back to the
-					 *   issue queue so we can keep trying.
-					 */
-					/*
-					 * REQUEST SENSE commands are issued without tagged
-					 * queueing, even on SCSI-II devices because the
-					 * contingent allegiance condition exists for the
-					 * entire unit.
-					 */
-					/* ++roman: ...and the standard also requires that
-					 * REQUEST SENSE command are untagged.
-					 */
+			/*
+			 * Attempt to establish an I_T_L nexus here.
+			 * On success, instance->hostdata->connected is set.
+			 * On failure, we must add the command back to the
+			 * issue queue so we can keep trying.
+			 */
+			/*
+			 * REQUEST SENSE commands are issued without tagged
+			 * queueing, even on SCSI-II devices because the
+			 * contingent allegiance condition exists for the
+			 * entire unit.
+			 */
+			/* ++roman: ...and the standard also requires that
+			 * REQUEST SENSE command are untagged.
+			 */
 
 #ifdef SUPPORT_TAGS
-					cmd_get_tag(tmp, tmp->cmnd[0] != REQUEST_SENSE);
+			cmd_get_tag(cmd, cmd->cmnd[0] != REQUEST_SENSE);
 #endif
-					if (!NCR5380_select(instance, tmp)) {
-						/* OK or bad target */
-						hostdata->retain_dma_intr--;
-						maybe_release_dma_irq(instance);
-					} else {
-						/* Need to retry */
-						list_add(&ncmd->list, &hostdata->unissued);
-						dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES,
-						         instance, "main: select() failed, %p returned to issue queue\n",
-						         tmp);
+			hostdata->retain_dma_intr++;
+			if (!NCR5380_select(instance, cmd)) {
+				dsprintk(NDEBUG_MAIN, instance, "main: selected target %d for command %p\n",
+				         scmd_id(cmd), cmd);
+				hostdata->retain_dma_intr--;
+				maybe_release_dma_irq(instance);
+			} else {
+				hostdata->retain_dma_intr--;
+				dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES, instance,
+				         "main: select failed, returning %p to queue\n", cmd);
+				requeue_cmd(instance, cmd);
 #ifdef SUPPORT_TAGS
-						cmd_free_tag(tmp);
+				cmd_free_tag(cmd);
 #endif
-						hostdata->retain_dma_intr--;
-						done = 0;
-					}
-					if (hostdata->connected)
-						break;
-				} /* if target/lun/target queue is not busy */
-			} /* for issue_queue */
-		} /* if (!hostdata->connected) */
-
+			}
+		}
 		if (hostdata->connected
 #ifdef REAL_DMA
 		    && !hostdata->dma_len
@@ -1997,48 +2048,21 @@ static void NCR5380_information_transfer
 					}
 #endif
 
-					/*
-					 * I'm not sure what the correct thing to do here is :
-					 *
-					 * If the command that just executed is NOT a request
-					 * sense, the obvious thing to do is to set the result
-					 * code to the values of the stored parameters.
-					 *
-					 * If it was a REQUEST SENSE command, we need some way to
-					 * differentiate between the failure code of the original
-					 * and the failure code of the REQUEST sense - the obvious
-					 * case is success, where we fall through and leave the
-					 * result code unchanged.
-					 *
-					 * The non-obvious place is where the REQUEST SENSE failed
-					 */
+					cmd->result &= ~0xffff;
+					cmd->result |= cmd->SCp.Status;
+					cmd->result |= cmd->SCp.Message << 8;
 
-					if (cmd->cmnd[0] != REQUEST_SENSE)
-						cmd->result = cmd->SCp.Status | (cmd->SCp.Message << 8);
-					else if (status_byte(cmd->SCp.Status) != GOOD)
-						cmd->result = (cmd->result & 0x00ffff) | (DID_ERROR << 16);
-
-					if ((cmd->cmnd[0] == REQUEST_SENSE) &&
-						hostdata->ses.cmd_len) {
-						scsi_eh_restore_cmnd(cmd, &hostdata->ses);
-						hostdata->ses.cmd_len = 0 ;
-					}
-
-					if ((cmd->cmnd[0] != REQUEST_SENSE) &&
-					    (status_byte(cmd->SCp.Status) == CHECK_CONDITION)) {
-						scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
-
-						list_add(&ncmd->list, &hostdata->unissued);
-						dsprintk(NDEBUG_AUTOSENSE | NDEBUG_QUEUES,
-						         instance, "REQUEST SENSE cmd %p added to head of issue queue\n",
-						         cmd);
-#ifdef SUPPORT_TAGS
-						cmd_free_tag(cmd);
-#else
-						hostdata->busy[cmd->device->id] &= ~(1 << cmd->device->lun);
-#endif
-					} else {
+					if (cmd->cmnd[0] == REQUEST_SENSE)
 						complete_cmd(instance, cmd);
+					else {
+						if (cmd->SCp.Status == SAM_STAT_CHECK_CONDITION ||
+						    cmd->SCp.Status == SAM_STAT_COMMAND_TERMINATED) {
+							dsprintk(NDEBUG_QUEUES, instance, "autosense: adding cmd %p to tail of autosense queue\n",
+							         cmd);
+							list_add_tail(&ncmd->list,
+							              &hostdata->autosense);
+						} else
+							complete_cmd(instance, cmd);
 					}
 
 					/*
@@ -2528,6 +2552,15 @@ static int NCR5380_bus_reset(struct scsi
 		dsprintk(NDEBUG_ABORT, instance, "reset aborted a connected command\n");
 	hostdata->connected = NULL;
 
+	if (hostdata->sensing) {
+		complete_cmd(instance, hostdata->sensing);
+		hostdata->sensing = NULL;
+	}
+
+	if (!list_empty(&hostdata->autosense))
+		dsprintk(NDEBUG_ABORT, instance, "reset aborted autosense list\n");
+	INIT_LIST_HEAD(&hostdata->autosense);
+
 	if (!list_empty(&hostdata->unissued))
 		dsprintk(NDEBUG_ABORT, instance, "reset aborted unissued list\n");
 	INIT_LIST_HEAD(&hostdata->unissued);
Index: linux/drivers/scsi/NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/NCR5380.c	2015-12-06 12:31:01.000000000 +1100
+++ linux/drivers/scsi/NCR5380.c	2015-12-06 12:31:02.000000000 +1100
@@ -244,6 +244,9 @@ static inline void initialize_SCp(struct
 		cmd->SCp.ptr = NULL;
 		cmd->SCp.this_residual = 0;
 	}
+
+	cmd->SCp.Status = 0;
+	cmd->SCp.Message = 0;
 }
 
 /**
@@ -622,6 +625,8 @@ static int NCR5380_init(struct Scsi_Host
 #endif
 	spin_lock_init(&hostdata->lock);
 	hostdata->connected = NULL;
+	hostdata->sensing = NULL;
+	INIT_LIST_HEAD(&hostdata->autosense);
 	INIT_LIST_HEAD(&hostdata->unissued);
 	INIT_LIST_HEAD(&hostdata->disconnected);
 
@@ -733,6 +738,16 @@ static void complete_cmd(struct Scsi_Hos
 
 	dsprintk(NDEBUG_QUEUES, instance, "complete_cmd: cmd %p\n", cmd);
 
+	if (hostdata->sensing == cmd) {
+		/* Autosense processing ends here */
+		if ((cmd->result & 0xff) != SAM_STAT_GOOD) {
+			scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+			set_host_byte(cmd, DID_ERROR);
+		} else
+			scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+		hostdata->sensing = NULL;
+	}
+
 	hostdata->busy[scmd_id(cmd)] &= ~(1 << cmd->device->lun);
 
 	cmd->scsi_done(cmd);
@@ -793,6 +808,64 @@ static int NCR5380_queue_command(struct
 }
 
 /**
+ * dequeue_next_cmd - dequeue a command for processing
+ * @instance: the scsi host instance
+ *
+ * Priority is given to commands on the autosense queue. These commands
+ * need autosense because of a CHECK CONDITION result.
+ *
+ * Returns a command pointer if a command is found for a target that is
+ * not already busy. Otherwise returns NULL.
+ */
+
+static struct scsi_cmnd *dequeue_next_cmd(struct Scsi_Host *instance)
+{
+	struct NCR5380_hostdata *hostdata = shost_priv(instance);
+	struct NCR5380_cmd *ncmd;
+	struct scsi_cmnd *cmd;
+
+	if (list_empty(&hostdata->autosense)) {
+		list_for_each_entry(ncmd, &hostdata->unissued, list) {
+			cmd = NCR5380_to_scmd(ncmd);
+			dsprintk(NDEBUG_QUEUES, instance, "dequeue: cmd=%p target=%d busy=0x%02x lun=%llu\n",
+			         cmd, scmd_id(cmd), hostdata->busy[scmd_id(cmd)], cmd->device->lun);
+
+			if (!(hostdata->busy[scmd_id(cmd)] & (1 << cmd->device->lun))) {
+				list_del(&ncmd->list);
+				dsprintk(NDEBUG_QUEUES, instance,
+				         "dequeue: removed %p from issue queue\n", cmd);
+				return cmd;
+			}
+		}
+	} else {
+		/* Autosense processing begins here */
+		ncmd = list_first_entry(&hostdata->autosense,
+		                        struct NCR5380_cmd, list);
+		list_del(&ncmd->list);
+		cmd = NCR5380_to_scmd(ncmd);
+		dsprintk(NDEBUG_QUEUES, instance,
+		         "dequeue: removed %p from autosense queue\n", cmd);
+		scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
+		hostdata->sensing = cmd;
+		return cmd;
+	}
+	return NULL;
+}
+
+static void requeue_cmd(struct Scsi_Host *instance, struct scsi_cmnd *cmd)
+{
+	struct NCR5380_hostdata *hostdata = shost_priv(instance);
+	struct NCR5380_cmd *ncmd = scsi_cmd_priv(cmd);
+
+	if (hostdata->sensing) {
+		scsi_eh_restore_cmnd(cmd, &hostdata->ses);
+		list_add(&ncmd->list, &hostdata->autosense);
+		hostdata->sensing = NULL;
+	} else
+		list_add(&ncmd->list, &hostdata->unissued);
+}
+
+/**
  *	NCR5380_main	-	NCR state machines
  *
  *	NCR5380_main is a coroutine that runs as long as more work can 
@@ -809,63 +882,40 @@ static void NCR5380_main(struct work_str
 	struct NCR5380_hostdata *hostdata =
 		container_of(work, struct NCR5380_hostdata, main_task);
 	struct Scsi_Host *instance = hostdata->host;
-	struct NCR5380_cmd *ncmd, *n;
+	struct scsi_cmnd *cmd;
 	int done;
 	
 	spin_lock_irq(&hostdata->lock);
 	do {
 		done = 1;
 
-		if (!hostdata->connected) {
-			dprintk(NDEBUG_MAIN, "scsi%d : not connected\n", instance->host_no);
+		while (!hostdata->connected &&
+		       (cmd = dequeue_next_cmd(instance))) {
+
+			dsprintk(NDEBUG_MAIN, instance, "main: dequeued %p\n", cmd);
+
 			/*
-			 * Search through the issue_queue for a command destined
-			 * for a target that's not busy.
+			 * Attempt to establish an I_T_L nexus here.
+			 * On success, instance->hostdata->connected is set.
+			 * On failure, we must add the command back to the
+			 * issue queue so we can keep trying.
+			 */
+			/*
+			 * REQUEST SENSE commands are issued without tagged
+			 * queueing, even on SCSI-II devices because the
+			 * contingent allegiance condition exists for the
+			 * entire unit.
 			 */
-			list_for_each_entry_safe(ncmd, n, &hostdata->unissued,
-			                         list) {
-				struct scsi_cmnd *tmp = NCR5380_to_scmd(ncmd);
-
-				dsprintk(NDEBUG_QUEUES, instance, "main: tmp=%p target=%d busy=%d lun=%llu\n",
-				         tmp, scmd_id(tmp), hostdata->busy[scmd_id(tmp)],
-				         tmp->device->lun);
-				/*  When we find one, remove it from the issue queue. */
-				if (!(hostdata->busy[tmp->device->id] &
-				      (1 << (u8)(tmp->device->lun & 0xff)))) {
-					list_del(&ncmd->list);
-					dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES,
-					         instance, "main: removed %p from issue queue\n",
-					         tmp);
-
-					/* 
-					 * Attempt to establish an I_T_L nexus here. 
-					 * On success, instance->hostdata->connected is set.
-					 * On failure, we must add the command back to the
-					 *   issue queue so we can keep trying. 
-					 */
-					/*
-					 * REQUEST SENSE commands are issued without tagged
-					 * queueing, even on SCSI-II devices because the
-					 * contingent allegiance condition exists for the
-					 * entire unit.
-					 */
-
-					if (!NCR5380_select(instance, tmp)) {
-						/* OK or bad target */
-					} else {
-						/* Need to retry */
-						list_add(&ncmd->list, &hostdata->unissued);
-						dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES,
-						         instance, "main: select() failed, %p returned to issue queue\n",
-						         tmp);
-						done = 0;
-					}
-					if (hostdata->connected)
-						break;
-				}	/* if target/lun is not busy */
-			}	/* for */
-		}	/* if (!hostdata->connected) */
 
+			if (!NCR5380_select(instance, cmd)) {
+				dsprintk(NDEBUG_MAIN, instance, "main: selected target %d for command %p\n",
+				         scmd_id(cmd), cmd);
+			} else {
+				dsprintk(NDEBUG_MAIN | NDEBUG_QUEUES, instance,
+				         "main: select failed, returning %p to queue\n", cmd);
+				requeue_cmd(instance, cmd);
+			}
+		}
 		if (hostdata->connected
 #ifdef REAL_DMA
 		    && !hostdata->dmalen
@@ -1848,43 +1898,21 @@ static void NCR5380_information_transfer
 
 					hostdata->connected = NULL;
 
-					/* 
-					 * I'm not sure what the correct thing to do here is : 
-					 * 
-					 * If the command that just executed is NOT a request 
-					 * sense, the obvious thing to do is to set the result
-					 * code to the values of the stored parameters.
-					 * 
-					 * If it was a REQUEST SENSE command, we need some way 
-					 * to differentiate between the failure code of the original
-					 * and the failure code of the REQUEST sense - the obvious
-					 * case is success, where we fall through and leave the result
-					 * code unchanged.
-					 * 
-					 * The non-obvious place is where the REQUEST SENSE failed 
-					 */
-
-					if (cmd->cmnd[0] != REQUEST_SENSE)
-						cmd->result = cmd->SCp.Status | (cmd->SCp.Message << 8);
-					else if (status_byte(cmd->SCp.Status) != GOOD)
-						cmd->result = (cmd->result & 0x00ffff) | (DID_ERROR << 16);
-
-					if ((cmd->cmnd[0] == REQUEST_SENSE) &&
-						hostdata->ses.cmd_len) {
-						scsi_eh_restore_cmnd(cmd, &hostdata->ses);
-						hostdata->ses.cmd_len = 0 ;
-					}
-
-					if ((cmd->cmnd[0] != REQUEST_SENSE) && (status_byte(cmd->SCp.Status) == CHECK_CONDITION)) {
-						scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
+					cmd->result &= ~0xffff;
+					cmd->result |= cmd->SCp.Status;
+					cmd->result |= cmd->SCp.Message << 8;
 
-						list_add(&ncmd->list, &hostdata->unissued);
-						dsprintk(NDEBUG_AUTOSENSE | NDEBUG_QUEUES,
-						         instance, "REQUEST SENSE cmd %p added to head of issue queue\n",
-						         cmd);
-						hostdata->busy[cmd->device->id] &= ~(1 << (cmd->device->lun & 0xFF));
-					} else {
+					if (cmd->cmnd[0] == REQUEST_SENSE)
 						complete_cmd(instance, cmd);
+					else {
+						if (cmd->SCp.Status == SAM_STAT_CHECK_CONDITION ||
+						    cmd->SCp.Status == SAM_STAT_COMMAND_TERMINATED) {
+							dsprintk(NDEBUG_QUEUES, instance, "autosense: adding cmd %p to tail of autosense queue\n",
+							         cmd);
+							list_add_tail(&ncmd->list,
+							              &hostdata->autosense);
+						} else
+							complete_cmd(instance, cmd);
 					}
 
 					/* 

  parent reply	other threads:[~2015-12-06  3:48 UTC|newest]

Thread overview: 156+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-06  1:31 [PATCH v2 00/72] More fixes, cleanup and modernization for NCR5380 drivers Finn Thain
2015-12-06  1:31 ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 01/72] atari_scsi: Fix SCSI host ID setting Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 02/72] ncr5380: Remove redundant static variable initializers Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 03/72] ncr5380: Eliminate PDEBUG*, TDEBUG* and DTCDEBUG* macros Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 04/72] ncr5380: Remove more pointless macros Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 05/72] ncr5380: Remove NCR5380_local_declare and NCR5380_setup macros Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 06/72] ncr5380: Remove NCR5380_instance_name macro Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 07/72] ncr5380: Split NCR5380_init() into two functions Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 08/72] ncr5380: Move NCR53C400-specific code Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 09/72] atari_NCR5380: Reset bus on driver initialization if required Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 10/72] atari_NCR5380: Remove RESET_BOOT, CONFIG_ATARI_SCSI_TOSHIBA_DELAY and CONFIG_ATARI_SCSI_RESET_BOOT Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 11/72] ncr5380: Simplify bus reset handlers Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 12/72] ncr5380: Remove unused hostdata->aborted flag Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 13/72] ncr5380: Remove redundant register writes Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 14/72] ncr5380: Use return instead of goto in NCR5380_select() Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 15/72] ncr5380: Always escalate bad target time-out " Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 16/72] ncr5380: Proceed with next command after NCR5380_select() calls scsi_done Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 17/72] ncr5380: Keep BSY asserted when entering SELECTION phase Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 18/72] ncr5380: Eliminate USLEEP_WAITLONG delay Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 19/72] ncr5380: Cleanup bogus {request,release}_region() calls Finn Thain
2015-12-06  1:31   ` [PATCH v2 19/72] ncr5380: Cleanup bogus {request, release}_region() calls Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 20/72] ncr5380: Introduce unbound workqueue Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 21/72] ncr5380: Sleep when polling, if possible Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06 10:18   ` Geert Uytterhoeven
2015-12-06 22:07     ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 22/72] ncr5380: Eliminate selecting state Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 23/72] ncr5380: Always retry arbitration and selection Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 24/72] ncr5380: Implement NCR5380_dma_xfer_len and remove LIMIT_TRANSFERSIZE macro Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 25/72] ncr5380: Rework disconnect versus poll logic Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 26/72] ncr5380: Fix NCR5380_transfer_pio() result Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 27/72] ncr5380: Add missing lock in eh_abort_handler Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 28/72] ncr5380: Drop DEF_SCSI_QCMD macro Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 29/72] ncr5380: Remove references to linked commands Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 30/72] ncr5380: Add missing break after case MESSAGE_REJECT Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 31/72] ncr5380: Fix !REQ timeout in do_abort() Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 32/72] ncr5380: Fix bus phase " Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:31 ` [PATCH v2 33/72] atari_NCR5380: Set do_abort() timeouts Finn Thain
2015-12-06  1:31   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 34/72] atari_NCR5380: Use arbitration timeout Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 35/72] ncr5380: Dont wait for BUS FREE after disconnect Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 36/72] ncr5380: Use work_struct instead of delayed_work Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 37/72] ncr5380: Standardize work queueing algorithm Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 38/72] ncr5380: Remove UNSAFE macro Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 39/72] ncr5380: Standardize interrupt handling Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 40/72] ncr5380: Introduce NCR5380_poll_politely2 Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 41/72] ncr5380: Replace redundant flags with FLAG_NO_DMA_FIXUP Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 42/72] ncr5380: Replace READ_OVERRUNS macro with FLAG_NO_DMA_FIXUPS Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 43/72] ncr5380: Standardize reselection handling Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 44/72] ncr5380: Fix off-by-one bug in extended_msg[] bounds check Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 45/72] ncr5380: Cleanup #include directives Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 46/72] ncr5380: Fix NDEBUG_NO_DATAOUT flag Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 47/72] ncr5380: Fix and cleanup scsi_host_template initializers Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 48/72] atari_NCR5380: Fix queue_size limit Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 49/72] ncr5380: Remove redundant ICR_ARBITRATION_LOST test and eliminate FLAG_DTC3181E Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 50/72] ncr5380: Change instance->host_lock to hostdata->lock Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 51/72] ncr5380: Remove command list debug code Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 52/72] ncr5380: Remove H_NO macro and introduce dsprintk Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 53/72] ncr5380: Use shost_priv helper Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 54/72] ncr5380: Use dsprintk() for queue debugging Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 55/72] ncr5380: Remove LIST and REMOVE macros Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 56/72] ncr5380: Remove redundant volatile qualifiers Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 57/72] ncr5380: Use standard list data structure Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 58/72] ncr5380: Refactor command completion Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` Finn Thain [this message]
2015-12-06  1:32   ` [PATCH v2 59/72] ncr5380: Fix autosense bugs Finn Thain
2015-12-06  1:32 ` [PATCH v2 60/72] ncr5380: Implement new eh_abort_handler Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 61/72] ncr5380: Fix EH during arbitration and selection Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 62/72] ncr5380: Implement new eh_bus_reset_handler Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 63/72] atari_NCR5380: Remove HOSTNO macro from printk() and seq_printf() calls Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 64/72] atari_NCR5380: Eliminate HOSTNO macro Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 65/72] atari_scsi, sun3_scsi: Remove global Scsi_Host pointer Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 66/72] ncr5380: Fix soft lockups Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 67/72] ncr5380: Cleanup comments Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 68/72] ncr5380: Fix whitespace issues using regexp Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 69/72] ncr5380: Merge changes from atari_NCR5380.c Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 70/72] atari_NCR5380: Merge changes from NCR5380.c Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 71/72] ncr5380: Cleanup whitespace and parentheses Finn Thain
2015-12-06  1:32   ` Finn Thain
2015-12-06  1:32 ` [PATCH v2 72/72] ncr5380: Fix pseudo DMA transfers on 53C400 Finn Thain
2015-12-06  1:32   ` Finn Thain

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=20151206013141.595517451@telegraphics.com.au \
    --to=fthain@telegraphics.com.au \
    --cc=JBottomley@odin.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=schmitzmic@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.