diff mbox series

scsi: qla2xxx: abort TMR commands

Message ID 20221129124204.3049-1-d.bogdanov@yadro.com
State Superseded
Headers show
Series scsi: qla2xxx: abort TMR commands | expand

Commit Message

Dmitry Bogdanov Nov. 29, 2022, 12:42 p.m. UTC
TCM calls aborted_task callback for TMR too and then releases the
command. But qla2xx ignores that callback for TMR and leaks an FC
exchange.
Add terminating the exchange of task management IOCBs to free its
FW resources.

Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
---
 drivers/scsi/qla2xxx/qla_target.c  | 121 ++++++++++++++++++++++++-----
 drivers/scsi/qla2xxx/qla_target.h  |   1 +
 drivers/scsi/qla2xxx/tcm_qla2xxx.c |   8 +-
 3 files changed, 111 insertions(+), 19 deletions(-)

Comments

kernel test robot Dec. 4, 2022, 2:56 a.m. UTC | #1
Hi Dmitry,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next linus/master v6.1-rc7 next-20221202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dmitry-Bogdanov/scsi-qla2xxx-abort-TMR-commands/20221130-084923
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link:    https://lore.kernel.org/r/20221129124204.3049-1-d.bogdanov%40yadro.com
patch subject: [PATCH] scsi: qla2xxx: abort TMR commands
config: arm64-randconfig-s043-20221204
compiler: aarch64-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/ab0162ffab887042add8d01af38a9a800aed7150
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Dmitry-Bogdanov/scsi-qla2xxx-abort-TMR-commands/20221130-084923
        git checkout ab0162ffab887042add8d01af38a9a800aed7150
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/clk/meson/ drivers/scsi/qla2xxx/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
>> drivers/scsi/qla2xxx/qla_target.c:1889:29: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le16 [usertype] control_flags @@     got int @@
   drivers/scsi/qla2xxx/qla_target.c:1889:29: sparse:     expected restricted __le16 [usertype] control_flags
   drivers/scsi/qla2xxx/qla_target.c:1889:29: sparse:     got int
   drivers/scsi/qla2xxx/qla_target.c:413:47: sparse: sparse: context imbalance in 'qlt_24xx_atio_pkt_all_vps' - unexpected unlock
   drivers/scsi/qla2xxx/qla_target.c: note: in included file (through arch/arm64/include/asm/io.h, include/linux/io.h, include/linux/irq.h, ...):
   include/asm-generic/io.h:335:15: sparse: sparse: cast to restricted __le32
   drivers/scsi/qla2xxx/qla_target.c:3822:39: sparse: sparse: context imbalance in 'qlt_send_term_exchange' - unexpected unlock
   drivers/scsi/qla2xxx/qla_target.c:5736:39: sparse: sparse: context imbalance in 'qlt_chk_qfull_thresh_hold' - unexpected unlock
   drivers/scsi/qla2xxx/qla_target.c:5775:55: sparse: sparse: context imbalance in 'qlt_24xx_atio_pkt' - unexpected unlock

