Fix erroneous handling of i/o's during a reset.

a) Set xs->status rather than cmd_tables->status because there will be
no interrupt processing to move it from cmd_tables->status to
xs->status.

b) Set cmd_c.status to correct value (CMDST_SENSE_DONE) when an active
sense command is reset.

c) Don't put a reset command from the ready queue into the free_list
twice, once in siop_scsicmd_end() and once manually.

Condition a) meant that the scsi layer was seeing successfully
completed i/o's (xs->error == XS_NOERROR) when they were in fact reset
and should have had xs->error == XS_TIMEOUT or xs->error == XS_RESET.
This meant lost data on output, and random or zero'ed data on input.

Condition b) meant that the wrong bus_dmamap_sync() was called, though
the actual action was apparently identical.

Condition c) meant that the free_list could become corrupt.

The problem was discovered by pb@ on a heavily loaded server that
experienced timeouts. This fix was tested by pb@ and henric@ to prove
it did not affect normal processing. If nothing else it will provide
better error messages if the problem is ever encountered again.

Probably a good candidate for -stable if pb@ can successfully
reproduce his timeout problems and not have his server crash.

1 files changed, 21 insertions, 24 deletions

diff --git a/sys/dev/ic/siop.c b/sys/dev/ic/siop.c
index ebbf44d1af8..6593df381a0 100644
--- a/sys/dev/ic/siop.c
+++ b/sys/dev/ic/siop.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: siop.c,v 1.25 2003/01/21 00:48:55 krw Exp $ */
+/*	$OpenBSD: siop.c,v 1.26 2003/06/09 02:28:13 krw Exp $ */
 /*	$NetBSD: siop.c,v 1.65 2002/11/08 22:04:41 bouyer Exp $	*/
 
 /*
@@ -1227,15 +1227,11 @@ siop_handle_reset(sc)
 				siop_cmd = siop_lun->siop_tag[tag].active;
 				if (siop_cmd == NULL)
 					continue;
-				sc_print_addr(siop_cmd->cmd_c.xs->sc_link);
-				printf("command with tag id %d reset\n", tag);
-				siop_cmd->cmd_c.xs->error =
-				    (siop_cmd->cmd_c.flags & CMDFL_TIMEOUT) ?
-		    		    XS_TIMEOUT : XS_RESET;
-				siop_cmd->cmd_c.xs->status = SCSI_SIOP_NOCHECK;
 				siop_lun->siop_tag[tag].active = NULL;
-				siop_cmd->cmd_c.status = CMDST_DONE;
-				siop_scsicmd_end(siop_cmd);
+				TAILQ_INSERT_TAIL(&reset_list, siop_cmd, next);
+				sc_print_addr(siop_cmd->cmd_c.xs->sc_link);
+				printf("cmd %p (tag %d) added to reset list\n",
+				    siop_cmd, tag);
 			}
 		}
 		sc->sc_c.targets[target]->status = TARST_ASYNC;
@@ -1248,41 +1244,42 @@ siop_handle_reset(sc)
 	for (siop_cmd = TAILQ_FIRST(&sc->urgent_list); siop_cmd != NULL;
 	    siop_cmd = next_siop_cmd) {
 		next_siop_cmd = TAILQ_NEXT(siop_cmd, next);
-		siop_cmd->cmd_c.flags &= ~CMDFL_TAG;
-		printf("cmd %p (target %d:%d) in reset list (wait)\n",
-		    siop_cmd, siop_cmd->cmd_c.xs->sc_link->target,
-		    siop_cmd->cmd_c.xs->sc_link->lun);
 		TAILQ_REMOVE(&sc->urgent_list, siop_cmd, next);
 		TAILQ_INSERT_TAIL(&reset_list, siop_cmd, next);
+		sc_print_addr(siop_cmd->cmd_c.xs->sc_link);
+		printf("cmd %p added to reset list from urgent list\n",
+		    siop_cmd);
 	}
-	/* Then command waiting in the input list */
+	/* Then commands waiting in the input list. */
 	for (siop_cmd = TAILQ_FIRST(&sc->ready_list); siop_cmd != NULL;
 	    siop_cmd = next_siop_cmd) {
 		next_siop_cmd = TAILQ_NEXT(siop_cmd, next);
-		siop_cmd->cmd_c.flags &= ~CMDFL_TAG;
-		printf("cmd %p (target %d:%d) in reset list (wait)\n",
-		    siop_cmd, siop_cmd->cmd_c.xs->sc_link->target,
-		    siop_cmd->cmd_c.xs->sc_link->lun);
 		TAILQ_REMOVE(&sc->ready_list, siop_cmd, next);
 		TAILQ_INSERT_TAIL(&reset_list, siop_cmd, next);
+		sc_print_addr(siop_cmd->cmd_c.xs->sc_link);
+		printf("cmd %p added to reset list from ready list\n",
+		    siop_cmd);
 	}
 
 	for (siop_cmd = TAILQ_FIRST(&reset_list); siop_cmd != NULL;
 	    siop_cmd = next_siop_cmd) {
 		next_siop_cmd = TAILQ_NEXT(siop_cmd, next);
-		siop_cmd->cmd_c.xs->error = (siop_cmd->cmd_c.flags & CMDFL_TIMEOUT) ?
-		    XS_TIMEOUT : XS_RESET;
-		siop_cmd->cmd_tables->status = htole32(SCSI_SIOP_NOCHECK);
-		printf("cmd %p (status %d) about to be processed\n", siop_cmd,
-		    siop_cmd->cmd_c.status);
+		siop_cmd->cmd_c.flags &= ~CMDFL_TAG;
+		siop_cmd->cmd_c.xs->error =
+		    (siop_cmd->cmd_c.flags & CMDFL_TIMEOUT)
+		    ? XS_TIMEOUT : XS_RESET;
+		siop_cmd->cmd_c.xs->status = SCSI_SIOP_NOCHECK;
+		printf("cmd %p (status %d) reset ",
+		    siop_cmd, siop_cmd->cmd_c.status);
 		if (siop_cmd->cmd_c.status == CMDST_SENSE ||
 		    siop_cmd->cmd_c.status == CMDST_SENSE_ACTIVE) 
 			siop_cmd->cmd_c.status = CMDST_SENSE_DONE;
 		else
 			siop_cmd->cmd_c.status = CMDST_DONE;
+		printf(" with status %d, xs->error %d\n",
+		    siop_cmd->cmd_c.status, siop_cmd->cmd_c.xs->error);
 		TAILQ_REMOVE(&reset_list, siop_cmd, next);
 		siop_scsicmd_end(siop_cmd);
-		TAILQ_INSERT_TAIL(&sc->free_list, siop_cmd, next);
 	}
 }