Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 7 Oct 2010 21:56:10 +0000 (UTC)
From:      "Kenneth D. Merry" <ken@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r213535 - in head: share/man/man4 sys/dev/mps
Message-ID:  <201010072156.o97LuAjk068045@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ken
Date: Thu Oct  7 21:56:10 2010
New Revision: 213535
URL: http://svn.freebsd.org/changeset/base/213535

Log:
  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

Modified:
  head/share/man/man4/mps.4
  head/sys/dev/mps/mps.c
  head/sys/dev/mps/mps_sas.c
  head/sys/dev/mps/mpsvar.h

Modified: head/share/man/man4/mps.4
==============================================================================
--- head/share/man/man4/mps.4	Thu Oct  7 20:31:07 2010	(r213534)
+++ head/share/man/man4/mps.4	Thu Oct  7 21:56:10 2010	(r213535)
@@ -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 tuna
 .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

Modified: head/sys/dev/mps/mps.c
==============================================================================
--- head/sys/dev/mps/mps.c	Thu Oct  7 20:31:07 2010	(r213534)
+++ head/sys/dev/mps/mps.c	Thu Oct  7 21:56:10 2010	(r213535)
@@ -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
 	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)
 {

Modified: head/sys/dev/mps/mps_sas.c
==============================================================================
--- head/sys/dev/mps/mps_sas.c	Thu Oct  7 20:31:07 2010	(r213534)
+++ head/sys/dev/mps/mps_sas.c	Thu Oct  7 21:56:10 2010	(r213535)
@@ -135,9 +135,16 @@ static void mpssas_probe_device_complete
 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_soft
 	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 *s
 
 	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 *
 	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, st
 {
 	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, st
 	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;
 
-	error = mps_map_command(sc, cm);
+	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);
 
-	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;
+		error = mpssas_map_tm_request(sc, cm);
+
+		/*
+		 * 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_sof
 	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_sof
 
 	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_sof
 	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 *sas
 	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_soft
 	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);
 }

Modified: head/sys/dev/mps/mpsvar.h
==============================================================================
--- head/sys/dev/mps/mpsvar.h	Thu Oct  7 20:31:07 2010	(r213534)
+++ head/sys/dev/mps/mpsvar.h	Thu Oct  7 21:56:10 2010	(r213535)
@@ -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;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201010072156.o97LuAjk068045>