vim +1889 drivers/scsi/qla2xxx/qla_target.c

  1859	
  1860	static int __qlt_send_term_abts_exchange(struct qla_qpair *qpair,
  1861						 struct scsi_qla_host *vha,
  1862						 struct abts_recv_from_24xx *abts)
  1863	{
  1864		struct qla_hw_data *ha = vha->hw;
  1865		struct abts_resp_to_24xx *resp;
  1866		__le32 f_ctl;
  1867		uint8_t *p;
  1868		int rc;
  1869	
  1870		ql_dbg(ql_dbg_tgt, vha, 0xe006,
  1871		    "Sending terminate exchange of ABTS (ha=%p)\n",
  1872		    ha);
  1873	
  1874		rc = qlt_check_reserve_free_req(qpair, 1);
  1875		if (rc) {
  1876			ql_dbg(ql_dbg_tgt, vha, 0xe04a,
  1877			    "qla_target(%d): %s failed: unable to allocate request packet\n",
  1878			    vha->vp_idx, __func__);
  1879			return -EAGAIN;
  1880		}
  1881	
  1882		resp = (struct abts_resp_to_24xx *)qpair->req->ring_ptr;
  1883		memset(resp, 0, sizeof(*resp));
  1884	
  1885		resp->entry_type = ABTS_RESP_24XX;
  1886		resp->entry_count = 1;
  1887		resp->handle = QLA_TGT_SKIP_HANDLE;
  1888		resp->nport_handle = abts->nport_handle;
> 1889		resp->control_flags = ABTS_CONTR_FLG_TERM_EXCHG;
  1890		resp->vp_index = vha->vp_idx;
  1891		resp->sof_type = abts->sof_type;
  1892		resp->exchange_address = abts->exchange_address;
  1893		/*
  1894		 * Although no basic accept/reject frame is sent to the wire, the basic
  1895		 * accept/reject frame (from IOCB offsets 14h-37h) must be filled by the
  1896		 * driver, because the QLogic ASIC firmware performs a cross-check
  1897		 * before terminating the ABTS exchange.
  1898		 */
  1899		resp->fcp_hdr_le = abts->fcp_hdr_le;
  1900		f_ctl = cpu_to_le32(F_CTL_EXCH_CONTEXT_RESP |
  1901		    F_CTL_LAST_SEQ | F_CTL_END_SEQ |
  1902		    F_CTL_SEQ_INITIATIVE);
  1903		p = (uint8_t *)&f_ctl;
  1904		resp->fcp_hdr_le.f_ctl[0] = *p++;
  1905		resp->fcp_hdr_le.f_ctl[1] = *p++;
  1906		resp->fcp_hdr_le.f_ctl[2] = *p;
  1907	
  1908		resp->fcp_hdr_le.d_id = abts->fcp_hdr_le.s_id;
  1909		resp->fcp_hdr_le.s_id = abts->fcp_hdr_le.d_id;
  1910		resp->fcp_hdr_le.r_ctl = R_CTL_BASIC_LINK_SERV | R_CTL_B_RJT;
  1911		resp->payload.ba_rjt.reason_code =
  1912			BA_RJT_REASON_CODE_UNABLE_TO_PERFORM;
  1913		resp->exchange_addr_to_abort = abts->exchange_addr_to_abort;
  1914	
  1915		vha->vha_tgt.qla_tgt->abts_resp_expected++;
  1916	
  1917		/* Memory Barrier */
  1918		wmb();
  1919		if (qpair->reqq_start_iocbs)
  1920			qpair->reqq_start_iocbs(qpair);
  1921		else
  1922			qla2x00_start_iocbs(vha, qpair->req);
  1923	
  1924		return rc;
  1925	}
  1926	/*
  1927	 * ha->hardware_lock supposed to be held on entry. Might drop it, then reaquire
  1928	 */
  1929	static void qlt_24xx_send_abts_resp(struct qla_qpair *qpair,
  1930		struct abts_recv_from_24xx *abts, uint32_t status,
  1931		bool ids_reversed)
  1932	{
  1933		struct scsi_qla_host *vha = qpair->vha;
  1934		struct qla_hw_data *ha = vha->hw;
  1935		struct abts_resp_to_24xx *resp;
  1936		__le32 f_ctl;
  1937		uint8_t *p;
  1938	
  1939		ql_dbg(ql_dbg_tgt, vha, 0xe006,
  1940		    "Sending task mgmt ABTS response (ha=%p, atio=%p, status=%x\n",
  1941		    ha, abts, status);
  1942	
  1943		resp = (struct abts_resp_to_24xx *)qla2x00_alloc_iocbs_ready(qpair,
  1944		    NULL);
  1945		if (!resp) {
  1946			ql_dbg(ql_dbg_tgt, vha, 0xe04a,
  1947			    "qla_target(%d): %s failed: unable to allocate "
  1948			    "request packet", vha->vp_idx, __func__);
  1949			return;
  1950		}
  1951	
  1952		resp->entry_type = ABTS_RESP_24XX;
  1953		resp->handle = QLA_TGT_SKIP_HANDLE;
  1954		resp->entry_count = 1;
  1955		resp->nport_handle = abts->nport_handle;
  1956		resp->vp_index = vha->vp_idx;
  1957		resp->sof_type = abts->sof_type;
  1958		resp->exchange_address = abts->exchange_address;
  1959		resp->fcp_hdr_le = abts->fcp_hdr_le;
  1960		f_ctl = cpu_to_le32(F_CTL_EXCH_CONTEXT_RESP |
  1961		    F_CTL_LAST_SEQ | F_CTL_END_SEQ |
  1962		    F_CTL_SEQ_INITIATIVE);
  1963		p = (uint8_t *)&f_ctl;
  1964		resp->fcp_hdr_le.f_ctl[0] = *p++;
  1965		resp->fcp_hdr_le.f_ctl[1] = *p++;
  1966		resp->fcp_hdr_le.f_ctl[2] = *p;
  1967		if (ids_reversed) {
  1968			resp->fcp_hdr_le.d_id = abts->fcp_hdr_le.d_id;
  1969			resp->fcp_hdr_le.s_id = abts->fcp_hdr_le.s_id;
  1970		} else {
  1971			resp->fcp_hdr_le.d_id = abts->fcp_hdr_le.s_id;
  1972			resp->fcp_hdr_le.s_id = abts->fcp_hdr_le.d_id;
  1973		}
  1974		resp->exchange_addr_to_abort = abts->exchange_addr_to_abort;
  1975		if (status == FCP_TMF_CMPL) {
  1976			resp->fcp_hdr_le.r_ctl = R_CTL_BASIC_LINK_SERV | R_CTL_B_ACC;
  1977			resp->payload.ba_acct.seq_id_valid = SEQ_ID_INVALID;
  1978			resp->payload.ba_acct.low_seq_cnt = 0x0000;
  1979			resp->payload.ba_acct.high_seq_cnt = cpu_to_le16(0xFFFF);
  1980			resp->payload.ba_acct.ox_id = abts->fcp_hdr_le.ox_id;
  1981			resp->payload.ba_acct.rx_id = abts->fcp_hdr_le.rx_id;
  1982		} else {
  1983			resp->fcp_hdr_le.r_ctl = R_CTL_BASIC_LINK_SERV | R_CTL_B_RJT;
  1984			resp->payload.ba_rjt.reason_code =
  1985				BA_RJT_REASON_CODE_UNABLE_TO_PERFORM;
  1986			/* Other bytes are zero */
  1987		}
  1988	
  1989		vha->vha_tgt.qla_tgt->abts_resp_expected++;
  1990	
  1991		/* Memory Barrier */
  1992		wmb();
  1993		if (qpair->reqq_start_iocbs)
  1994			qpair->reqq_start_iocbs(qpair);
  1995		else
  1996			qla2x00_start_iocbs(vha, qpair->req);
  1997	}
  1998
