| Message ID | 20221129124204.3049-1-d.bogdanov@yadro.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: qla2xxx: abort TMR commands | expand |
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 --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);
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(-)