qla2xxx patches for kernel v5.12 and v5.13
Subsystems (2)
v2 cover qla2xxx patches for kernel v5.12 and v5.13
Hi Martin,
Please consider the first patch in this series for kernel v5.12 and the remaining patches for kernel v5.13.
Thanks,
Bart.
Changes compared to v1:
- Improved the description of patch 1/6.
- Dropped one patch that is already upstream.
Bart Van Assche (6):
Revert "qla2xxx: Make sure that aborted commands are freed"
qla2xxx: Constify struct qla_tgt_func_tmpl
qla2xxx: Fix endianness annotations
qla2xxx: Suppress Coverity complaints about dseg_r*
qla2xxx: Simplify qla8044_minidump_process_control()
qla2xxx: Always check the return value of qla24xx_get_isp_stats()
drivers/scsi/qla2xxx/qla_attr.c | 6 +++++- drivers/scsi/qla2xxx/qla_def.h | 4 ++-- drivers/scsi/qla2xxx/qla_iocb.c | 3 ++- drivers/scsi/qla2xxx/qla_isr.c | 2 +- drivers/scsi/qla2xxx/qla_mr.c | 12 ++++++------ drivers/scsi/qla2xxx/qla_mr.h | 4 ++-- drivers/scsi/qla2xxx/qla_nx2.c | 8 -------- drivers/scsi/qla2xxx/qla_sup.c | 9 +++++---- drivers/scsi/qla2xxx/qla_target.c | 13 +++++-------- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 6 +----- 10 files changed, 29 insertions(+), 38 deletions(-)
PATCH v2 1/6 Revert "qla2xxx: Make sure that aborted commands are freed"
Calling vha->hw->tgt.tgt_ops->free_cmd() from qlt_xmit_response() is wrong since the command for which a response is sent must remain valid until the SCSI target core calls .release_cmd(). It has been observed that the following scenario triggers a kernel crash:
- qlt_xmit_response() calls qlt_check_reserve_free_req().
- qlt_check_reserve_free_req() returns -EAGAIN.
- qlt_xmit_response() calls vha->hw->tgt.tgt_ops->free_cmd(cmd).
- transport_handle_queue_full() tries to retransmit the response.
Fix this crash by reverting the patch that introduced it.
Fixes 0dcec41acb85 ("scsi: qla2xxx: Make sure that aborted commands are freed")
Cc Quinn Tran <[email protected]>
Cc Mike Christie <[email protected]>
Reviewed-by Daniel Wagner <[email protected]>
Reviewed-by Himanshu Madhani <[email protected]>
Signed-off-by Bart Van Assche <[email protected]>
--- drivers/scsi/qla2xxx/qla_target.c | 13 +++++-------- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 4 ---- 2 files changed, 5 insertions(+), 12 deletions(-)
drivers/scsi/qla2xxx/qla_target.c +5 -8
@@ -3222,8 +3222,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,if (!qpair->fw_started || (cmd->reset_count != qpair->chip_reset) || (cmd->sess && cmd->sess->deleted)) { cmd->state =QLA_TGT_STATE_PROCESSED ; - res =0 ; - goto free; +return 0 ; } ql_dbg_qp(ql_dbg_tgt, qpair, 0xe018,
@@ -3234,8 +3233,9 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type, res =qlt_pre_xmit_response (cmd, &prm, xmit_type, scsi_status, &full_req_cnt); -if (unlikely (res !=0 )) - goto free; +if (unlikely (res !=0 )) { +return res; + }spin_lock_irqsave (qpair->qp_lock_ptr, flags);
@@ -3255,8 +3255,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type, vha->flags.online,qla2x00_reset_active (vha), cmd->reset_count, qpair->chip_reset);spin_unlock_irqrestore (qpair->qp_lock_ptr, flags); - res =0 ; - goto free; +return 0 ; }/* Does F/W have an IOCBs for this request */
@@ -3359,8 +3358,6 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,qlt_unmap_sg (vha, cmd);spin_unlock_irqrestore (qpair->qp_lock_ptr, flags); -free: - vha->hw->tgt.tgt_ops->free_cmd (cmd);return res; }EXPORT_SYMBOL (qlt_xmit_response);
drivers/scsi/qla2xxx/tcm_qla2xxx.c +0 -4
@@ -653,7 +653,6 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd) {struct qla_tgt_cmd *cmd =container_of (se_cmd, struct qla_tgt_cmd, se_cmd); -struct scsi_qla_host *vha = cmd->vha;if (cmd->aborted) {/* Cmd can loop during Q-full. tcm_qla2xxx_aborted_task
@@ -666,7 +665,6 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd) cmd->se_cmd.transport_state, cmd->se_cmd.t_state, cmd->se_cmd.se_cmd_flags); - vha->hw->tgt.tgt_ops->free_cmd (cmd);return 0 ; }
@@ -694,7 +692,6 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd) {struct qla_tgt_cmd *cmd =container_of (se_cmd, struct qla_tgt_cmd, se_cmd); -struct scsi_qla_host *vha = cmd->vha;int xmit_type =QLA_TGT_XMIT_STATUS ;if (cmd->aborted) {
@@ -708,7 +705,6 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd) cmd,kref_read (&cmd->se_cmd.cmd_kref), cmd->se_cmd.transport_state, cmd->se_cmd.t_state, cmd->se_cmd.se_cmd_flags); - vha->hw->tgt.tgt_ops->free_cmd (cmd);return 0 ; } cmd->bufflen = se_cmd->data_length;
PATCH v2 2/6 qla2xxx: Constify struct qla_tgt_func_tmpl
In reply to Bart Van AsscheSince the target function pointers are not modified at runtime, declare the data structure with the target function pointers const.
Cc Quinn Tran <[email protected]>
Cc Mike Christie <[email protected]>
Reviewed-by Daniel Wagner <[email protected]>
Reviewed-by Himanshu Madhani <[email protected]>
Signed-off-by Bart Van Assche <[email protected]>
--- drivers/scsi/qla2xxx/qla_def.h | 2 +- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/qla2xxx/qla_def.h +1 -1
@@ -3815,7 +3815,7 @@ struct qlt_hw_data {__le32 __iomem *atio_q_in;__le32 __iomem *atio_q_out; -struct qla_tgt_func_tmpl *tgt_ops; +const struct qla_tgt_func_tmpl *tgt_ops;struct qla_tgt_vp_map *tgt_vp_map;int saved_set;
drivers/scsi/qla2xxx/tcm_qla2xxx.c +1 -1
@@ -1578,7 +1578,7 @@ static void tcm_qla2xxx_update_sess(struct fc_port *sess, port_id_t s_id,/* * Calls into tcm_qla2xxx used by qla2xxx LLD I/O path. */ -static struct qla_tgt_func_tmpl tcm_qla2xxx_template = { +static const struct qla_tgt_func_tmpl tcm_qla2xxx_template = { .find_cmd_by_tag = tcm_qla2xxx_find_cmd_by_tag, .handle_cmd = tcm_qla2xxx_handle_cmd, .handle_data = tcm_qla2xxx_handle_data,
Re: [PATCH v2 2/6] qla2xxx: Constify struct qla_tgt_func_tmpl
Quoted 32 lines · Bart Van Assche
Since the target function pointers are not modified at runtime, declare the data structure with the target function pointers const.
Cc Quinn Tran <[email protected]>Cc Mike Christie <[email protected]>Reviewed-by Daniel Wagner <[email protected]>Reviewed-by Himanshu Madhani <[email protected]>Signed-off-by Bart Van Assche <[email protected]>--- drivers/scsi/qla2xxx/qla_def.h | 2 +- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)drivers/scsi/qla2xxx/qla_def.h +1
-1@@ -3815,7 +3815,7 @@ struct qlt_hw_data {__le32 __iomem *atio_q_in;__le32 __iomem *atio_q_out; -struct qla_tgt_func_tmpl *tgt_ops; +const struct qla_tgt_func_tmpl *tgt_ops;struct qla_tgt_vp_map *tgt_vp_map;int saved_set;drivers/scsi/qla2xxx/tcm_qla2xxx.c +0
-0
drivers/scsi/qla2xxx/tcm_qla2xxx.c static void tcm_qla2xxx_update_sess(struct fc_port *sess, port_id_t s_id, · line 1584
Quoted 8 lines
drivers/scsi/qla2xxx/tcm_qla2xxx.c +1
-1@@ -1578,7 +1578,7 @@ static void tcm_qla2xxx_update_sess(struct fc_port *sess, port_id_t s_id,/* * Calls into tcm_qla2xxx used by qla2xxx LLD I/O path. */ -static struct qla_tgt_func_tmpl tcm_qla2xxx_template = { +static const struct qla_tgt_func_tmpl tcm_qla2xxx_template = { .find_cmd_by_tag = tcm_qla2xxx_find_cmd_by_tag, .handle_cmd = tcm_qla2xxx_handle_cmd, .handle_data = tcm_qla2xxx_handle_data,
Reviewed-by Lee Duncan <[email protected]>
PATCH v2 3/6 qla2xxx: Fix endianness annotations
In reply to Bart Van AsscheFix all recently introduced endianness annotation issues.
Cc Quinn Tran <[email protected]>
Cc Mike Christie <[email protected]>
Reviewed-by Daniel Wagner <[email protected]>
Reviewed-by Himanshu Madhani <[email protected]>
Signed-off-by Bart Van Assche <[email protected]>
--- drivers/scsi/qla2xxx/qla_def.h | 2 +- drivers/scsi/qla2xxx/qla_iocb.c | 3 ++- drivers/scsi/qla2xxx/qla_isr.c | 2 +- drivers/scsi/qla2xxx/qla_sup.c | 9 +++++---- 4 files changed, 9 insertions(+), 7 deletions(-)
drivers/scsi/qla2xxx/qla_def.h +1 -1
@@ -1527,7 +1527,7 @@ struct init_sf_cb {* BIT_12 = Remote Write Optimization (1 - Enabled, 0 - Disabled) * BIT 11-0 = Reserved */ -uint16_t flags; +__le16 flags;uint8_t reserved1[32 ];uint16_t discard_OHRB_timeout_value;uint16_t remote_write_opt_queue_num;
drivers/scsi/qla2xxx/qla_iocb.c +2 -1
@@ -2379,7 +2379,8 @@ qla24xx_prli_iocb(srb_t *sp, struct logio_entry_24xx *logio)cpu_to_le32 (NVME_PRLI_SP_FIRST_BURST );if (sp->vha->flags.nvme2_enabled) {/* Set service parameter BIT_7 for NVME CONF support */ - logio->io_parameter[0 ] |=NVME_PRLI_SP_CONF ; + logio->io_parameter[0 ] |= +cpu_to_le32 (NVME_PRLI_SP_CONF );/* Set service parameter BIT_8 for SLER support */ logio->io_parameter[0 ] |=cpu_to_le32 (NVME_PRLI_SP_SLER );
drivers/scsi/qla2xxx/qla_isr.c +1 -1
@@ -3440,7 +3440,7 @@ qla24xx_abort_iocb_entry(scsi_qla_host_t *vha, struct req_que *req,return ; abt = &sp->u.iocb_cmd; - abt->u.abt.comp_status =le16_to_cpu (pkt->comp_status); + abt->u.abt.comp_status = pkt->comp_status; orig_sp = sp->cmd_sp;/* Need to pass original sp */ if (orig_sp)
drivers/scsi/qla2xxx/qla_sup.c +5 -4
@@ -2621,10 +2621,11 @@ qla24xx_read_optrom_data(struct scsi_qla_host *vha, void *buf, }static int -qla28xx_extract_sfub_and_verify (struct scsi_qla_host *vha,uint32_t *buf, +qla28xx_extract_sfub_and_verify (struct scsi_qla_host *vha,__le32 *buf,uint32_t len,uint32_t buf_size_without_sfub,uint8_t *sfub_buf) { -uint32_t *p, check_sum =0 ; +uint32_t check_sum =0 ; +__le32 *p;int i; p = buf + buf_size_without_sfub;
@@ -2790,8 +2791,8 @@ qla28xx_write_flash_data(scsi_qla_host_t *vha, uint32_t *dwptr, uint32_t faddr, goto done; } - rval =qla28xx_extract_sfub_and_verify (vha, dwptr, dwords, - buf_size_without_sfub, (uint8_t *)sfub); + rval =qla28xx_extract_sfub_and_verify (vha, (__le32 *)dwptr, + dwords, buf_size_without_sfub, (uint8_t *)sfub);if (rval !=QLA_SUCCESS ) goto done;
Re: [PATCH v2 3/6] qla2xxx: Fix endianness annotations
Quoted 74 lines · Bart Van Assche
Fix all recently introduced endianness annotation issues.
Cc Quinn Tran <[email protected]>Cc Mike Christie <[email protected]>Reviewed-by Daniel Wagner <[email protected]>Reviewed-by Himanshu Madhani <[email protected]>Signed-off-by Bart Van Assche <[email protected]>--- drivers/scsi/qla2xxx/qla_def.h | 2 +- drivers/scsi/qla2xxx/qla_iocb.c | 3 ++- drivers/scsi/qla2xxx/qla_isr.c | 2 +- drivers/scsi/qla2xxx/qla_sup.c | 9 +++++---- 4 files changed, 9 insertions(+), 7 deletions(-)drivers/scsi/qla2xxx/qla_def.h +1
-1@@ -1527,7 +1527,7 @@ struct init_sf_cb {* BIT_12 = Remote Write Optimization (1 - Enabled, 0 - Disabled) * BIT 11-0 = Reserved */ -uint16_t flags; +__le16 flags;uint8_t reserved1[32 ];uint16_t discard_OHRB_timeout_value;uint16_t remote_write_opt_queue_num;drivers/scsi/qla2xxx/qla_iocb.c +2
-1@@ -2379,7 +2379,8 @@ qla24xx_prli_iocb(srb_t *sp, struct logio_entry_24xx *logio)cpu_to_le32 (NVME_PRLI_SP_FIRST_BURST );if (sp->vha->flags.nvme2_enabled) {/* Set service parameter BIT_7 for NVME CONF support */ - logio->io_parameter[0 ] |=NVME_PRLI_SP_CONF ; + logio->io_parameter[0 ] |= +cpu_to_le32 (NVME_PRLI_SP_CONF );/* Set service parameter BIT_8 for SLER support */ logio->io_parameter[0 ] |=cpu_to_le32 (NVME_PRLI_SP_SLER );drivers/scsi/qla2xxx/qla_isr.c +1
-1@@ -3440,7 +3440,7 @@ qla24xx_abort_iocb_entry(scsi_qla_host_t *vha, struct req_que *req,return ; abt = &sp->u.iocb_cmd; - abt->u.abt.comp_status =le16_to_cpu (pkt->comp_status); + abt->u.abt.comp_status = pkt->comp_status; orig_sp = sp->cmd_sp;/* Need to pass original sp */ if (orig_sp)drivers/scsi/qla2xxx/qla_sup.c +3
-2@@ -2621,10 +2621,11 @@ qla24xx_read_optrom_data(struct scsi_qla_host *vha, void *buf, }static int -qla28xx_extract_sfub_and_verify (struct scsi_qla_host *vha,uint32_t *buf, +qla28xx_extract_sfub_and_verify (struct scsi_qla_host *vha,__le32 *buf,uint32_t len,uint32_t buf_size_without_sfub,uint8_t *sfub_buf) { -uint32_t *p, check_sum =0 ; +uint32_t check_sum =0 ; +__le32 *p;int i; p = buf + buf_size_without_sfub;
drivers/scsi/qla2xxx/qla_sup.c qla28xx_write_flash_data(scsi_qla_host_t *vha, uint32_t *dwptr, uint32_t faddr, · line 2798
Quoted 10 lines
drivers/scsi/qla2xxx/qla_sup.c +2
-2@@ -2790,8 +2791,8 @@ qla28xx_write_flash_data(scsi_qla_host_t *vha, uint32_t *dwptr, uint32_t faddr, goto done; } - rval =qla28xx_extract_sfub_and_verify (vha, dwptr, dwords, - buf_size_without_sfub, (uint8_t *)sfub); + rval =qla28xx_extract_sfub_and_verify (vha, (__le32 *)dwptr, + dwords, buf_size_without_sfub, (uint8_t *)sfub);if (rval !=QLA_SUCCESS ) goto done;
Reviewed-by Lee Duncan <[email protected]>
PATCH v2 4/6 qla2xxx: Suppress Coverity complaints about dseg_r*
In reply to Bart Van AsscheChange dseq_rq and dseg_rsp from scalar structure members into single-element arrays such that Coverity does not complain about the (*cur_dsd)++ statement in append_dsd64().
Cc Quinn Tran <[email protected]>
Cc Mike Christie <[email protected]>
Reviewed-by Daniel Wagner <[email protected]>
Reviewed-by Himanshu Madhani <[email protected]>
Signed-off-by Bart Van Assche <[email protected]>
--- drivers/scsi/qla2xxx/qla_mr.c | 12 ++++++------ drivers/scsi/qla2xxx/qla_mr.h | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-)
drivers/scsi/qla2xxx/qla_mr.c +6 -6
@@ -3266,8 +3266,8 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb) fx_iocb.req_xfrcnt =cpu_to_le16 (fxio->u.fxiocb.req_len);put_unaligned_le64 (fxio->u.fxiocb.req_dma_handle, - &fx_iocb.dseg_rq.address); - fx_iocb.dseg_rq.length = + &fx_iocb.dseg_rq[0 ].address); + fx_iocb.dseg_rq[0 ].length =cpu_to_le32 (fxio->u.fxiocb.req_len); }
@@ -3276,8 +3276,8 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb) fx_iocb.rsp_xfrcnt =cpu_to_le16 (fxio->u.fxiocb.rsp_len);put_unaligned_le64 (fxio->u.fxiocb.rsp_dma_handle, - &fx_iocb.dseg_rsp.address); - fx_iocb.dseg_rsp.length = + &fx_iocb.dseg_rsp[0 ].address); + fx_iocb.dseg_rsp[0 ].length =cpu_to_le32 (fxio->u.fxiocb.rsp_len); }
@@ -3314,7 +3314,7 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)cpu_to_le16 (bsg_job->request_payload.sg_cnt); tot_dsds = bsg_job->request_payload.sg_cnt; - cur_dsd = &fx_iocb.dseg_rq; + cur_dsd = &fx_iocb.dseg_rq[0 ]; avail_dsds =1 ;for_each_sg (bsg_job->request_payload.sg_list, sg, tot_dsds, index) {
@@ -3369,7 +3369,7 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb) fx_iocb.rsp_dsdcnt =cpu_to_le16 (bsg_job->reply_payload.sg_cnt); tot_dsds = bsg_job->reply_payload.sg_cnt; - cur_dsd = &fx_iocb.dseg_rsp; + cur_dsd = &fx_iocb.dseg_rsp[0 ]; avail_dsds =1 ; for_each_sg(bsg_job->reply_payload.sg_list, sg,
drivers/scsi/qla2xxx/qla_mr.h +2 -2
@@ -176,8 +176,8 @@ struct fxdisc_entry_fx00 {uint8_t flags;uint8_t reserved_1; -struct dsd64 dseg_rq; -struct dsd64 dseg_rsp; +struct dsd64 dseg_rq[1 ]; +struct dsd64 dseg_rsp[1 ];__le32 dataword;__le32 adapid;
Re: [PATCH v2 4/6] qla2xxx: Suppress Coverity complaints about dseg_r*
Quoted 64 lines · Bart Van Assche
Change
dseq_rqanddseg_rspfrom scalar structure members into single-element arrays such that Coverity does not complain about the (*cur_dsd)++ statement inappend_dsd64().Cc Quinn Tran <[email protected]>Cc Mike Christie <[email protected]>Reviewed-by Daniel Wagner <[email protected]>Reviewed-by Himanshu Madhani <[email protected]>Signed-off-by Bart Van Assche <[email protected]>--- drivers/scsi/qla2xxx/qla_mr.c | 12 ++++++------ drivers/scsi/qla2xxx/qla_mr.h | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-)drivers/scsi/qla2xxx/qla_mr.c +6
-6@@ -3266,8 +3266,8 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb) fx_iocb.req_xfrcnt =cpu_to_le16 (fxio->u.fxiocb.req_len);put_unaligned_le64 (fxio->u.fxiocb.req_dma_handle, - &fx_iocb.dseg_rq.address); - fx_iocb.dseg_rq.length = + &fx_iocb.dseg_rq[0 ].address); + fx_iocb.dseg_rq[0 ].length =cpu_to_le32 (fxio->u.fxiocb.req_len); }@@ -3276,8 +3276,8 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb) fx_iocb.rsp_xfrcnt =cpu_to_le16 (fxio->u.fxiocb.rsp_len);put_unaligned_le64 (fxio->u.fxiocb.rsp_dma_handle, - &fx_iocb.dseg_rsp.address); - fx_iocb.dseg_rsp.length = + &fx_iocb.dseg_rsp[0 ].address); + fx_iocb.dseg_rsp[0 ].length =cpu_to_le32 (fxio->u.fxiocb.rsp_len); }@@ -3314,7 +3314,7 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb)cpu_to_le16 (bsg_job->request_payload.sg_cnt); tot_dsds = bsg_job->request_payload.sg_cnt; - cur_dsd = &fx_iocb.dseg_rq; + cur_dsd = &fx_iocb.dseg_rq[0 ]; avail_dsds =1 ;for_each_sg (bsg_job->request_payload.sg_list, sg, tot_dsds, index) {@@ -3369,7 +3369,7 @@ qlafx00_fxdisc_iocb(srb_t *sp, struct fxdisc_entry_fx00 *pfxiocb) fx_iocb.rsp_dsdcnt =cpu_to_le16 (bsg_job->reply_payload.sg_cnt); tot_dsds = bsg_job->reply_payload.sg_cnt; - cur_dsd = &fx_iocb.dseg_rsp; + cur_dsd = &fx_iocb.dseg_rsp[0 ]; avail_dsds =1 ; for_each_sg(bsg_job->reply_payload.sg_list, sg,drivers/scsi/qla2xxx/qla_mr.h +0
-0
drivers/scsi/qla2xxx/qla_mr.h struct fxdisc_entry_fx00 { · line 183
Quoted 10 lines
drivers/scsi/qla2xxx/qla_mr.h +2
-2@@ -176,8 +176,8 @@ struct fxdisc_entry_fx00 {uint8_t flags;- & uint8_t reserved_1; -struct dsd64 dseg_rq; -struct dsd64 dseg_rsp; +struct dsd64 dseg_rq[1 ]; +struct dsd64 dseg_rsp[1 ];__le32 dataword;__le32 adapid;
I dislike such an uncommented change just to keep a tool happy. Could you add a comment in qla_mr.h saying why these are one-element-long arrays, just so nobody optimizes those out later?
If you do that, please add my:
Reviewed-by Lee Duncan <[email protected]>
Signature · 1 line
Lee Duncan
Re: [PATCH v2 4/6] qla2xxx: Suppress Coverity complaints about dseg_r*
Quoted 3 lines · Lee Duncan
I dislike such an uncommented change just to keep a tool happy. Could you add a comment in
qla_mr.hsaying why these are one-element-long arrays, just so nobody optimizes those out later?
I will add a comment. Thanks for the feedback.
Bart.
PATCH v2 5/6 qla2xxx: Simplify qla8044_minidump_process_control()
In reply to Bart Van AsscheThis patch fixes the following Coverity complaint:
CID 177490 (#1 of 1): Unused value (UNUSED_VALUE)
assigned_value: Assigning value from opcode & 0xffffff7fU to opcode
here, but that stored value is overwritten before it can be used.Cc Quinn Tran <[email protected]>
Cc Mike Christie <[email protected]>
Reviewed-by Daniel Wagner <[email protected]>
Reviewed-by Himanshu Madhani <[email protected]>
Signed-off-by Bart Van Assche <[email protected]>
--- drivers/scsi/qla2xxx/qla_nx2.c | 8 -------- 1 file changed, 8 deletions(-)
drivers/scsi/qla2xxx/qla_nx2.c +0 -8
@@ -2226,19 +2226,16 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,if (opcode &QLA82XX_DBG_OPCODE_WR ) {qla8044_wr_reg_indirect (vha, crb_addr, crb_entry->value_1); - opcode &= ~QLA82XX_DBG_OPCODE_WR ; }if (opcode &QLA82XX_DBG_OPCODE_RW ) {qla8044_rd_reg_indirect (vha, crb_addr, &read_value);qla8044_wr_reg_indirect (vha, crb_addr, read_value); - opcode &= ~QLA82XX_DBG_OPCODE_RW ; }if (opcode &QLA82XX_DBG_OPCODE_AND ) {qla8044_rd_reg_indirect (vha, crb_addr, &read_value); read_value &= crb_entry->value_2; - opcode &= ~QLA82XX_DBG_OPCODE_AND ;if (opcode &QLA82XX_DBG_OPCODE_OR ) { read_value |= crb_entry->value_3; opcode &= ~QLA82XX_DBG_OPCODE_OR ;
@@ -2249,7 +2246,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,qla8044_rd_reg_indirect (vha, crb_addr, &read_value); read_value |= crb_entry->value_3;qla8044_wr_reg_indirect (vha, crb_addr, read_value); - opcode &= ~QLA82XX_DBG_OPCODE_OR ; }if (opcode &QLA82XX_DBG_OPCODE_POLL ) { poll_time = crb_entry->crb_strd.poll_timeout;
@@ -2269,7 +2265,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha, crb_addr, &read_value); } }while (1 ); - opcode &= ~QLA82XX_DBG_OPCODE_POLL ; }if (opcode &QLA82XX_DBG_OPCODE_RDSTATE ) {
@@ -2283,7 +2278,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,qla8044_rd_reg_indirect (vha, addr, &read_value); index = crb_entry->crb_ctrl.state_index_v; tmplt_hdr->saved_state_array[index] = read_value; - opcode &= ~QLA82XX_DBG_OPCODE_RDSTATE ; }if (opcode &QLA82XX_DBG_OPCODE_WRSTATE ) {
@@ -2303,7 +2297,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha, }qla8044_wr_reg_indirect (vha, addr, read_value); - opcode &= ~QLA82XX_DBG_OPCODE_WRSTATE ; }if (opcode &QLA82XX_DBG_OPCODE_MDSTATE ) {
@@ -2316,7 +2309,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha, read_value |= crb_entry->value_3; read_value += crb_entry->value_1; tmplt_hdr->saved_state_array[index] = read_value; - opcode &= ~QLA82XX_DBG_OPCODE_MDSTATE ; } crb_addr += crb_entry->crb_strd.addr_stride; }
Re: [PATCH v2 5/6] qla2xxx: Simplify qla8044_minidump_process_control()
Quoted 73 lines · Bart Van Assche
This patch fixes the following Coverity complaint:
CID 177490 (#1 of 1): Unused value (UNUSED_VALUE) assigned_value: Assigning value from opcode & 0xffffff7fU to opcode here, but that stored value is overwritten before it can be used.Cc Quinn Tran <[email protected]>Cc Mike Christie <[email protected]>Reviewed-by Daniel Wagner <[email protected]>Reviewed-by Himanshu Madhani <[email protected]>Signed-off-by Bart Van Assche <[email protected]>--- drivers/scsi/qla2xxx/qla_nx2.c | 8 -------- 1 file changed, 8 deletions(-)drivers/scsi/qla2xxx/qla_nx2.c +0
-7@@ -2226,19 +2226,16 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,if (opcode &QLA82XX_DBG_OPCODE_WR ) {qla8044_wr_reg_indirect (vha, crb_addr, crb_entry->value_1); - opcode &= ~QLA82XX_DBG_OPCODE_WR ; }if (opcode &QLA82XX_DBG_OPCODE_RW ) {qla8044_rd_reg_indirect (vha, crb_addr, &read_value);qla8044_wr_reg_indirect (vha, crb_addr, read_value); - opcode &= ~QLA82XX_DBG_OPCODE_RW ; }if (opcode &QLA82XX_DBG_OPCODE_AND ) {qla8044_rd_reg_indirect (vha, crb_addr, &read_value); read_value &= crb_entry->value_2; - opcode &= ~QLA82XX_DBG_OPCODE_AND ;if (opcode &QLA82XX_DBG_OPCODE_OR ) { read_value |= crb_entry->value_3; opcode &= ~QLA82XX_DBG_OPCODE_OR ;@@ -2249,7 +2246,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,qla8044_rd_reg_indirect (vha, crb_addr, &read_value); read_value |= crb_entry->value_3;qla8044_wr_reg_indirect (vha, crb_addr, read_value); - opcode &= ~QLA82XX_DBG_OPCODE_OR ; }if (opcode &QLA82XX_DBG_OPCODE_POLL ) { poll_time = crb_entry->crb_strd.poll_timeout;@@ -2269,7 +2265,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha, crb_addr, &read_value); } }while (1 ); - opcode &= ~QLA82XX_DBG_OPCODE_POLL ; }if (opcode &QLA82XX_DBG_OPCODE_RDSTATE ) {@@ -2283,7 +2278,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha,qla8044_rd_reg_indirect (vha, addr, &read_value); index = crb_entry->crb_ctrl.state_index_v; tmplt_hdr->saved_state_array[index] = read_value; - opcode &= ~QLA82XX_DBG_OPCODE_RDSTATE ; }if (opcode &QLA82XX_DBG_OPCODE_WRSTATE ) {@@ -2303,7 +2297,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha, }qla8044_wr_reg_indirect (vha, addr, read_value); - opcode &= ~QLA82XX_DBG_OPCODE_WRSTATE ; }if (opcode &QLA82XX_DBG_OPCODE_MDSTATE ) {
drivers/scsi/qla2xxx/qla_nx2.c qla8044_minidump_process_control(struct scsi_qla_host *vha, · line 2314
Quoted 7 lines
drivers/scsi/qla2xxx/qla_nx2.c +0
-1@@ -2316,7 +2309,6 @@ qla8044_minidump_process_control(struct scsi_qla_host *vha, read_value |= crb_entry->value_3; read_value += crb_entry->value_1; tmplt_hdr->saved_state_array[index] = read_value; - opcode &= ~QLA82XX_DBG_OPCODE_MDSTATE ; } crb_addr += crb_entry->crb_strd.addr_stride; }
Reviewed-by Lee Duncan <[email protected]>
PATCH v2 6/6 qla2xxx: Always check the return value of qla24xx_get_isp_stats()
In reply to Bart Van AsscheThis patch fixes the following Coverity warning:
CID 361199 (#1 of 1): Unchecked return value (CHECKED_RETURN)
3. check_return: Calling qla24xx_get_isp_stats without checking return value (as is done elsewhere 4 out of 5 times).
Cc Quinn Tran <[email protected]>
Cc Mike Christie <[email protected]>
Cc Himanshu Madhani <[email protected]>
Cc Daniel Wagner <[email protected]>
Signed-off-by Bart Van Assche <[email protected]>
--- drivers/scsi/qla2xxx/qla_attr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_attr.c +5 -1
@@ -2864,6 +2864,8 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost) vha->qla_stats.jiffies_at_last_reset =get_jiffies_64 ();if (IS_FWI2_CAPABLE (ha)) { +int rval; + stats =dma_alloc_coherent (&ha->pdev->dev,sizeof (*stats), &stats_dma,GFP_KERNEL );if (!stats) {
@@ -2873,7 +2875,9 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost) }/* reset firmware statistics */ -qla24xx_get_isp_stats (base_vha, stats, stats_dma,BIT_0 ); + rval =qla24xx_get_isp_stats (base_vha, stats, stats_dma,BIT_0 ); +ql_log (ql_log_warn, vha,0x70d9 , +"Resetting ISP statistics failed: rval = %d\n" , rval);dma_free_coherent (&ha->pdev->dev,sizeof (*stats), stats, stats_dma);
Re: [PATCH v2 6/6] qla2xxx: Always check the return value of qla24xx_get_isp_stats()
drivers/scsi/qla2xxx/qla_attr.c qla2x00_reset_host_stats(struct Scsi_Host *shost) · line 2880
Quoted 8 lines · Bart Van Assche
drivers/scsi/qla2xxx/qla_attr.c +3
-1@@ -2873,7 +2875,9 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost) }/* reset firmware statistics */ -qla24xx_get_isp_stats (base_vha, stats, stats_dma,BIT_0 ); + rval =qla24xx_get_isp_stats (base_vha, stats, stats_dma,BIT_0 ); +ql_log (ql_log_warn, vha,0x70d9 , +"Resetting ISP statistics failed: rval = %d\n" , rval);
Is there not an 'if' missing?
if (rval) ql_log(...)
Re: [PATCH v2 6/6] qla2xxx: Always check the return value of qla24xx_get_isp_stats()
Quoted 12 lines · Daniel Wagner
drivers/scsi/qla2xxx/qla_attr.c qla2x00_reset_host_stats(struct Scsi_Host *shost) · line 2880
Quoted 8 lines · Bart Van Assche
drivers/scsi/qla2xxx/qla_attr.c +3
-1@@ -2873,7 +2875,9 @@ qla2x00_reset_host_stats(struct Scsi_Host *shost) }/* reset firmware statistics */ -qla24xx_get_isp_stats (base_vha, stats, stats_dma,BIT_0 ); + rval =qla24xx_get_isp_stats (base_vha, stats, stats_dma,BIT_0 ); +ql_log (ql_log_warn, vha,0x70d9 , +"Resetting ISP statistics failed: rval = %d\n" , rval);Is there not an 'if' missing?
Quoted 2 lines
if (rval) ql_log(...)
Right, I will add an if-statement.
Thanks,
Bart.