Turn on serialization of task management commands going down to the

controller, but make it optional.

After a problem report from Andrew Boyer, it looks like the LSI
chip may have issues (the watchdog timer fired) if too many aborts
are sent down to the chip at the same time.  We know that task
management commands are serialized, and although the manual doesn't
say it, it may be a good idea to just send one at a time.

But, since I'm not certain that this is necessary, add a tunable
and sysctl variable (hw.mps.%d.allow_multiple_tm_cmds) to control
the driver's behavior.

mps.c:		Add support for the sysctl and tunable, and add a
		comment about the possible return values to
		mps_map_command().

mps_sas.c:	Run all task management commands through two new
		routines, mpssas_issue_tm_request() and
		mpssas_complete_tm_request().

		This allows us to optionally serialize task
		management commands.  Also, change things so that
		the response to a task management command always
		comes back through the callback.  (Before it could
		come via the callback or the return value.)

mpsvar.h:	Add softc variables for the list of active task
		management commands, the number of active commands,
		and whether we should allow multiple active task
		management commands.  Add an active command flag.

mps.4:		Describe the new sysctl/loader tunable variable.

Sponsored by:	Spectra Logic Corporation

4 files changed, 241 insertions, 54 deletions

diff --git a/share/man/man4/mps.4 b/share/man/man4/mps.4
index 644e02f..c6dd527 100644
--- a/share/man/man4/mps.4
+++ b/share/man/man4/mps.4
@@ -31,7 +31,7 @@
 .\"
 .\" Author: Ken Merry <ken@FreeBSD.org>
 .\"
-.\" $Id: //depot/SpectraBSD/head/share/man/man4/mps.4#1 $
+.\" $Id: //depot/SpectraBSD/head/share/man/man4/mps.4#4 $
 .\" $FreeBSD$
 .\"
 .Dd September 13, 2010
@@ -97,6 +97,20 @@ driver instances, set the following tunable value in
 .Bd -literal -offset indent
 hw.mps.disable_msix=1
 .Ed
+.Pp
+To allow the driver to send multiple task management commands (like abort,
+LUN reset, etc.), set the following variable:
+.Bd -literal -offset indent
+hw.mps.X.allow_multiple_tm_cmds=1
+.Ed
+.Pp
+via
+.Xr loader.conf 5
+or
+.Xr sysctl 8 ,
+where X is the adapter number.
+By default the driver only sends one task management command at a time, to
+avoid causing a potential controller lock-up.
 .Sh DEBUGGING
 To enable debugging prints from the
 .Nm
diff --git a/sys/dev/mps/mps.c b/sys/dev/mps/mps.c
index d57c7df..548aa57 100644
--- a/sys/dev/mps/mps.c
+++ b/sys/dev/mps/mps.c
@@ -805,6 +805,9 @@ mps_attach(struct mps_softc *sc)
 	snprintf(tmpstr, sizeof(tmpstr), "hw.mps.%d.debug_level",
 	    device_get_unit(sc->mps_dev));
 	TUNABLE_INT_FETCH(tmpstr, &sc->mps_debug);
+	snprintf(tmpstr, sizeof(tmpstr), "hw.mps.%d.allow_multiple_tm_cmds",
+	    device_get_unit(sc->mps_dev));
+	TUNABLE_INT_FETCH(tmpstr, &sc->allow_multiple_tm_cmds);
 
 	mps_dprint(sc, MPS_TRACE, "%s\n", __func__);
 
@@ -831,6 +834,11 @@ mps_attach(struct mps_softc *sc)
 	    OID_AUTO, "debug_level", CTLFLAG_RW, &sc->mps_debug, 0,
 	    "mps debug level");
 
+	SYSCTL_ADD_INT(&sc->sysctl_ctx, SYSCTL_CHILDREN(sc->sysctl_tree),
+	    OID_AUTO, "allow_multiple_tm_cmds", CTLFLAG_RW,
+	    &sc->allow_multiple_tm_cmds, 0,
+	    "allow multiple simultaneous task management cmds");
+
 	if ((error = mps_transition_ready(sc)) != 0)
 		return (error);
 
@@ -873,6 +881,7 @@ mps_attach(struct mps_softc *sc)
 	    sc->facts->MaxReplyDescriptorPostQueueDepth) - 1;
 	TAILQ_INIT(&sc->req_list);
 	TAILQ_INIT(&sc->chain_list);