diff mbox series

Patch

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index bb754a950802..494506ae99d4 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1857,6 +1857,72 @@  static int qlt_build_abts_resp_iocb(struct qla_tgt_mgmt_cmd *mcmd)
 	return rc;
 }
 
+static int __qlt_send_term_abts_exchange(struct qla_qpair *qpair,
+					 struct scsi_qla_host *vha,
+					 struct abts_recv_from_24xx *abts)
+{
+	struct qla_hw_data *ha = vha->hw;
+	struct abts_resp_to_24xx *resp;
+	__le32 f_ctl;
+	uint8_t *p;
+	int rc;
+
+	ql_dbg(ql_dbg_tgt, vha, 0xe006,
+	    "Sending terminate exchange of ABTS (ha=%p)\n",
+	    ha);
+
+	rc = qlt_check_reserve_free_req(qpair, 1);
+	if (rc) {
+		ql_dbg(ql_dbg_tgt, vha, 0xe04a,
+		    "qla_target(%d): %s failed: unable to allocate request packet\n",
+		    vha->vp_idx, __func__);
+		return -EAGAIN;
+	}
+
+	resp = (struct abts_resp_to_24xx *)qpair->req->ring_ptr;
+	memset(resp, 0, sizeof(*resp));
+
+	resp->entry_type = ABTS_RESP_24XX;
+	resp->entry_count = 1;
+	resp->handle = QLA_TGT_SKIP_HANDLE;
+	resp->nport_handle = abts->nport_handle;
+	resp->control_flags = ABTS_CONTR_FLG_TERM_EXCHG;
+	resp->vp_index = vha->vp_idx;
+	resp->sof_type = abts->sof_type;
+	resp->exchange_address = abts->exchange_address;
+	/*
+	 * Although no basic accept/reject frame is sent to the wire, the basic
+	 * accept/reject frame (from IOCB offsets 14h-37h) must be filled by the
+	 * driver, because the QLogic ASIC firmware performs a cross-check
+	 * before terminating the ABTS exchange.
+	 */
+	resp->fcp_hdr_le = abts->fcp_hdr_le;
+	f_ctl = cpu_to_le32(F_CTL_EXCH_CONTEXT_RESP |
+	    F_CTL_LAST_SEQ | F_CTL_END_SEQ |
+	    F_CTL_SEQ_INITIATIVE);
+	p = (uint8_t *)&f_ctl;
+	resp->fcp_hdr_le.f_ctl[0] = *p++;
+	resp->fcp_hdr_le.f_ctl[1] = *p++;
+	resp->fcp_hdr_le.f_ctl[2] = *p;
+
+	resp->fcp_hdr_le.d_id = abts->fcp_hdr_le.s_id;
+	resp->fcp_hdr_le.s_id = abts->fcp_hdr_le.d_id;
+	resp->fcp_hdr_le.r_ctl = R_CTL_BASIC_LINK_SERV | R_CTL_B_RJT;
+	resp->payload.ba_rjt.reason_code =
+		BA_RJT_REASON_CODE_UNABLE_TO_PERFORM;
+	resp->exchange_addr_to_abort = abts->exchange_addr_to_abort;
+
+	vha->vha_tgt.qla_tgt->abts_resp_expected++;
+
+	/* Memory Barrier */
+	wmb();
+	if (qpair->reqq_start_iocbs)
+		qpair->reqq_start_iocbs(qpair);
+	else
+		qla2x00_start_iocbs(vha, qpair->req);
+
+	return rc;
+}
 /*
  * ha->hardware_lock supposed to be held on entry. Might drop it, then reaquire
  */
