svn commit: r358308 - in head/sys/dev: mpr mps
Warner Losh
imp at FreeBSD.org
Tue Feb 25 04:27:24 UTC 2020
Author: imp
Date: Tue Feb 25 04:27:23 2020
New Revision: 358308
URL: https://svnweb.freebsd.org/changeset/base/358308
Log:
Before issing the REMOVE_DEVICE command to the firmware, make sure that all
commands have completed.
It's not OK to force complete any pending commands before we send the
REMOVE_DEVICE. Instead, make sure that all pending commands are complete before
sending that. By trying to second guess the firmware here, we run the risk of
completing commands twice, which leads to corruption.
This removes the forced completion of commands introduced in r218811. So it's a
partial backout of that commit, but replaces it with a more rebust
mechanism. Either these commands will complete due to the TARGET RESET, or they
will timeout and be aborted, but they will all complete.
Add assert that all commands are complete to REMOVE_DEVICE completion
routine. We attempt to assure this programatically, so we shouldn't have any
commands in the queue because we've waited for them all. Any commands that make
it into our action routine after we mark the target in removal will complete
immediately with an error.
When we're removing a target that's not a volume, advertise up the stack that
it's actually gone, as opposed to having a transient selection error we should
retry. Do this both in the action routine, and when we get a notification of an
aborted command. We don't do this for volumes because the driver tries hard not
to advertise to the OS a volume has disappeared.
Apply these changes to both mpr and mps since they are based on quite similar
designs.
Discussed with: scottl@
Differential Revision: https://reviews.freebsd.org/D23768
Modified:
head/sys/dev/mpr/mpr_sas.c
head/sys/dev/mpr/mpr_sas.h
head/sys/dev/mps/mps_sas.c
head/sys/dev/mps/mps_sas.h
Modified: head/sys/dev/mpr/mpr_sas.c
==============================================================================
--- head/sys/dev/mpr/mpr_sas.c Tue Feb 25 03:35:52 2020 (r358307)
+++ head/sys/dev/mpr/mpr_sas.c Tue Feb 25 04:27:23 2020 (r358308)
@@ -559,7 +559,6 @@ mprsas_remove_device(struct mpr_softc *sc, struct mpr_
MPI2_SCSI_TASK_MANAGE_REPLY *reply;
MPI2_SAS_IOUNIT_CONTROL_REQUEST *req;
struct mprsas_target *targ;
- struct mpr_command *next_cm;
uint16_t handle;
MPR_FUNCTRACE(sc);
@@ -609,7 +608,18 @@ mprsas_remove_device(struct mpr_softc *sc, struct mpr_
tm->cm_complete = mprsas_remove_complete;
tm->cm_complete_data = (void *)(uintptr_t)handle;
- mpr_map_command(sc, tm);
+ /*
+ * Wait to send the REMOVE_DEVICE until all the commands have cleared.
+ * They should be aborted or time out and we'll kick thus off there
+ * if so.
+ */
+ if (TAILQ_FIRST(&targ->commands) == NULL) {
+ mpr_dprint(sc, MPR_INFO, "No pending commands: starting remove_device\n");
+ mpr_map_command(sc, tm);
+ targ->pending_remove_tm = NULL;
+ } else {
+ targ->pending_remove_tm = tm;
+ }
mpr_dprint(sc, MPR_INFO, "clearing target %u handle 0x%04x\n",
targ->tid, handle);
@@ -618,15 +628,6 @@ mprsas_remove_device(struct mpr_softc *sc, struct mpr_
"connector name (%4s)\n", targ->encl_level, targ->encl_slot,
targ->connector_name);
}
- TAILQ_FOREACH_SAFE(tm, &targ->commands, cm_link, next_cm) {
- union ccb *ccb;
-
- mpr_dprint(sc, MPR_XINFO, "Completing missed command %p\n", tm);
- ccb = tm->cm_complete_data;
- mprsas_set_ccbstatus(ccb, CAM_DEV_NOT_THERE);
- tm->cm_state = MPR_CM_STATE_BUSY;
- mprsas_scsiio_complete(sc, tm);
- }
}
static void
@@ -642,7 +643,16 @@ mprsas_remove_complete(struct mpr_softc *sc, struct mp
reply = (MPI2_SAS_IOUNIT_CONTROL_REPLY *)tm->cm_reply;
handle = (uint16_t)(uintptr_t)tm->cm_complete_data;
+ targ = tm->cm_targ;
+
/*
+ * At this point, we should have no pending commands for the target.
+ * The remove target has just completed.
+ */
+ KASSERT(TAILQ_FIRST(&targ->commands) == NULL,
+ ("%s: no commands should be pending\n", __func__));
+
+ /*
* Currently there should be no way we can hit this case. It only
* happens when we have a failure to allocate chain frames, and
* task management commands don't have S/G lists.
@@ -674,7 +684,6 @@ mprsas_remove_complete(struct mpr_softc *sc, struct mp
*/
if ((le16toh(reply->IOCStatus) & MPI2_IOCSTATUS_MASK) ==
MPI2_IOCSTATUS_SUCCESS) {
- targ = tm->cm_targ;
targ->handle = 0x0;
targ->encl_handle = 0x0;
targ->encl_level_valid = 0x0;
@@ -1964,13 +1973,17 @@ mprsas_action_scsiio(struct mprsas_softc *sassc, union
/*
* If devinfo is 0 this will be a volume. In that case don't tell CAM
* that the volume has timed out. We want volumes to be enumerated
- * until they are deleted/removed, not just failed.
+ * until they are deleted/removed, not just failed. In either event,
+ * we're removing the target due to a firmware event telling us
+ * the device is now gone (as opposed to some transient event). Since
+ * we're opting to remove failed devices from the OS's view, we need
+ * to propagate that status up the stack.
*/
if (targ->flags & MPRSAS_TARGET_INREMOVAL) {
if (targ->devinfo == 0)
mprsas_set_ccbstatus(ccb, CAM_REQ_CMP);
else
- mprsas_set_ccbstatus(ccb, CAM_SEL_TIMEOUT);
+ mprsas_set_ccbstatus(ccb, CAM_DEV_NOT_THERE);
xpt_done(ccb);
return;
}
@@ -2844,15 +2857,22 @@ mprsas_scsiio_complete(struct mpr_softc *sc, struct mp
* potential risk of flagging false failures in the event
* of a topology-related error (e.g. a SAS expander problem
* causes a command addressed to a drive to fail), but
- * avoiding getting into an infinite retry loop.
+ * avoiding getting into an infinite retry loop. However,
+ * if we get them while were moving a device, we should
+ * fail the request as 'not there' because the device
+ * is effectively gone.
*/
- mprsas_set_ccbstatus(ccb, CAM_REQ_CMP_ERR);
+ if (cm->cm_targ->flags & MPRSAS_TARGET_INREMOVAL)
+ mprsas_set_ccbstatus(ccb, CAM_DEV_NOT_THERE);
+ else
+ mprsas_set_ccbstatus(ccb, CAM_REQ_CMP_ERR);
mpr_dprint(sc, MPR_INFO,
- "Controller reported %s tgt %u SMID %u loginfo %x\n",
+ "Controller reported %s tgt %u SMID %u loginfo %x%s\n",
mpr_describe_table(mpr_iocstatus_string,
le16toh(rep->IOCStatus) & MPI2_IOCSTATUS_MASK),
target_id, cm->cm_desc.Default.SMID,
- le32toh(rep->IOCLogInfo));
+ le32toh(rep->IOCLogInfo),
+ (cm->cm_targ->flags & MPRSAS_TARGET_INREMOVAL) ? " departing" : "");
mpr_dprint(sc, MPR_XINFO,
"SCSIStatus %x SCSIState %x xfercount %u\n",
rep->SCSIStatus, rep->SCSIState,
@@ -2898,6 +2918,21 @@ mprsas_scsiio_complete(struct mpr_softc *sc, struct mp
if (mprsas_get_ccbstatus(ccb) != CAM_REQ_CMP) {
ccb->ccb_h.status |= CAM_DEV_QFRZN;
xpt_freeze_devq(ccb->ccb_h.path, /*count*/ 1);
+ }
+
+ /*
+ * Check to see if we're removing the device. If so, and this is the
+ * last command on the queue, proceed with the deferred removal of the
+ * device. Note, for removing a volume, this won't trigger because
+ * pending_remove_tm will be NULL.
+ */
+ if (cm->cm_targ->flags & MPRSAS_TARGET_INREMOVAL) {
+ if (TAILQ_FIRST(&cm->cm_targ->commands) == NULL &&
+ cm->cm_targ->pending_remove_tm != NULL) {
+ mpr_dprint(sc, MPR_INFO, "Last pending command complete: starting remove_device\n");
+ mpr_map_command(sc, cm->cm_targ->pending_remove_tm);
+ cm->cm_targ->pending_remove_tm = NULL;
+ }
}
mpr_free_command(sc, cm);
Modified: head/sys/dev/mpr/mpr_sas.h
==============================================================================
--- head/sys/dev/mpr/mpr_sas.h Tue Feb 25 03:35:52 2020 (r358307)
+++ head/sys/dev/mpr/mpr_sas.h Tue Feb 25 04:27:23 2020 (r358308)
@@ -64,6 +64,7 @@ struct mprsas_target {
SLIST_HEAD(, mprsas_lun) luns;
TAILQ_HEAD(, mpr_command) commands;
struct mpr_command *tm;
+ struct mpr_command *pending_remove_tm;
TAILQ_HEAD(, mpr_command) timedout_commands;
uint16_t exp_dev_handle;
uint16_t phy_num;
Modified: head/sys/dev/mps/mps_sas.c
==============================================================================
--- head/sys/dev/mps/mps_sas.c Tue Feb 25 03:35:52 2020 (r358307)
+++ head/sys/dev/mps/mps_sas.c Tue Feb 25 04:27:23 2020 (r358308)
@@ -556,7 +556,6 @@ mpssas_remove_device(struct mps_softc *sc, struct mps_
MPI2_SCSI_TASK_MANAGE_REPLY *reply;
MPI2_SAS_IOUNIT_CONTROL_REQUEST *req;
struct mpssas_target *targ;
- struct mps_command *next_cm;
uint16_t handle;
MPS_FUNCTRACE(sc);
@@ -609,19 +608,22 @@ mpssas_remove_device(struct mps_softc *sc, struct mps_
tm->cm_complete = mpssas_remove_complete;
tm->cm_complete_data = (void *)(uintptr_t)handle;
- mps_map_command(sc, tm);
+ /*
+ * Wait to send the REMOVE_DEVICE until all the commands have cleared.
+ * They should be aborted or time out and we'll kick thus off there
+ * if so.
+ */
+ if (TAILQ_FIRST(&targ->commands) == NULL) {
+ mps_dprint(sc, MPS_INFO, "No pending commands: starting remove_device\n");
+ mps_map_command(sc, tm);
+ targ->pending_remove_tm = NULL;
+ } else {
+ targ->pending_remove_tm = tm;
+ }
+
mps_dprint(sc, MPS_XINFO, "clearing target %u handle 0x%04x\n",
targ->tid, handle);
- TAILQ_FOREACH_SAFE(tm, &targ->commands, cm_link, next_cm) {
- union ccb *ccb;
-
- mps_dprint(sc, MPS_XINFO, "Completing missed command %p\n", tm);
- ccb = tm->cm_complete_data;
- mpssas_set_ccbstatus(ccb, CAM_DEV_NOT_THERE);
- tm->cm_state = MPS_CM_STATE_BUSY;
- mpssas_scsiio_complete(sc, tm);
- }
}
static void
@@ -636,8 +638,17 @@ mpssas_remove_complete(struct mps_softc *sc, struct mp
reply = (MPI2_SAS_IOUNIT_CONTROL_REPLY *)tm->cm_reply;
handle = (uint16_t)(uintptr_t)tm->cm_complete_data;
+ targ = tm->cm_targ;
/*
+ * At this point, we should have no pending commands for the target.
+ * The remove target has just completed.
+ */
+ KASSERT(TAILQ_FIRST(&targ->commands) == NULL,
+ ("%s: no commands should be pending\n", __func__));
+
+
+ /*
* Currently there should be no way we can hit this case. It only
* happens when we have a failure to allocate chain frames, and
* task management commands don't have S/G lists.
@@ -671,7 +682,6 @@ mpssas_remove_complete(struct mps_softc *sc, struct mp
*/
if ((le16toh(reply->IOCStatus) & MPI2_IOCSTATUS_MASK) ==
MPI2_IOCSTATUS_SUCCESS) {
- targ = tm->cm_targ;
targ->handle = 0x0;
targ->encl_handle = 0x0;
targ->encl_slot = 0x0;
@@ -1713,13 +1723,17 @@ mpssas_action_scsiio(struct mpssas_softc *sassc, union
/*
* If devinfo is 0 this will be a volume. In that case don't tell CAM
* that the volume has timed out. We want volumes to be enumerated
- * until they are deleted/removed, not just failed.
+ * until they are deleted/removed, not just failed. In either event,
+ * we're removing the target due to a firmware event telling us
+ * the device is now gone (as opposed to some transient event). Since
+ * we're opting to remove failed devices from the OS's view, we need
+ * to propagate that status up the stack.
*/
if (targ->flags & MPSSAS_TARGET_INREMOVAL) {
if (targ->devinfo == 0)
mpssas_set_ccbstatus(ccb, CAM_REQ_CMP);
else
- mpssas_set_ccbstatus(ccb, CAM_SEL_TIMEOUT);
+ mpssas_set_ccbstatus(ccb, CAM_DEV_NOT_THERE);
xpt_done(ccb);
return;
}
@@ -2352,15 +2366,22 @@ mpssas_scsiio_complete(struct mps_softc *sc, struct mp
* potential risk of flagging false failures in the event
* of a topology-related error (e.g. a SAS expander problem
* causes a command addressed to a drive to fail), but
- * avoiding getting into an infinite retry loop.
+ * avoiding getting into an infinite retry loop. However,
+ * if we get them while were moving a device, we should
+ * fail the request as 'not there' because the device
+ * is effectively gone.
*/
- mpssas_set_ccbstatus(ccb, CAM_REQ_CMP_ERR);
+ if (cm->cm_targ->flags & MPSSAS_TARGET_INREMOVAL)
+ mpssas_set_ccbstatus(ccb, CAM_DEV_NOT_THERE);
+ else
+ mpssas_set_ccbstatus(ccb, CAM_REQ_CMP_ERR);
mps_dprint(sc, MPS_INFO,
- "Controller reported %s tgt %u SMID %u loginfo %x\n",
+ "Controller reported %s tgt %u SMID %u loginfo %x%s\n",
mps_describe_table(mps_iocstatus_string,
le16toh(rep->IOCStatus) & MPI2_IOCSTATUS_MASK),
target_id, cm->cm_desc.Default.SMID,
- le32toh(rep->IOCLogInfo));
+ le32toh(rep->IOCLogInfo),
+ (cm->cm_targ->flags & MPSSAS_TARGET_INREMOVAL) ? " departing" : "");
mps_dprint(sc, MPS_XINFO,
"SCSIStatus %x SCSIState %x xfercount %u\n",
rep->SCSIStatus, rep->SCSIState,
@@ -2399,6 +2420,21 @@ mpssas_scsiio_complete(struct mps_softc *sc, struct mp
if (mpssas_get_ccbstatus(ccb) != CAM_REQ_CMP) {
ccb->ccb_h.status |= CAM_DEV_QFRZN;
xpt_freeze_devq(ccb->ccb_h.path, /*count*/ 1);
+ }
+
+ /*
+ * Check to see if we're removing the device. If so, and this is the
+ * last command on the queue, proceed with the deferred removal of the
+ * device. Note, for removing a volume, this won't trigger because
+ * pending_remove_tm will be NULL.
+ */
+ if (cm->cm_targ->flags & MPSSAS_TARGET_INREMOVAL) {
+ if (TAILQ_FIRST(&cm->cm_targ->commands) == NULL &&
+ cm->cm_targ->pending_remove_tm != NULL) {
+ mps_dprint(sc, MPS_INFO, "Last pending command complete: starting remove_device\n");
+ mps_map_command(sc, cm->cm_targ->pending_remove_tm);
+ cm->cm_targ->pending_remove_tm = NULL;
+ }
}
mps_free_command(sc, cm);
Modified: head/sys/dev/mps/mps_sas.h
==============================================================================
--- head/sys/dev/mps/mps_sas.h Tue Feb 25 03:35:52 2020 (r358307)
+++ head/sys/dev/mps/mps_sas.h Tue Feb 25 04:27:23 2020 (r358308)
@@ -62,6 +62,7 @@ struct mpssas_target {
SLIST_HEAD(, mpssas_lun) luns;
TAILQ_HEAD(, mps_command) commands;
struct mps_command *tm;
+ struct mps_command *pending_remove_tm;
TAILQ_HEAD(, mps_command) timedout_commands;
uint16_t exp_dev_handle;
uint16_t phy_num;
More information about the svn-src-head
mailing list