Skip to content

Commit a07100e

Browse files
Quinn TranNicholas Bellinger
Quinn Tran
authored and
Nicholas Bellinger
committed
qla2xxx: Fix TMR ABORT interaction issue between qla2xxx and TCM
During lun reset, TMR thread from TCM would issue abort to qla driver. At abort time, each command is in different state. Depending on the state, qla will use the TMR thread to trigger a command free(cmd_kref--) if command is not down at firmware. Signed-off-by: Quinn Tran <quinn.tran@qlogic.com> Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com> Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
1 parent dacb582 commit a07100e

File tree

3 files changed

+142
-45
lines changed

3 files changed

+142
-45
lines changed

‎drivers/scsi/qla2xxx/qla_target.c

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ static void qlt_response_pkt(struct scsi_qla_host *ha, response_t *pkt);
105105
static int qlt_issue_task_mgmt(struct qla_tgt_sess *sess, uint32_t lun,
106106
int fn, void *iocb, int flags);
107107
static void qlt_send_term_exchange(struct scsi_qla_host *ha, struct qla_tgt_cmd
108-
*cmd, struct atio_from_isp *atio, int ha_locked);
108+
*cmd, struct atio_from_isp *atio, int ha_locked, int ul_abort);
109109
static void qlt_reject_free_srr_imm(struct scsi_qla_host *ha,
110110
struct qla_tgt_srr_imm *imm, int ha_lock);
111111
static void qlt_abort_cmd_on_host_reset(struct scsi_qla_host *vha,
@@ -2665,7 +2665,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
26652665
/* no need to terminate. FW already freed exchange. */
26662666
qlt_abort_cmd_on_host_reset(cmd->vha, cmd);
26672667
else
2668-
qlt_send_term_exchange(vha, cmd, &cmd->atio, 1);
2668+
qlt_send_term_exchange(vha, cmd, &cmd->atio, 1, 0);
26692669
spin_unlock_irqrestore(&ha->hardware_lock, flags);
26702670
return 0;
26712671
}
@@ -3173,7 +3173,8 @@ static int __qlt_send_term_exchange(struct scsi_qla_host *vha,
31733173
}
31743174

