Commit 5f572526 authored by Nicholas Bellinger's avatar Nicholas Bellinger

qla2xxx: Fix incorrect tcm_qla2xxx_free_cmd use during TMR ABORT

This patch drops two incorrect usages of tcm_qla2xxx_free_cmd()
during TMR ABORT within tcm_qla2xxx_handle_data_work() and
tcm_qla2xxx_aborted_task(), which where attempting to dispatch
into workqueue context to do tcm_qla2xxx_complete_free() and
subsequently invoke transport_generic_free_cmd().

This is incorrect because during TMR ABORT target-core will
drop the outstanding se_cmd->cmd_kref references once it has
quiesced the se_cmd via transport_wait_for_tasks(), and in
the case of qla2xxx it should not attempt to do it's own
transport_generic_free_cmd() once the abort has occured.

As reported by Pascal, this was originally manifesting as a
BUG_ON(cmd->cmd_in_wq) in qlt_free_cmd() during TMR ABORT,
with a LIO backend that had sufficently high enough WRITE
latency to trigger a host side TMR ABORT_TASK.

In addition, for the case in tcm_qla2xxx_write_pending_status()
and tcm_qla2xxx_handle_data_work() that waits for outstanding
FCP WRITE data transfer to complete before preceeding with a
TMR ABORT, avoid se_cmd->t_transport_stop_comp that is already
used by transport_wait_for_tasks() and use a qla2xxx internal
struct completion instead.
Reported-by: default avatarPascal de Bruijn <p.debruijn@unilogic.nl>
Tested-by: default avatarPascal de Bruijn <p.debruijn@unilogic.nl>
Acked-by: default avatarHimanshu Madhani <himanshu.madhani@cavium.com>
Cc: Quinn Tran <quinn.tran@cavium.com>
Cc: <stable@vger.kernel.org> # 3.10+
Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
parent fce50a2f
...@@ -4079,6 +4079,8 @@ static struct qla_tgt_cmd *qlt_get_tag(scsi_qla_host_t *vha, ...@@ -4079,6 +4079,8 @@ static struct qla_tgt_cmd *qlt_get_tag(scsi_qla_host_t *vha,
cmd = &((struct qla_tgt_cmd *)se_sess->sess_cmd_map)[tag]; cmd = &((struct qla_tgt_cmd *)se_sess->sess_cmd_map)[tag];
memset(cmd, 0, sizeof(struct qla_tgt_cmd)); memset(cmd, 0, sizeof(struct qla_tgt_cmd));
init_completion(&cmd->write_pending_abort_comp);
memcpy(&cmd->atio, atio, sizeof(*atio)); memcpy(&cmd->atio, atio, sizeof(*atio));
cmd->state = QLA_TGT_STATE_NEW; cmd->state = QLA_TGT_STATE_NEW;
cmd->tgt = vha->vha_tgt.qla_tgt; cmd->tgt = vha->vha_tgt.qla_tgt;
...@@ -5083,6 +5085,8 @@ qlt_alloc_qfull_cmd(struct scsi_qla_host *vha, ...@@ -5083,6 +5085,8 @@ qlt_alloc_qfull_cmd(struct scsi_qla_host *vha,
qlt_incr_num_pend_cmds(vha); qlt_incr_num_pend_cmds(vha);
INIT_LIST_HEAD(&cmd->cmd_list); INIT_LIST_HEAD(&cmd->cmd_list);
init_completion(&cmd->write_pending_abort_comp);
memcpy(&cmd->atio, atio, sizeof(*atio)); memcpy(&cmd->atio, atio, sizeof(*atio));
cmd->tgt = vha->vha_tgt.qla_tgt; cmd->tgt = vha->vha_tgt.qla_tgt;
......
...@@ -868,6 +868,7 @@ struct qla_tgt_cmd { ...@@ -868,6 +868,7 @@ struct qla_tgt_cmd {
unsigned char sense_buffer[TRANSPORT_SENSE_BUFFER]; unsigned char sense_buffer[TRANSPORT_SENSE_BUFFER];
spinlock_t cmd_lock; spinlock_t cmd_lock;
struct completion write_pending_abort_comp;
/* to save extra sess dereferences */ /* to save extra sess dereferences */
unsigned int conf_compl_supported:1; unsigned int conf_compl_supported:1;
unsigned int sg_mapped:1; unsigned int sg_mapped:1;
......
...@@ -415,6 +415,7 @@ static int tcm_qla2xxx_write_pending(struct se_cmd *se_cmd) ...@@ -415,6 +415,7 @@ static int tcm_qla2xxx_write_pending(struct se_cmd *se_cmd)
static int tcm_qla2xxx_write_pending_status(struct se_cmd *se_cmd) static int tcm_qla2xxx_write_pending_status(struct se_cmd *se_cmd)
{ {
struct qla_tgt_cmd *cmd = container_of(se_cmd, struct qla_tgt_cmd, se_cmd);
unsigned long flags; unsigned long flags;
/* /*
* Check for WRITE_PENDING status to determine if we need to wait for * Check for WRITE_PENDING status to determine if we need to wait for
...@@ -424,8 +425,7 @@ static int tcm_qla2xxx_write_pending_status(struct se_cmd *se_cmd) ...@@ -424,8 +425,7 @@ static int tcm_qla2xxx_write_pending_status(struct se_cmd *se_cmd)
if (se_cmd->t_state == TRANSPORT_WRITE_PENDING || if (se_cmd->t_state == TRANSPORT_WRITE_PENDING ||
se_cmd->t_state == TRANSPORT_COMPLETE_QF_WP) { se_cmd->t_state == TRANSPORT_COMPLETE_QF_WP) {
spin_unlock_irqrestore(&se_cmd->t_state_lock, flags); spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
wait_for_completion_timeout(&se_cmd->t_transport_stop_comp, wait_for_completion(&cmd->write_pending_abort_comp);
50);
return 0; return 0;
} }
spin_unlock_irqrestore(&se_cmd->t_state_lock, flags); spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
...@@ -501,7 +501,6 @@ static int tcm_qla2xxx_handle_cmd(scsi_qla_host_t *vha, struct qla_tgt_cmd *cmd, ...@@ -501,7 +501,6 @@ static int tcm_qla2xxx_handle_cmd(scsi_qla_host_t *vha, struct qla_tgt_cmd *cmd,
static void tcm_qla2xxx_handle_data_work(struct work_struct *work) static void tcm_qla2xxx_handle_data_work(struct work_struct *work)
{ {
struct qla_tgt_cmd *cmd = container_of(work, struct qla_tgt_cmd, work); struct qla_tgt_cmd *cmd = container_of(work, struct qla_tgt_cmd, work);
unsigned long flags;
/* /*
* Ensure that the complete FCP WRITE payload has been received. * Ensure that the complete FCP WRITE payload has been received.
...@@ -509,17 +508,6 @@ static void tcm_qla2xxx_handle_data_work(struct work_struct *work) ...@@ -509,17 +508,6 @@ static void tcm_qla2xxx_handle_data_work(struct work_struct *work)
*/ */
cmd->cmd_in_wq = 0; cmd->cmd_in_wq = 0;
spin_lock_irqsave(&cmd->cmd_lock, flags);
cmd->data_work = 1;
if (cmd->aborted) {
cmd->data_work_free = 1;
spin_unlock_irqrestore(&cmd->cmd_lock, flags);
tcm_qla2xxx_free_cmd(cmd);
return;
}
spin_unlock_irqrestore(&cmd->cmd_lock, flags);
cmd->vha->tgt_counters.qla_core_ret_ctio++; cmd->vha->tgt_counters.qla_core_ret_ctio++;
if (!cmd->write_data_transferred) { if (!cmd->write_data_transferred) {
/* /*
...@@ -527,7 +515,7 @@ static void tcm_qla2xxx_handle_data_work(struct work_struct *work) ...@@ -527,7 +515,7 @@ static void tcm_qla2xxx_handle_data_work(struct work_struct *work)
* waiting upon completion in tcm_qla2xxx_write_pending_status() * waiting upon completion in tcm_qla2xxx_write_pending_status()
*/ */
if (cmd->se_cmd.transport_state & CMD_T_ABORTED) { if (cmd->se_cmd.transport_state & CMD_T_ABORTED) {
complete(&cmd->se_cmd.t_transport_stop_comp); complete(&cmd->write_pending_abort_comp);
return; return;
} }
...@@ -753,31 +741,13 @@ static void tcm_qla2xxx_queue_tm_rsp(struct se_cmd *se_cmd) ...@@ -753,31 +741,13 @@ static void tcm_qla2xxx_queue_tm_rsp(struct se_cmd *se_cmd)
qlt_xmit_tm_rsp(mcmd); qlt_xmit_tm_rsp(mcmd);
} }
#define DATA_WORK_NOT_FREE(_cmd) (_cmd->data_work && !_cmd->data_work_free)
static void tcm_qla2xxx_aborted_task(struct se_cmd *se_cmd) static void tcm_qla2xxx_aborted_task(struct se_cmd *se_cmd)
{ {
struct qla_tgt_cmd *cmd = container_of(se_cmd, struct qla_tgt_cmd *cmd = container_of(se_cmd,
struct qla_tgt_cmd, se_cmd); struct qla_tgt_cmd, se_cmd);
unsigned long flags;
if (qlt_abort_cmd(cmd)) if (qlt_abort_cmd(cmd))
return; return;
spin_lock_irqsave(&cmd->cmd_lock, flags);
if ((cmd->state == QLA_TGT_STATE_NEW)||
((cmd->state == QLA_TGT_STATE_DATA_IN) &&
DATA_WORK_NOT_FREE(cmd))) {
cmd->data_work_free = 1;
spin_unlock_irqrestore(&cmd->cmd_lock, flags);
/*
* cmd has not reached fw, Use this trigger to free it.
*/
tcm_qla2xxx_free_cmd(cmd);
return;
}
spin_unlock_irqrestore(&cmd->cmd_lock, flags);
return;
} }
static void tcm_qla2xxx_clear_sess_lookup(struct tcm_qla2xxx_lport *, static void tcm_qla2xxx_clear_sess_lookup(struct tcm_qla2xxx_lport *,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment