svn commit: r342355 - in head/sys/dev: mpr mps
Conrad Meyer
cem at FreeBSD.org
Fri Dec 21 20:30:55 UTC 2018
Author: cem
Date: Fri Dec 21 20:30:52 2018
New Revision: 342355
URL: https://svnweb.freebsd.org/changeset/base/342355
Log:
mps(4), mpr(4): remove SATA ID command cancellation hack
Add a generic mechanism to override mp?_wait_command's timeout behavior,
which continues to invoke reinit by default. Invokers who set
cm_timeout_handler may avoid automatic reinit and do their own handling.
Adapt mp?sas_get_sata_identify to this mechanism and remove its callout
hack.
Reviewed by: scottl
Sponsored by: Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D18614
Modified:
head/sys/dev/mpr/mpr.c
head/sys/dev/mpr/mpr_sas_lsi.c
head/sys/dev/mpr/mprvar.h
head/sys/dev/mps/mps.c
head/sys/dev/mps/mps_sas_lsi.c
head/sys/dev/mps/mpsvar.h
Modified: head/sys/dev/mpr/mpr.c
==============================================================================
--- head/sys/dev/mpr/mpr.c Fri Dec 21 20:29:16 2018 (r342354)
+++ head/sys/dev/mpr/mpr.c Fri Dec 21 20:30:52 2018 (r342355)
@@ -3815,12 +3815,15 @@ mpr_wait_command(struct mpr_softc *sc, struct mpr_comm
}
if (error == EWOULDBLOCK) {
- mpr_dprint(sc, MPR_FAULT, "Calling Reinit from %s, timeout=%d,"
- " elapsed=%jd\n", __func__, timeout,
- (intmax_t)cur_time.tv_sec);
- rc = mpr_reinit(sc);
- mpr_dprint(sc, MPR_FAULT, "Reinit %s\n", (rc == 0) ? "success" :
- "failed");
+ if (cm->cm_timeout_handler == NULL) {
+ mpr_dprint(sc, MPR_FAULT, "Calling Reinit from %s, timeout=%d,"
+ " elapsed=%jd\n", __func__, timeout,
+ (intmax_t)cur_time.tv_sec);
+ rc = mpr_reinit(sc);
+ mpr_dprint(sc, MPR_FAULT, "Reinit %s\n", (rc == 0) ? "success" :
+ "failed");
+ } else
+ cm->cm_timeout_handler(sc, cm);
if (sc->mpr_flags & MPR_FLAGS_REALLOCATED) {
/*
* Tell the caller that we freed the command in a
Modified: head/sys/dev/mpr/mpr_sas_lsi.c
==============================================================================
--- head/sys/dev/mpr/mpr_sas_lsi.c Fri Dec 21 20:29:16 2018 (r342354)
+++ head/sys/dev/mpr/mpr_sas_lsi.c Fri Dec 21 20:30:52 2018 (r342355)
@@ -124,7 +124,7 @@ static int mprsas_add_pcie_device(struct mpr_softc *sc
static int mprsas_get_sata_identify(struct mpr_softc *sc, u16 handle,
Mpi2SataPassthroughReply_t *mpi_reply, char *id_buffer, int sz,
u32 devinfo);
-static void mprsas_ata_id_timeout(void *data);
+static void mprsas_ata_id_timeout(struct mpr_softc *, struct mpr_command *);
int mprsas_get_sas_address_for_sata_disk(struct mpr_softc *sc,
u64 *sas_address, u16 handle, u32 device_info, u8 *is_SATA_SSD);
static int mprsas_volume_add(struct mpr_softc *sc,
@@ -1167,23 +1167,19 @@ mprsas_get_sata_identify(struct mpr_softc *sc, u16 han
cm->cm_length = htole32(sz);
/*
- * Start a timeout counter specifically for the SATA ID command. This
- * is used to fix a problem where the FW does not send a reply sometimes
+ * Use a custom handler to avoid reinit'ing the controller on timeout.
+ * This fixes a problem where the FW does not send a reply sometimes
* when a bad disk is in the topology. So, this is used to timeout the
* command so that processing can continue normally.
*/
- mpr_dprint(sc, MPR_XINFO, "%s start timeout counter for SATA ID "
- "command\n", __func__);
- callout_reset(&cm->cm_callout, MPR_ATA_ID_TIMEOUT * hz,
- mprsas_ata_id_timeout, cm);
- error = mpr_wait_command(sc, &cm, 60, CAN_SLEEP);
- mpr_dprint(sc, MPR_XINFO, "%s stop timeout counter for SATA ID "
- "command\n", __func__);
- /* XXX KDM need to fix the case where this command is destroyed */
- callout_stop(&cm->cm_callout);
+ cm->cm_timeout_handler = mprsas_ata_id_timeout;
- if (cm != NULL)
- reply = (Mpi2SataPassthroughReply_t *)cm->cm_reply;
+ error = mpr_wait_command(sc, &cm, MPR_ATA_ID_TIMEOUT, CAN_SLEEP);
+
+ /* mprsas_ata_id_timeout does not reset controller */
+ KASSERT(cm != NULL, ("%s: surprise command freed", __func__));
+
+ reply = (Mpi2SataPassthroughReply_t *)cm->cm_reply;
if (error || (reply == NULL)) {
/* FIXME */
/*
@@ -1210,61 +1206,28 @@ out:
/*
* If the SATA_ID_TIMEOUT flag has been set for this command, don't free
* it. The command and buffer will be freed after sending an Abort
- * Task TM. If the command did timeout, use EWOULDBLOCK.
+ * Task TM.
*/
if ((cm->cm_flags & MPR_CM_FLAGS_SATA_ID_TIMEOUT) == 0) {
mpr_free_command(sc, cm);
free(buffer, M_MPR);
- } else if (error == 0)
- error = EWOULDBLOCK;
+ }
return (error);
}
static void
-mprsas_ata_id_timeout(void *data)
+mprsas_ata_id_timeout(struct mpr_softc *sc, struct mpr_command *cm)
{
- struct mpr_softc *sc;
- struct mpr_command *cm;
- cm = (struct mpr_command *)data;
- sc = cm->cm_sc;
- mtx_assert(&sc->mpr_mtx, MA_OWNED);
-
- mpr_dprint(sc, MPR_INFO, "%s checking ATA ID command %p sc %p\n",
+ mpr_dprint(sc, MPR_INFO, "%s ATA ID command timeout cm %p sc %p\n",
__func__, cm, sc);
- if ((callout_pending(&cm->cm_callout)) ||
- (!callout_active(&cm->cm_callout))) {
- mpr_dprint(sc, MPR_INFO, "%s ATA ID command almost timed out\n",
- __func__);
- return;
- }
- callout_deactivate(&cm->cm_callout);
/*
- * Run the interrupt handler to make sure it's not pending. This
- * isn't perfect because the command could have already completed
- * and been re-used, though this is unlikely.
+ * The Abort Task cannot be sent from here because the driver has not
+ * completed setting up targets. Instead, the command is flagged so
+ * that special handling will be used to send the abort.
*/
- mpr_intr_locked(sc);
- if (cm->cm_state == MPR_CM_STATE_FREE) {
- mpr_dprint(sc, MPR_INFO, "%s ATA ID command almost timed out\n",
- __func__);
- return;
- }
-
- mpr_dprint(sc, MPR_INFO, "ATA ID command timeout cm %p\n", cm);
-
- /*
- * Send wakeup() to the sleeping thread that issued this ATA ID command.
- * wakeup() will cause msleep to return a 0 (not EWOULDBLOCK), and this
- * will keep reinit() from being called. This way, an Abort Task TM can
- * be issued so that the timed out command can be cleared. The Abort
- * Task cannot be sent from here because the driver has not completed
- * setting up targets. Instead, the command is flagged so that special
- * handling will be used to send the abort.
- */
cm->cm_flags |= MPR_CM_FLAGS_SATA_ID_TIMEOUT;
- wakeup(cm);
}
static int
Modified: head/sys/dev/mpr/mprvar.h
==============================================================================
--- head/sys/dev/mpr/mprvar.h Fri Dec 21 20:29:16 2018 (r342354)
+++ head/sys/dev/mpr/mprvar.h Fri Dec 21 20:30:52 2018 (r342355)
@@ -252,6 +252,7 @@ struct mpr_command {
uint32_t cm_req_busaddr;
bus_addr_t cm_sense_busaddr;
struct callout cm_callout;
+ mpr_command_callback_t *cm_timeout_handler;
};
struct mpr_column_map {
@@ -613,6 +614,7 @@ mpr_alloc_command(struct mpr_softc *sc)
TAILQ_REMOVE(&sc->req_list, cm, cm_link);
cm->cm_state = MPR_CM_STATE_BUSY;
+ cm->cm_timeout_handler = NULL;
return (cm);
}
@@ -654,6 +656,7 @@ mpr_alloc_high_priority_command(struct mpr_softc *sc)
TAILQ_REMOVE(&sc->high_priority_req_list, cm, cm_link);
cm->cm_state = MPR_CM_STATE_BUSY;
+ cm->cm_timeout_handler = NULL;
return (cm);
}
Modified: head/sys/dev/mps/mps.c
==============================================================================
--- head/sys/dev/mps/mps.c Fri Dec 21 20:29:16 2018 (r342354)
+++ head/sys/dev/mps/mps.c Fri Dec 21 20:30:52 2018 (r342355)
@@ -3103,12 +3103,15 @@ mps_wait_command(struct mps_softc *sc, struct mps_comm
}
if (error == EWOULDBLOCK) {
- mps_dprint(sc, MPS_FAULT, "Calling Reinit from %s, timeout=%d,"
- " elapsed=%jd\n", __func__, timeout,
- (intmax_t)cur_time.tv_sec);
- rc = mps_reinit(sc);
- mps_dprint(sc, MPS_FAULT, "Reinit %s\n", (rc == 0) ? "success" :
- "failed");
+ if (cm->cm_timeout_handler == NULL) {
+ mps_dprint(sc, MPS_FAULT, "Calling Reinit from %s, timeout=%d,"
+ " elapsed=%jd\n", __func__, timeout,
+ (intmax_t)cur_time.tv_sec);
+ rc = mps_reinit(sc);
+ mps_dprint(sc, MPS_FAULT, "Reinit %s\n", (rc == 0) ? "success" :
+ "failed");
+ } else
+ cm->cm_timeout_handler(sc, cm);
if (sc->mps_flags & MPS_FLAGS_REALLOCATED) {
/*
* Tell the caller that we freed the command in a
Modified: head/sys/dev/mps/mps_sas_lsi.c
==============================================================================
--- head/sys/dev/mps/mps_sas_lsi.c Fri Dec 21 20:29:16 2018 (r342354)
+++ head/sys/dev/mps/mps_sas_lsi.c Fri Dec 21 20:30:52 2018 (r342355)
@@ -123,7 +123,7 @@ static int mpssas_add_device(struct mps_softc *sc, u16
static int mpssas_get_sata_identify(struct mps_softc *sc, u16 handle,
Mpi2SataPassthroughReply_t *mpi_reply, char *id_buffer, int sz,
u32 devinfo);
-static void mpssas_ata_id_timeout(void *data);
+static void mpssas_ata_id_timeout(struct mps_softc *, struct mps_command *);
int mpssas_get_sas_address_for_sata_disk(struct mps_softc *sc,
u64 *sas_address, u16 handle, u32 device_info, u8 *is_SATA_SSD);
static int mpssas_volume_add(struct mps_softc *sc,
@@ -959,23 +959,19 @@ mpssas_get_sata_identify(struct mps_softc *sc, u16 han
cm->cm_length = htole32(sz);
/*
- * Start a timeout counter specifically for the SATA ID command. This
- * is used to fix a problem where the FW does not send a reply sometimes
+ * Use a custom handler to avoid reinit'ing the controller on timeout.
+ * This fixes a problem where the FW does not send a reply sometimes
* when a bad disk is in the topology. So, this is used to timeout the
* command so that processing can continue normally.
*/
- mps_dprint(sc, MPS_XINFO, "%s start timeout counter for SATA ID "
- "command\n", __func__);
- callout_reset(&cm->cm_callout, MPS_ATA_ID_TIMEOUT * hz,
- mpssas_ata_id_timeout, cm);
- error = mps_wait_command(sc, &cm, 60, CAN_SLEEP);
- mps_dprint(sc, MPS_XINFO, "%s stop timeout counter for SATA ID "
- "command\n", __func__);
- /* XXX KDM need to fix the case where this command is destroyed */
- callout_stop(&cm->cm_callout);
+ cm->cm_timeout_handler = mpssas_ata_id_timeout;
- if (cm != NULL)
- reply = (Mpi2SataPassthroughReply_t *)cm->cm_reply;
+ error = mps_wait_command(sc, &cm, MPS_ATA_ID_TIMEOUT, CAN_SLEEP);
+
+ /* mpssas_ata_id_timeout does not reset controller */
+ KASSERT(cm != NULL, ("%s: surprise command freed", __func__));
+
+ reply = (Mpi2SataPassthroughReply_t *)cm->cm_reply;
if (error || (reply == NULL)) {
/* FIXME */
/*
@@ -1002,62 +998,28 @@ out:
/*
* If the SATA_ID_TIMEOUT flag has been set for this command, don't free
* it. The command and buffer will be freed after sending an Abort
- * Task TM. If the command did timeout, use EWOULDBLOCK.
+ * Task TM.
*/
- if ((cm != NULL)
- && (cm->cm_flags & MPS_CM_FLAGS_SATA_ID_TIMEOUT) == 0) {
+ if ((cm->cm_flags & MPS_CM_FLAGS_SATA_ID_TIMEOUT) == 0) {
mps_free_command(sc, cm);
free(buffer, M_MPT2);
- } else if (error == 0)
- error = EWOULDBLOCK;
+ }
return (error);
}
static void
-mpssas_ata_id_timeout(void *data)
+mpssas_ata_id_timeout(struct mps_softc *sc, struct mps_command *cm)
{
- struct mps_softc *sc;
- struct mps_command *cm;
- cm = (struct mps_command *)data;
- sc = cm->cm_sc;
- mtx_assert(&sc->mps_mtx, MA_OWNED);
-
- mps_dprint(sc, MPS_INFO, "%s checking ATA ID command %p sc %p\n",
+ mps_dprint(sc, MPS_INFO, "%s ATA ID command timeout cm %p sc %p\n",
__func__, cm, sc);
- if ((callout_pending(&cm->cm_callout)) ||
- (!callout_active(&cm->cm_callout))) {
- mps_dprint(sc, MPS_INFO, "%s ATA ID command almost timed out\n",
- __func__);
- return;
- }
- callout_deactivate(&cm->cm_callout);
/*
- * Run the interrupt handler to make sure it's not pending. This
- * isn't perfect because the command could have already completed
- * and been re-used, though this is unlikely.
+ * The Abort Task cannot be sent from here because the driver has not
+ * completed setting up targets. Instead, the command is flagged so
+ * that special handling will be used to send the abort.
*/
- mps_intr_locked(sc);
- if (cm->cm_state == MPS_CM_STATE_FREE) {
- mps_dprint(sc, MPS_INFO, "%s ATA ID command almost timed out\n",
- __func__);
- return;
- }
-
- mps_dprint(sc, MPS_INFO, "ATA ID command timeout cm %p\n", cm);
-
- /*
- * Send wakeup() to the sleeping thread that issued this ATA ID command.
- * wakeup() will cause msleep to return a 0 (not EWOULDBLOCK), and this
- * will keep reinit() from being called. This way, an Abort Task TM can
- * be issued so that the timed out command can be cleared. The Abort
- * Task cannot be sent from here because the driver has not completed
- * setting up targets. Instead, the command is flagged so that special
- * handling will be used to send the abort.
- */
cm->cm_flags |= MPS_CM_FLAGS_SATA_ID_TIMEOUT;
- wakeup(cm);
}
static int
Modified: head/sys/dev/mps/mpsvar.h
==============================================================================
--- head/sys/dev/mps/mpsvar.h Fri Dec 21 20:29:16 2018 (r342354)
+++ head/sys/dev/mps/mpsvar.h Fri Dec 21 20:30:52 2018 (r342355)
@@ -250,6 +250,7 @@ struct mps_command {
uint32_t cm_req_busaddr;
uint32_t cm_sense_busaddr;
struct callout cm_callout;
+ mps_command_callback_t *cm_timeout_handler;
};
struct mps_column_map {
@@ -581,6 +582,7 @@ mps_alloc_command(struct mps_softc *sc)
TAILQ_REMOVE(&sc->req_list, cm, cm_link);
cm->cm_state = MPS_CM_STATE_BUSY;
+ cm->cm_timeout_handler = NULL;
return (cm);
}
@@ -622,6 +624,7 @@ mps_alloc_high_priority_command(struct mps_softc *sc)
TAILQ_REMOVE(&sc->high_priority_req_list, cm, cm_link);
cm->cm_state = MPS_CM_STATE_BUSY;
+ cm->cm_timeout_handler = NULL;
return (cm);
}
More information about the svn-src-all
mailing list