31753175
static void qlt_send_term_exchange(struct scsi_qla_host *vha,
3176-
struct qla_tgt_cmd *cmd, struct atio_from_isp *atio, int ha_locked)
3176+
struct qla_tgt_cmd *cmd, struct atio_from_isp *atio, int ha_locked,
3177+
int ul_abort)
31773178
{
31783179
unsigned long flags = 0;
31793180
int rc;
@@ -3193,8 +3194,7 @@ static void qlt_send_term_exchange(struct scsi_qla_host *vha,
31933194
qlt_alloc_qfull_cmd(vha, atio, 0, 0);
31943195

31953196
done:
3196-
if (cmd && (!cmd->aborted ||
3197-
!cmd->cmd_sent_to_fw)) {
3197+
if (cmd && !ul_abort && !cmd->aborted) {
31983198
if (cmd->sg_mapped)
31993199
qlt_unmap_sg(vha, cmd);
32003200
vha->hw->tgt.tgt_ops->free_cmd(cmd);
@@ -3253,21 +3253,38 @@ static void qlt_chk_exch_leak_thresh_hold(struct scsi_qla_host *vha)
32533253

32543254
}
32553255

3256-
void qlt_abort_cmd(struct qla_tgt_cmd *cmd)
3256+
int qlt_abort_cmd(struct qla_tgt_cmd *cmd)
32573257
{
32583258
struct qla_tgt *tgt = cmd->tgt;
32593259
struct scsi_qla_host *vha = tgt->vha;
32603260
struct se_cmd *se_cmd = &cmd->se_cmd;
3261+
unsigned long flags;
32613262

32623263
ql_dbg(ql_dbg_tgt_mgt, vha, 0xf014,
32633264
"qla_target(%d): terminating exchange for aborted cmd=%p "
32643265
"(se_cmd=%p, tag=%llu)", vha->vp_idx, cmd, &cmd->se_cmd,
32653266
se_cmd->tag);
32663267

3268+
spin_lock_irqsave(&cmd->cmd_lock, flags);
3269+
if (cmd->aborted) {
3270+
spin_unlock_irqrestore(&cmd->cmd_lock, flags);
3271+
/*
3272+
* It's normal to see 2 calls in this path:
3273+
* 1) XFER Rdy completion + CMD_T_ABORT
3274+
* 2) TCM TMR - drain_state_list
3275+
*/
3276+
ql_dbg(ql_dbg_tgt_mgt, vha, 0xffff,
3277+
"multiple abort. %p transport_state %x, t_state %x,"
3278+
" se_cmd_flags %x \n", cmd, cmd->se_cmd.transport_state,
3279+
cmd->se_cmd.t_state,cmd->se_cmd.se_cmd_flags);
3280+
return EIO;
3281+
}
32673282
cmd->aborted = 1;
32683283
cmd->cmd_flags |= BIT_6;
3284+
spin_unlock_irqrestore(&cmd->cmd_lock, flags);
32693285

3270-
qlt_send_term_exchange(vha, cmd, &cmd->atio, 0);
3286+
qlt_send_term_exchange(vha, cmd, &cmd->atio, 0, 1);
3287+
return 0;
32713288
}
32723289
EXPORT_SYMBOL(qlt_abort_cmd);
32733290

@@ -3282,6 +3299,9 @@ void qlt_free_cmd(struct qla_tgt_cmd *cmd)
32823299

32833300
BUG_ON(cmd->cmd_in_wq);
32843301

3302+
if (cmd->sg_mapped)
3303+
qlt_unmap_sg(cmd->vha, cmd);
3304+
32853305
if (!cmd->q_full)
32863306
qlt_decr_num_pend_cmds(cmd->vha);
32873307

@@ -3399,7 +3419,7 @@ static int qlt_term_ctio_exchange(struct scsi_qla_host *vha, void *ctio,
33993419
term = 1;
34003420

34013421
if (term)
3402-
qlt_send_term_exchange(vha, cmd, &cmd->atio, 1);
3422+
qlt_send_term_exchange(vha, cmd, &cmd->atio, 1, 0);
34033423

34043424
return term;
34053425
}
@@ -3755,6 +3775,7 @@ static void __qlt_do_work(struct qla_tgt_cmd *cmd)
37553775
goto out_term;
37563776
}
37573777

3778+
spin_lock_init(&cmd->cmd_lock);
37583779
cdb = &atio->u.isp24.fcp_cmnd.cdb[0];
37593780
cmd->se_cmd.tag = atio->u.isp24.exchange_addr;
37603781
cmd->unpacked_lun = scsilun_to_int(
@@ -3797,7 +3818,7 @@ static void __qlt_do_work(struct qla_tgt_cmd *cmd)
37973818
*/
37983819
cmd->cmd_flags |= BIT_2;
37993820
spin_lock_irqsave(&ha->hardware_lock, flags);
3800-
qlt_send_term_exchange(vha, NULL, &cmd->atio, 1);
3821+
qlt_send_term_exchange(vha, NULL, &cmd->atio, 1, 0);
38013822

38023823
qlt_decr_num_pend_cmds(vha);
38033824
percpu_ida_free(&sess->se_sess->sess_tag_pool, cmd->se_cmd.map_tag);
@@ -3919,7 +3940,7 @@ static void qlt_create_sess_from_atio(struct work_struct *work)
39193940

39203941
out_term:
39213942
spin_lock_irqsave(&ha->hardware_lock, flags);
3922-
qlt_send_term_exchange(vha, NULL, &op->atio, 1);
3943+
qlt_send_term_exchange(vha, NULL, &op->atio, 1, 0);
39233944
spin_unlock_irqrestore(&ha->hardware_lock, flags);
39243945
kfree(op);
39253946

@@ -4772,7 +4793,7 @@ static void qlt_handle_srr(struct scsi_qla_host *vha,
47724793
dump_stack();
47734794
} else {
47744795
cmd->cmd_flags |= BIT_9;
4775-
qlt_send_term_exchange(vha, cmd, &cmd->atio, 1);
4796+
qlt_send_term_exchange(vha, cmd, &cmd->atio, 1, 0);
47764797
}
47774798
spin_unlock_irqrestore(&ha->hardware_lock, flags);
47784799
}
@@ -4951,7 +4972,7 @@ static void qlt_prepare_srr_imm(struct scsi_qla_host *vha,
49514972
sctio, sctio->srr_id);
49524973
list_del(&sctio->srr_list_entry);
49534974
qlt_send_term_exchange(vha, sctio->cmd,
4954-
&sctio->cmd->atio, 1);
4975+
&sctio->cmd->atio, 1, 0);
49554976
kfree(sctio);
49564977
}
49574978
}
@@ -5124,7 +5145,7 @@ static int __qlt_send_busy(struct scsi_qla_host *vha,
51245145
atio->u.isp24.fcp_hdr.s_id);
51255146
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);
51265147
if (!sess) {
5127-
qlt_send_term_exchange(vha, NULL, atio, 1);
5148+
qlt_send_term_exchange(vha, NULL, atio, 1, 0);
51285149
return 0;
51295150
}
51305151
/* Sending marker isn't necessary, since we called from ISR */
@@ -5407,7 +5428,7 @@ static void qlt_24xx_atio_pkt(struct scsi_qla_host *vha,
54075428
#if 1 /* With TERM EXCHANGE some FC cards refuse to boot */
54085429
qlt_send_busy(vha, atio, SAM_STAT_BUSY);
54095430
#else
5410-
qlt_send_term_exchange(vha, NULL, atio, 1);
5431+
qlt_send_term_exchange(vha, NULL, atio, 1, 0);
54115432
#endif
54125433

54135434
if (!ha_locked)
@@ -5524,7 +5545,7 @@ static void qlt_response_pkt(struct scsi_qla_host *vha, response_t *pkt)
55245545
#if 1 /* With TERM EXCHANGE some FC cards refuse to boot */
55255546
qlt_send_busy(vha, atio, 0);
55265547
#else
5527-
qlt_send_term_exchange(vha, NULL, atio, 1);
5548+
qlt_send_term_exchange(vha, NULL, atio, 1, 0);
55285549
#endif
55295550
} else {
55305551
if (tgt->tgt_stop) {
@@ -5533,7 +5554,7 @@ static void qlt_response_pkt(struct scsi_qla_host *vha, response_t *pkt)
55335554
"command to target, sending TERM "
55345555
"EXCHANGE for rsp\n");
55355556
qlt_send_term_exchange(vha, NULL,
5536-
atio, 1);
5557+
atio, 1, 0);
55375558
} else {
55385559
ql_dbg(ql_dbg_tgt, vha, 0xe060,
55395560
"qla_target(%d): Unable to send "
@@ -5961,7 +5982,7 @@ static void qlt_tmr_work(struct qla_tgt *tgt,
59615982
return;
59625983

59635984
out_term:
5964-
qlt_send_term_exchange(vha, NULL, &prm->tm_iocb2, 0);
5985+
qlt_send_term_exchange(vha, NULL, &prm->tm_iocb2, 1, 0);
59655986
if (sess)
59665987
ha->tgt.tgt_ops->put_sess(sess);
59675988
spin_unlock_irqrestore(&ha->tgt.sess_lock, flags);

‎drivers/scsi/qla2xxx/qla_target.h

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,36 @@ struct qla_tgt_sess {
943943
qlt_plogi_ack_t *plogi_link[QLT_PLOGI_LINK_MAX];
944944
};
945945

946+
typedef enum {
947+
/*
948+
* BIT_0 - Atio Arrival / schedule to work
949+
* BIT_1 - qlt_do_work
950+
* BIT_2 - qlt_do work failed
951+
* BIT_3 - xfer rdy/tcm_qla2xxx_write_pending
952+
* BIT_4 - read respond/tcm_qla2xx_queue_data_in
953+
* BIT_5 - status respond / tcm_qla2xx_queue_status
954+
* BIT_6 - tcm request to abort/Term exchange.
955+
* pre_xmit_response->qlt_send_term_exchange
956+
* BIT_7 - SRR received (qlt_handle_srr->qlt_xmit_response)
957+
* BIT_8 - SRR received (qlt_handle_srr->qlt_rdy_to_xfer)
958+
* BIT_9 - SRR received (qla_handle_srr->qlt_send_term_exchange)
959+
* BIT_10 - Data in - hanlde_data->tcm_qla2xxx_handle_data
960+
961+
* BIT_12 - good completion - qlt_ctio_do_completion -->free_cmd
962+
* BIT_13 - Bad completion -
963+
* qlt_ctio_do_completion --> qlt_term_ctio_exchange
964+
* BIT_14 - Back end data received/sent.
965+
* BIT_15 - SRR prepare ctio
966+
* BIT_16 - complete free
967+
* BIT_17 - flush - qlt_abort_cmd_on_host_reset
968+
* BIT_18 - completion w/abort status
969+
* BIT_19 - completion w/unknown status
970+
* BIT_20 - tcm_qla2xxx_free_cmd
971+
*/
972+
CMD_FLAG_DATA_WORK = BIT_11,
973+
CMD_FLAG_DATA_WORK_FREE = BIT_21,
974+
} cmd_flags_t;
975+
946976
struct qla_tgt_cmd {
947977
struct se_cmd se_cmd;
948978
struct qla_tgt_sess *sess;
@@ -952,6 +982,7 @@ struct qla_tgt_cmd {
952982
/* Sense buffer that will be mapped into outgoing status */
953983
unsigned char sense_buffer[TRANSPORT_SENSE_BUFFER];
954984

985+
spinlock_t cmd_lock;
955986
/* to save extra sess dereferences */
956987
unsigned int conf_compl_supported:1;
957988
unsigned int sg_mapped:1;
@@ -986,30 +1017,8 @@ struct qla_tgt_cmd {
9861017

9871018
uint64_t jiffies_at_alloc;
9881019
uint64_t jiffies_at_free;
989-
/* BIT_0 - Atio Arrival / schedule to work
990-
* BIT_1 - qlt_do_work
991-
* BIT_2 - qlt_do work failed
992-
* BIT_3 - xfer rdy/tcm_qla2xxx_write_pending
993-
* BIT_4 - read respond/tcm_qla2xx_queue_data_in
994-
* BIT_5 - status respond / tcm_qla2xx_queue_status
995-
* BIT_6 - tcm request to abort/Term exchange.
996-
* pre_xmit_response->qlt_send_term_exchange
997-
* BIT_7 - SRR received (qlt_handle_srr->qlt_xmit_response)
998-
* BIT_8 - SRR received (qlt_handle_srr->qlt_rdy_to_xfer)
999-
* BIT_9 - SRR received (qla_handle_srr->qlt_send_term_exchange)
1000-
* BIT_10 - Data in - hanlde_data->tcm_qla2xxx_handle_data
1001-
* BIT_11 - Data actually going to TCM : tcm_qla2xx_handle_data_work
1002-
* BIT_12 - good completion - qlt_ctio_do_completion -->free_cmd
1003-
* BIT_13 - Bad completion -
1004-
* qlt_ctio_do_completion --> qlt_term_ctio_exchange
1005-
* BIT_14 - Back end data received/sent.
1006-
* BIT_15 - SRR prepare ctio
1007-
* BIT_16 - complete free
1008-
* BIT_17 - flush - qlt_abort_cmd_on_host_reset
1009-
* BIT_18 - completion w/abort status
1010-
* BIT_19 - completion w/unknown status
1011-
*/
1012-
uint32_t cmd_flags;
1020+
1021+
cmd_flags_t cmd_flags;
10131022
};
10141023

10151024
struct qla_tgt_sess_work_param {
@@ -1148,7 +1157,7 @@ static inline void sid_to_portid(const uint8_t *s_id, port_id_t *p)
11481157
extern void qlt_response_pkt_all_vps(struct scsi_qla_host *, response_t *);
11491158
extern int qlt_rdy_to_xfer(struct qla_tgt_cmd *);
11501159
extern int qlt_xmit_response(struct qla_tgt_cmd *, int, uint8_t);
1151-
extern void qlt_abort_cmd(struct qla_tgt_cmd *);
1160+
extern int qlt_abort_cmd(struct qla_tgt_cmd *);
11521161
extern void qlt_xmit_tm_rsp(struct qla_tgt_mgmt_cmd *);
11531162
extern void qlt_free_mcmd(struct qla_tgt_mgmt_cmd *);
11541163
extern void qlt_free_cmd(struct qla_tgt_cmd *cmd);

‎drivers/scsi/qla2xxx/tcm_qla2xxx.c

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,10 @@ static void tcm_qla2xxx_free_cmd(struct qla_tgt_cmd *cmd)
298298
{
299299
cmd->vha->tgt_counters.core_qla_free_cmd++;
300300
cmd->cmd_in_wq = 1;
301+
302+
BUG_ON(cmd->cmd_flags & BIT_20);
303+
cmd->cmd_flags |= BIT_20;
304+
301305
INIT_WORK(&cmd->work, tcm_qla2xxx_complete_free);
302306
queue_work_on(smp_processor_id(), tcm_qla2xxx_free_wq, &cmd->work);
303307
}
@@ -374,6 +378,20 @@ static int tcm_qla2xxx_write_pending(struct se_cmd *se_cmd)
374378
{
375379
struct qla_tgt_cmd *cmd = container_of(se_cmd,
376380
struct qla_tgt_cmd, se_cmd);
381+
382+
if (cmd->aborted) {
383+
/* Cmd can loop during Q-full. tcm_qla2xxx_aborted_task
384+
* can get ahead of this cmd. tcm_qla2xxx_aborted_task
385+
* already kick start the free.
386+
*/
387+
pr_debug("write_pending aborted cmd[%p] refcount %d "
388+
"transport_state %x, t_state %x, se_cmd_flags %x\n",
389+
cmd,cmd->se_cmd.cmd_kref.refcount.counter,
390+
cmd->se_cmd.transport_state,
391+
cmd->se_cmd.t_state,
392+
cmd->se_cmd.se_cmd_flags);
393+
return 0;
394+
}
377395
cmd->cmd_flags |= BIT_3;
378396
cmd->bufflen = se_cmd->data_length;
379397
cmd->dma_data_direction = target_reverse_dma_direction(se_cmd);
@@ -405,7 +423,7 @@ static int tcm_qla2xxx_write_pending_status(struct se_cmd *se_cmd)
405423
se_cmd->t_state == TRANSPORT_COMPLETE_QF_WP) {
406424
spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
407425
wait_for_completion_timeout(&se_cmd->t_transport_stop_comp,
408-
3 * HZ);
426+
50);
409427
return 0;
410428
}
411429
spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
@@ -465,13 +483,25 @@ static int tcm_qla2xxx_handle_cmd(scsi_qla_host_t *vha, struct qla_tgt_cmd *cmd,
465483
static void tcm_qla2xxx_handle_data_work(struct work_struct *work)
466484
{
467485
struct qla_tgt_cmd *cmd = container_of(work, struct qla_tgt_cmd, work);
486+
unsigned long flags;
468487

469488
/*
470489
* Ensure that the complete FCP WRITE payload has been received.
471490
* Otherwise return an exception via CHECK_CONDITION status.
472491
*/
473492
cmd->cmd_in_wq = 0;
474-
cmd->cmd_flags |= BIT_11;
493+
494+
spin_lock_irqsave(&cmd->cmd_lock, flags);
495+
cmd->cmd_flags |= CMD_FLAG_DATA_WORK;
496+
if (cmd->aborted) {
497+
cmd->cmd_flags |= CMD_FLAG_DATA_WORK_FREE;
498+
spin_unlock_irqrestore(&cmd->cmd_lock, flags);
499+
500+
tcm_qla2xxx_free_cmd(cmd);
501+
return;
502+
}
503+
spin_unlock_irqrestore(&cmd->cmd_lock, flags);
504+
475505
cmd->vha->tgt_counters.qla_core_ret_ctio++;
476506
if (!cmd->write_data_transferred) {
477507
/*
@@ -546,6 +576,20 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd)
546576
struct qla_tgt_cmd *cmd = container_of(se_cmd,
547577
struct qla_tgt_cmd, se_cmd);
548578

579+
if (cmd->aborted) {
580+
/* Cmd can loop during Q-full. tcm_qla2xxx_aborted_task
581+
* can get ahead of this cmd. tcm_qla2xxx_aborted_task
582+
* already kick start the free.
583+
*/
584+
pr_debug("queue_data_in aborted cmd[%p] refcount %d "
585+
"transport_state %x, t_state %x, se_cmd_flags %x\n",
586+
cmd,cmd->se_cmd.cmd_kref.refcount.counter,
587+
cmd->se_cmd.transport_state,
588+
cmd->se_cmd.t_state,
589+
cmd->se_cmd.se_cmd_flags);
590+
return 0;
591+
}
592+
549593
cmd->cmd_flags |= BIT_4;
550594
cmd->bufflen = se_cmd->data_length;
551595
cmd->dma_data_direction = target_reverse_dma_direction(se_cmd);
@@ -637,11 +681,34 @@ static void tcm_qla2xxx_queue_tm_rsp(struct se_cmd *se_cmd)
637681
qlt_xmit_tm_rsp(mcmd);
638682
}
639683

684+
685+
#define DATA_WORK_NOT_FREE(_flags) \
686+
(( _flags & (CMD_FLAG_DATA_WORK|CMD_FLAG_DATA_WORK_FREE)) == \
687+
CMD_FLAG_DATA_WORK)
640688
static void tcm_qla2xxx_aborted_task(struct se_cmd *se_cmd)
641689
{
642690
struct qla_tgt_cmd *cmd = container_of(se_cmd,
643691
struct qla_tgt_cmd, se_cmd);
644-
qlt_abort_cmd(cmd);
692+
unsigned long flags;
693+
694+
if (qlt_abort_cmd(cmd))
695+
return;
696+
697+
spin_lock_irqsave(&cmd->cmd_lock, flags);
698+
if ((cmd->state == QLA_TGT_STATE_NEW)||
699+
((cmd->state == QLA_TGT_STATE_DATA_IN) &&
700+
DATA_WORK_NOT_FREE(cmd->cmd_flags)) ) {
701+
702+
cmd->cmd_flags |= CMD_FLAG_DATA_WORK_FREE;
703+
spin_unlock_irqrestore(&cmd->cmd_lock, flags);
704+
/* Cmd have not reached firmware.
705+
* Use this trigger to free it. */
706+
tcm_qla2xxx_free_cmd(cmd);
707+
return;
708+
}
709+
spin_unlock_irqrestore(&cmd->cmd_lock, flags);
710+
return;
711+
645712
}
646713

647714
static void tcm_qla2xxx_clear_sess_lookup(struct tcm_qla2xxx_lport *,

0 commit comments

Comments
 (0)