+	TAILQ_INIT(&sc->tm_list);
 
 	if (((error = mps_alloc_queues(sc)) != 0) ||
 	    ((error = mps_alloc_replies(sc)) != 0) ||
@@ -1470,6 +1479,10 @@ mps_data_cb(void *arg, bus_dma_segment_t *segs, int nsegs, int error)
 	return;
 }
 
+/*
+ * Note that the only error path here is from bus_dmamap_load(), which can
+ * return EINPROGRESS if it is waiting for resources.
+ */
 int
 mps_map_command(struct mps_softc *sc, struct mps_command *cm)
 {
diff --git a/sys/dev/mps/mps_sas.c b/sys/dev/mps/mps_sas.c
index f13d304..5883e49 100644
--- a/sys/dev/mps/mps_sas.c
+++ b/sys/dev/mps/mps_sas.c
@@ -135,9 +135,16 @@ static void mpssas_probe_device_complete(struct mps_softc *sc,
 static void mpssas_scsiio_timeout(void *data);
 static void mpssas_abort_complete(struct mps_softc *sc, struct mps_command *cm);
 static void mpssas_recovery(struct mps_softc *, struct mps_command *);
+static int mpssas_map_tm_request(struct mps_softc *sc, struct mps_command *cm);
+static void mpssas_issue_tm_request(struct mps_softc *sc,
+				    struct mps_command *cm);
+static void mpssas_tm_complete(struct mps_softc *sc, struct mps_command *cm,
+			       int error);
+static int mpssas_complete_tm_request(struct mps_softc *sc,
+				      struct mps_command *cm, int free_cm);
 static void mpssas_action_scsiio(struct mpssas_softc *, union ccb *);
 static void mpssas_scsiio_complete(struct mps_softc *, struct mps_command *);
-static int mpssas_resetdev(struct mpssas_softc *, struct mps_command *);
+static void mpssas_resetdev(struct mpssas_softc *, struct mps_command *);
 static void mpssas_action_resetdev(struct mpssas_softc *, union ccb *);
 static void mpssas_resetdev_complete(struct mps_softc *, struct mps_command *);
 static void mpssas_freeze_device(struct mpssas_softc *, struct mpssas_target *);
@@ -438,8 +445,7 @@ mpssas_prepare_remove(struct mpssas_softc *sassc, MPI2_EVENT_SAS_TOPO_PHY_ENTRY
 	cm->cm_desc.Default.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_DEFAULT_TYPE;
 	cm->cm_complete = mpssas_remove_device;
 	cm->cm_targ = targ;
-	xpt_freeze_simq(sc->sassc->sim, 1);
-	mps_map_command(sc, cm);
+	mpssas_issue_tm_request(sc, cm);
 }
 
 static void
@@ -454,7 +460,9 @@ mpssas_remove_device(struct mps_softc *sc, struct mps_command *cm)
 
 	reply = (MPI2_SCSI_TASK_MANAGE_REPLY *)cm->cm_reply;
 	handle = cm->cm_targ->handle;
-	xpt_release_simq(sc->sassc->sim, 1);
+
+	mpssas_complete_tm_request(sc, cm, /*free_cm*/ 0);
+
 	if (reply->IOCStatus != MPI2_IOCSTATUS_SUCCESS) {
 		mps_printf(sc, "Failure 0x%x reseting device 0x%04x\n", 
 		   reply->IOCStatus, handle);
@@ -1020,12 +1028,7 @@ mpssas_abort_complete(struct mps_softc *sc, struct mps_command *cm)
 	mps_printf(sc, "%s: abort request on handle %#04x SMID %d "
 		   "complete\n", __func__, req->DevHandle, req->TaskMID);
 
-	/*
-	 * Release the SIM queue, we froze it when we sent the abort.
-	 */
-	xpt_release_simq(sc->sassc->sim, 1);
-
-	mps_free_command(sc, cm);
+	mpssas_complete_tm_request(sc, cm, /*free_cm*/ 1);
 }
 
 static void
@@ -1033,7 +1036,6 @@ mpssas_recovery(struct mps_softc *sc, struct mps_command *abort_cm)
 {
 	struct mps_command *cm;
 	MPI2_SCSI_TASK_MANAGE_REQUEST *req, *orig_req;
-	int error;
 
 	cm = mps_alloc_command(sc);
 	if (cm == NULL) {
@@ -1055,34 +1057,204 @@ mpssas_recovery(struct mps_softc *sc, struct mps_command *abort_cm)
 	cm->cm_data = NULL;
 	cm->cm_desc.Default.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_DEFAULT_TYPE;
 
+	mpssas_issue_tm_request(sc, cm);
+
+}
+
+/*
+ * Can return 0 or EINPROGRESS on success.  Any other value means failure.
+ */
+static int
+mpssas_map_tm_request(struct mps_softc *sc, struct mps_command *cm)
+{
+	int error;
+
+	error = 0;
+
+	cm->cm_flags |= MPS_CM_FLAGS_ACTIVE;
+	error = mps_map_command(sc, cm);
+	if ((error == 0)
+	 || (error == EINPROGRESS))
+		sc->tm_cmds_active++;
+
+	return (error);
+}
+
+static void
+mpssas_issue_tm_request(struct mps_softc *sc, struct mps_command *cm)
+{
+	int freeze_queue, send_command, error;
+
+	freeze_queue = 0;
+	send_command = 0;
+	error = 0;
+
+	mtx_assert(&sc->mps_mtx, MA_OWNED);
+
 	/*
-	 * Freeze the SIM queue while we issue the abort.  According to the
-	 * Fusion-MPT 2.0 spec, task management requests are serialized,
-	 * and so the host should not send any I/O requests while task
-	 * management requests are pending.
+	 * If there are no other pending task management commands, go
+	 * ahead and send this one.  There is a small amount of anecdotal
+	 * evidence that sending lots of task management commands at once
+	 * may cause the controller to lock up.  Or, if the user has
+	 * configured the driver (via the allow_multiple_tm_cmds variable) to
+	 * not serialize task management commands, go ahead and send the
+	 * command if even other task management commands are pending.
 	 */
-	xpt_freeze_simq(sc->sassc->sim, 1);
+	if (TAILQ_FIRST(&sc->tm_list) == NULL) {
+		send_command = 1;
+		freeze_queue = 1;
+	} else if (sc->allow_multiple_tm_cmds != 0)
+		send_command = 1;
+
+	TAILQ_INSERT_TAIL(&sc->tm_list, cm, cm_link);
+	if (send_command != 0) {
+		/*
+		 * Freeze the SIM queue while we issue the task management
+		 * command.  According to the Fusion-MPT 2.0 spec, task
+		 * management requests are serialized, and so the host
+		 * should not send any I/O requests while task management
+		 * requests are pending.
+		 */
+		if (freeze_queue != 0)
+			xpt_freeze_simq(sc->sassc->sim, 1);
 
-	error = mps_map_command(sc, cm);
+		error = mpssas_map_tm_request(sc, cm);
 
-	if (error != 0) {
-		mps_printf(sc, "%s: error mapping abort request!\n", __func__);
-		xpt_release_simq(sc->sassc->sim, 1);
-	}
-#if 0
-	error = mpssas_reset(sc, targ, &resetcm);
-	if ((error != 0) && (error != EBUSY)) {
-		mps_printf(sc, "Error resetting device!\n");
-		mps_unlock(sc);
-		return;
+		/*
+		 * At present, there is no error path back from
+		 * mpssas_map_tm_request() (which calls mps_map_command())
+		 * when cm->cm_data == NULL.  But since there is a return
+		 * value, we check it just in case the implementation
+		 * changes later.
+		 */
+		if ((error != 0)
+		 && (error != EINPROGRESS))
+			mpssas_tm_complete(sc, cm,
+			    MPI2_SCSITASKMGMT_RSP_TM_FAILED);
 	}
+}
 
-	targ->flags |= MPSSAS_TARGET_INRESET;
+static void
+mpssas_tm_complete(struct mps_softc *sc, struct mps_command *cm, int error)
+{
+	MPI2_SCSI_TASK_MANAGE_REPLY *resp;
 
-	cm->cm_complete = mpssas_resettimeout_complete;
-	cm->cm_complete_data = cm;
-	mps_map_command(sassc->sc, cm);
-#endif
+	resp = (MPI2_SCSI_TASK_MANAGE_REPLY *)cm->cm_reply;
+
+	resp->ResponseCode = error;
+
+	/*
+	 * Call the callback for this command, it will be
+	 * removed from the list and freed via the callback.
+	 */
+	cm->cm_complete(sc, cm);
+}
+
+/*
+ * Complete a task management request.  The basic completion operation will
+ * always succeed.  Returns status for sending any further task management
+ * commands that were queued.
+ */
+static int
+mpssas_complete_tm_request(struct mps_softc *sc, struct mps_command *cm,
+			   int free_cm)
+{
+	int error;
+
+	error = 0;
+
+	mtx_assert(&sc->mps_mtx, MA_OWNED);
+
+	TAILQ_REMOVE(&sc->tm_list, cm, cm_link);
+	cm->cm_flags &= ~MPS_CM_FLAGS_ACTIVE;
+	sc->tm_cmds_active--;
+
+	if (free_cm != 0)
+		mps_free_command(sc, cm);
+
+	if (TAILQ_FIRST(&sc->tm_list) == NULL) {
+		/*
+		 * Release the SIM queue, we froze it when we sent the first
+		 * task management request.
+		 */
+		xpt_release_simq(sc->sassc->sim, 1);
+	} else if ((sc->tm_cmds_active == 0)
+		|| (sc->allow_multiple_tm_cmds != 0)) {
+		int error;
+		struct mps_command *cm2;
+
+restart_traversal:
+
+		/*
+		 * We don't bother using TAILQ_FOREACH_SAFE here, but
+		 * rather use the standard version and just restart the
+		 * list traversal if we run into the error case.
+		 * TAILQ_FOREACH_SAFE allows safe removal of the current
+		 * list element, but if you have a queue of task management
+		 * commands, all of which have mapping errors, you'll end
+		 * up with recursive calls to this routine and so you could
+		 * wind up removing more than just the current list element.
+		 */
+		TAILQ_FOREACH(cm2, &sc->tm_list, cm_link) {
+			MPI2_SCSI_TASK_MANAGE_REQUEST *req;
+
+			/* This command is active, no need to send it again */
+			if (cm2->cm_flags & MPS_CM_FLAGS_ACTIVE)
+				continue;
+
+			req = (MPI2_SCSI_TASK_MANAGE_REQUEST *)cm2->cm_req;
+
+			mps_printf(sc, "%s: sending deferred task management "
+			    "request for handle %#04x SMID %d\n", __func__,
+			    req->DevHandle, req->TaskMID);
+
+			error = mpssas_map_tm_request(sc, cm2);
+
+			/*
+			 * Check for errors.  If we had an error, complete
+			 * this command with an error, and keep going through
+			 * the list until we are able to send at least one
+			 * command or all of them are completed with errors.
+			 *
+			 * We don't want to wind up in a situation where
+			 * we're stalled out with no way for queued task
+			 * management commands to complete.
+			 *
+			 * Note that there is not currently an error path
+			 * back from mpssas_map_tm_request() (which calls
+			 * mps_map_command()) when cm->cm_data == NULL.
+			 * But we still want to check for errors here in
+			 * case the implementation changes, or in case
+			 * there is some reason for a data payload here.
+			 */
+			if ((error != 0)
+			 && (error != EINPROGRESS)) {
+				mpssas_tm_complete(sc, cm,
+				    MPI2_SCSITASKMGMT_RSP_TM_FAILED);
+
+				/*
+				 * If we don't currently have any commands
+				 * active, go back to the beginning and see
+				 * if there are any more that can be started.
+				 * Otherwise, we're done here.
+				 */
+				if (sc->tm_cmds_active == 0)
+					goto restart_traversal;
+				else
+					break;
+			}
+
+			/*
+			 * If the user only wants one task management command
+			 * active at a time, we're done, since we've
+			 * already successfully sent a command at this point.
+			 */
+			if (sc->allow_multiple_tm_cmds == 0)
+				break;
+		}
+	}
+
+	return (error);
 }
 
 static void
@@ -1359,7 +1531,6 @@ mpssas_action_resetdev(struct mpssas_softc *sassc, union ccb *ccb)
 	struct mps_softc *sc;
 	struct mps_command *cm;
 	struct mpssas_target *targ;
-	int error;
 
 	sc = sassc->sc;
 	targ = &sassc->targets[ccb->ccb_h.target_id];
@@ -1372,7 +1543,7 @@ mpssas_action_resetdev(struct mpssas_softc *sassc, union ccb *ccb)
 
 	cm = mps_alloc_command(sc);
 	if (cm == NULL) {
-		mps_printf(sc, "mpssas_action_resetdev: cannot alloc command\n");
+		mps_printf(sc, "%s: cannot alloc command\n", __func__);
 		ccb->ccb_h.status = CAM_RESRC_UNAVAIL;
 		xpt_done(ccb);
 		return;
@@ -1382,20 +1553,14 @@ mpssas_action_resetdev(struct mpssas_softc *sassc, union ccb *ccb)
 	cm->cm_complete = mpssas_resetdev_complete;
 	cm->cm_complete_data = ccb;
 
-	error = mpssas_resetdev(sassc, cm);
-	if (error) {
-		ccb->ccb_h.status = CAM_RESRC_UNAVAIL;
-		xpt_done(ccb);
-		return;
-	}
+	mpssas_resetdev(sassc, cm);
 }
 
-static int
+static void
 mpssas_resetdev(struct mpssas_softc *sassc, struct mps_command *cm)
 {
 	MPI2_SCSI_TASK_MANAGE_REQUEST *req;
 	struct mps_softc *sc;
-	int error;
 
 	mps_dprint(sassc->sc, MPS_TRACE, "%s\n", __func__);
 
@@ -1412,14 +1577,7 @@ mpssas_resetdev(struct mpssas_softc *sassc, struct mps_command *cm)
 	cm->cm_data = NULL;
 	cm->cm_desc.Default.RequestFlags = MPI2_REQ_DESCRIPT_FLAGS_DEFAULT_TYPE;
 
-	xpt_freeze_simq(sassc->sim, 1);
-
-	error = mps_map_command(sassc->sc, cm);
-
-	if (error != 0)
-		xpt_release_simq(sassc->sim, 1);
-
-	return (error);
+	mpssas_issue_tm_request(sc, cm);
 }
 
 static void
@@ -1441,9 +1599,7 @@ mpssas_resetdev_complete(struct mps_softc *sc, struct mps_command *cm)
 	else
 		ccb->ccb_h.status = CAM_REQ_CMP_ERR;
 
-	mps_free_command(sc, cm);
-
-	xpt_release_simq(sc->sassc->sim, 1);
+	mpssas_complete_tm_request(sc, cm, /*free_cm*/ 1);
 
 	xpt_done(ccb);
 }
diff --git a/sys/dev/mps/mpsvar.h b/sys/dev/mps/mpsvar.h
index 47489b9..cf27e5d 100644
--- a/sys/dev/mps/mpsvar.h
+++ b/sys/dev/mps/mpsvar.h
@@ -81,6 +81,7 @@ struct mps_command {
 #define MPS_CM_FLAGS_DATAOUT		(1 << 3)
 #define MPS_CM_FLAGS_DATAIN		(1 << 4)
 #define MPS_CM_FLAGS_WAKEUP		(1 << 5)
+#define MPS_CM_FLAGS_ACTIVE		(1 << 6)
 	u_int				cm_state;
 #define MPS_CM_STATE_FREE		0
 #define MPS_CM_STATE_BUSY		1
@@ -109,6 +110,8 @@ struct mps_softc {
 #define MPS_FLAGS_BUSY		(1 << 2)
 #define MPS_FLAGS_SHUTDOWN	(1 << 3)
 	u_int				mps_debug;
+	u_int				allow_multiple_tm_cmds;
+	int				tm_cmds_active;
 	struct sysctl_ctx_list		sysctl_ctx;
 	struct sysctl_oid		*sysctl_tree;
 	struct mps_command		*commands;
@@ -119,6 +122,7 @@ struct mps_softc {
 
 	TAILQ_HEAD(, mps_command)	req_list;
 	TAILQ_HEAD(, mps_chain)		chain_list;
+	TAILQ_HEAD(, mps_command)	tm_list;
 	int				replypostindex;
 	int				replyfreeindex;
 	int				replycurindex;