From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
This patch converts tcm_qla2xxx_release_cmd() + tcm_qla2xxx_check_stop_free() to
obtain qla_hw_data->hardware_lock when checking for qla_tgt_cmd related shutdown
bits. This forces sychronization with qla_target.c:qla_tgt_wait_for_cmds()
code for the main release path in tcm_qla2xxx_release_cmd(), and also with
tcm_qla2xxx_check_stop_free() to determine if tcm_qla2xxx_release_cmd()
is waiting for the TFO->check_stop_free() to complete on ->cmd_stop_free_comp
It also removes the qla_tgt->tgt_stop and qla_tgt_sess->tearing_down checks
in tcm_qla2xxx_free_cmd(), and will always queue up outstanding descriptors
through the main tcm_qla2xxx_release_cmd() path in process context. This
prevents an race where tcm_qla2xxx_free_cmd() was completing ->cmd_free_comp
to qla_tgt_wait_for_cmds() before TFO->check_stop_free() finished processing.
Reported-by: Roland Dreier <roland@xxxxxxxxxxxxxxx>
Cc: Roland Dreier <roland@xxxxxxxxxxxxxxx>
Cc: Madhuranath Iyengar <mni@xxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxxxxxxxx>
---
drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c | 68 +++++++++++++----------
1 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c b/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
index 37f0b03..2660a57 100644
--- a/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
+++ b/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
@@ -385,7 +385,7 @@ u32 tcm_qla2xxx_tpg_get_inst_index(struct se_portal_group *se_tpg)
/*
* Called from qla_target_template->free_cmd(), and will call
* tcm_qla2xxx_release_cmd via normal struct target_core_fabric_ops
- * release callback.
+ * release callback. qla_hw_data->hardware_lock is expected to be held
*/
void tcm_qla2xxx_free_cmd(struct qla_tgt_cmd *cmd)
{
@@ -403,20 +403,6 @@ void tcm_qla2xxx_free_cmd(struct qla_tgt_cmd *cmd)
return;
}
- if (cmd->tgt->tgt_stop) {
- pr_warn("tcm_qla2xxx_free_cmd: Detected tgt_stop"
- " for cmd: %p !!!!!!\n", cmd);
- complete(&cmd->cmd_free_comp);
- return;
- }
-
- if (cmd->sess->tearing_down) {
- pr_warn("tcm_qla2xxx_free_cmd: Detected tearing_down"
- " for cmd: %p !!!!!!\n", cmd);
- complete(&cmd->cmd_free_comp);
- return;
- }
-
if (!atomic_read(&cmd->se_cmd.t_transport_complete)) {
atomic_set(&cmd->cmd_stop_free, 1);
smp_mb__after_atomic_dec();
@@ -435,6 +421,8 @@ void tcm_qla2xxx_check_stop_free(struct se_cmd *se_cmd)
{
struct qla_tgt_cmd *cmd = container_of(se_cmd, struct qla_tgt_cmd, se_cmd);
struct qla_tgt_mgmt_cmd *mcmd;
+ struct qla_hw_data *ha;
+ unsigned long flags;
if (se_cmd->se_tmr_req) {
mcmd = container_of(se_cmd, struct qla_tgt_mgmt_cmd, se_cmd);
@@ -446,9 +434,16 @@ void tcm_qla2xxx_check_stop_free(struct se_cmd *se_cmd)
qla_tgt_free_mcmd(mcmd);
return;
}
-
+ ha = cmd->sess->vha->hw;
+ /*
+ * Set cmd_stop_free and wakeup cmd_stop_free_comp if necessary
+ * if tcm_qla2xxx_release_cmd() context is waiting for completion.
+ */
+ spin_lock_irqsave(&ha->hardware_lock, flags);
atomic_set(&cmd->cmd_stop_free, 1);
- barrier();
+ if (atomic_read(&cmd->cmd_free) != 0)
+ complete(&cmd->cmd_stop_free_comp);
+ spin_unlock_irqrestore(&ha->hardware_lock, flags);
}
/*
@@ -458,7 +453,8 @@ void tcm_qla2xxx_release_cmd(struct se_cmd *se_cmd)
{
struct qla_tgt_cmd *cmd;
struct qla_tgt_sess *sess;
- unsigned long flags;
+ struct qla_hw_data *ha;
+ unsigned long flags, flags2;
if (se_cmd->se_tmr_req != NULL)
return;
@@ -469,27 +465,41 @@ void tcm_qla2xxx_release_cmd(struct se_cmd *se_cmd)
if (!sess)
BUG();
- while (atomic_read(&cmd->cmd_stop_free) != 1) {
- pr_warn("Hit atomic_read(&cmd->cmd_stop_free)=1"
- " in tcm_qla2xxx_release_cmd\n");
- cpu_relax();
+ ha = sess->vha->hw;
+ /*
+ * If the callback to tcm_qla2xxx_check_stop_free() has not finished,
+ * before the release path is invoked, go ahead and wait on
+ * cmd_stop_free_comp until tcm_qla2xxx_check_stop_free completes.
+ */
+ spin_lock_irqsave(&ha->hardware_lock, flags);
+ if ((atomic_read(&cmd->cmd_stop_free) != 1) &&
+ (atomic_read(&cmd->cmd_free_comp_set) == 0)) {
+ pr_warn("Detected cmd->cmd_stop_free != 0, waiting on"
+ " cmd_stop_free_comp for cmd: %p\n", cmd);
+ spin_unlock_irqrestore(&ha->hardware_lock, flags);
+ wait_for_completion(&cmd->cmd_stop_free_comp);
+ spin_lock_irqsave(&ha->hardware_lock, flags);
}
-
-
- spin_lock_irqsave(&sess->sess_cmd_lock, flags);
+ /*
+ * During shutdown, qla_tgt_wait_for_cmds() will be waiting on
+ * this outstanding qla_tgt_cmd descriptor. For this case,
+ * perform the completion and return, and qla_tgt_wait_for_cmds()
+ * will handle the direct call to qla_tgt_free_cmd()
+ */
if (cmd->tgt->tgt_stop || sess->tearing_down) {
if (atomic_read(&cmd->cmd_free_comp_set) ||
atomic_read(&cmd->cmd_free)) {
pr_warn("Detected shutdown, calling complete("
"&cmd->cmd_free_comp): cmd: %p\n", cmd);
- spin_unlock_irqrestore(&sess->sess_cmd_lock, flags);
+ spin_unlock_irqrestore(&ha->hardware_lock, flags);
complete(&cmd->cmd_free_comp);
return;
}
}
- if (atomic_read(&cmd->cmd_free))
- list_del(&cmd->cmd_list);
- spin_unlock_irqrestore(&sess->sess_cmd_lock, flags);
+ spin_lock_irqsave(&sess->sess_cmd_lock, flags2);
+ list_del(&cmd->cmd_list);
+ spin_unlock_irqrestore(&sess->sess_cmd_lock, flags2);
+ spin_unlock_irqrestore(&ha->hardware_lock, flags);
qla_tgt_free_cmd(cmd);
}
--
1.7.2.5
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