@@ -3666,10 +3732,9 @@  static void qlt_send_term_imm_notif(struct scsi_qla_host *vha,
  * This function sends the appropriate CTIO to ISP 2xxx or 24xx
  */
 static int __qlt_send_term_exchange(struct qla_qpair *qpair,
-	struct qla_tgt_cmd *cmd,
+	struct scsi_qla_host *vha,
 	struct atio_from_isp *atio)
 {
-	struct scsi_qla_host *vha = qpair->vha;
 	struct ctio7_to_24xx *ctio24;
 	struct qla_hw_data *ha = vha->hw;
 	request_t *pkt;
@@ -3678,9 +3743,6 @@  static int __qlt_send_term_exchange(struct qla_qpair *qpair,
 
 	ql_dbg(ql_dbg_tgt, vha, 0xe009, "Sending TERM EXCH CTIO (ha=%p)\n", ha);
 
-	if (cmd)
-		vha = cmd->vha;
-
 	pkt = (request_t *)qla2x00_alloc_iocbs_ready(qpair, NULL);
 	if (pkt == NULL) {
 		ql_dbg(ql_dbg_tgt, vha, 0xe050,
@@ -3689,16 +3751,6 @@  static int __qlt_send_term_exchange(struct qla_qpair *qpair,
 		return -ENOMEM;
 	}
 
-	if (cmd != NULL) {
-		if (cmd->state < QLA_TGT_STATE_PROCESSED) {
-			ql_dbg(ql_dbg_tgt, vha, 0xe051,
-			    "qla_target(%d): Terminating cmd %p with "
-			    "incorrect state %d\n", vha->vp_idx, cmd,
-			    cmd->state);
-		} else
-			ret = 1;
-	}
-
 	qpair->tgt_counters.num_term_xchg_sent++;
 	pkt->entry_count = 1;
 	pkt->handle = QLA_TGT_SKIP_HANDLE | CTIO_COMPLETION_HANDLE_MARK;
@@ -3739,14 +3791,23 @@  static void qlt_send_term_exchange(struct qla_qpair *qpair,
 	else
 		vha = qpair->vha;
 
+	if (cmd != NULL) {
+		if (cmd->state < QLA_TGT_STATE_PROCESSED) {
+			ql_dbg(ql_dbg_tgt, vha, 0xe051,
+			    "qla_target(%d): Terminating cmd %p with "
+			    "incorrect state %d\n", vha->vp_idx, cmd,
+			    cmd->state);
+		}
+	}
+
 	if (ha_locked) {
-		rc = __qlt_send_term_exchange(qpair, cmd, atio);
+		rc = __qlt_send_term_exchange(qpair, vha, atio);
 		if (rc == -ENOMEM)
 			qlt_alloc_qfull_cmd(vha, atio, 0, 0);
 		goto done;
 	}
 	spin_lock_irqsave(qpair->qp_lock_ptr, flags);
-	rc = __qlt_send_term_exchange(qpair, cmd, atio);
+	rc = __qlt_send_term_exchange(qpair, vha, atio);
 	if (rc == -ENOMEM)
 		qlt_alloc_qfull_cmd(vha, atio, 0, 0);
 
@@ -3810,6 +3871,30 @@  static void qlt_chk_exch_leak_thresh_hold(struct scsi_qla_host *vha)
 
 }
 
+int qlt_abort_mcmd(struct qla_tgt_mgmt_cmd *mcmd)
+{
+	struct scsi_qla_host *vha = mcmd->vha;
+	struct se_cmd *se_cmd = &mcmd->se_cmd;
+	unsigned long flags;
+	int rc = 0;
+
+	ql_dbg(ql_dbg_tgt_mgt, vha, 0xf022,
+	    "qla_target(%d): terminating exchange for aborted mcmd=%p (se_cmd=%p, tag=%llu)",
+	    vha->vp_idx, mcmd, se_cmd, se_cmd->tag);
+
+	spin_lock_irqsave(mcmd->qpair->qp_lock_ptr, flags);
+	if (mcmd->orig_iocb.atio.u.raw.entry_type == ABTS_RECV_24XX)
+		rc = __qlt_send_term_abts_exchange(mcmd->qpair, vha,
+						   &mcmd->orig_iocb.abts);
+	else
+		rc = __qlt_send_term_exchange(mcmd->qpair, vha,
+					      &mcmd->orig_iocb.atio);
+	spin_unlock_irqrestore(mcmd->qpair->qp_lock_ptr, flags);
+
+	return rc;
+}
+EXPORT_SYMBOL(qlt_abort_mcmd);
+
 int qlt_abort_cmd(struct qla_tgt_cmd *cmd)
 {
 	struct qla_tgt *tgt = cmd->tgt;
@@ -5578,7 +5663,7 @@  qlt_free_qfull_cmds(struct qla_qpair *qpair)
 			/* cmd->state is a borrowed field to hold status */
 			rc = __qlt_send_busy(qpair, &cmd->atio, cmd->state);
 		else if (cmd->term_exchg)
-			rc = __qlt_send_term_exchange(qpair, NULL, &cmd->atio);
+			rc = __qlt_send_term_exchange(qpair, vha, &cmd->atio);
 
 		if (rc == -ENOMEM)
 			break;
diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
index 7df86578214f..7902f5b5f4aa 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -1057,6 +1057,7 @@  extern void qlt_response_pkt_all_vps(struct scsi_qla_host *, struct rsp_que *,
 extern int qlt_rdy_to_xfer(struct qla_tgt_cmd *);
 extern int qlt_xmit_response(struct qla_tgt_cmd *, int, uint8_t);
 extern int qlt_abort_cmd(struct qla_tgt_cmd *);
+extern int qlt_abort_mcmd(struct qla_tgt_mgmt_cmd *mcmd);
 extern void qlt_xmit_tm_rsp(struct qla_tgt_mgmt_cmd *);
 extern void qlt_free_mcmd(struct qla_tgt_mgmt_cmd *);
 extern void qlt_free_cmd(struct qla_tgt_cmd *cmd);
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 8fa0056b56dd..0ef09b43e0f7 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -773,8 +773,14 @@  static void tcm_qla2xxx_aborted_task(struct se_cmd *se_cmd)
 	struct qla_tgt_cmd *cmd;
 	unsigned long flags;
 
-	if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
+	if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB) {
+		struct qla_tgt_mgmt_cmd *mcmd = container_of(se_cmd,
+				struct qla_tgt_mgmt_cmd, se_cmd);
+
+		qlt_abort_mcmd(mcmd);
+
 		return;
+	}
 
 	cmd  = container_of(se_cmd, struct qla_tgt_cmd, se_cmd);