Hi Ben,
> On Mar 20, 2018, at 2:36 PM, Ben Hutchings <ben.hutchi...@codethink.co.uk>
> wrote:
>
> qla2x00_init_timer() calls add_timer() on the iocb timeout timer,
> which means the timeout function pointer and any data that the
> function depends on must be initialised beforehand.
>
> Move this initialisation before each call to qla2x00_init_timer(). In
> some cases qla2x00_init_timer() initialises a completion structure
> needed by the timeout function, so move the call to add_timer() after
> that.
>
> Signed-off-by: Ben Hutchings <ben.hutchi...@codethink.co.uk>
> ---
> Changes from v1:
>
> Rebased to remove textual dependency on a patch I haven't yet sent
> (oops).
>
> Ben.
>
> drivers/scsi/qla2xxx/qla_gs.c | 12 +++++++-----
> drivers/scsi/qla2xxx/qla_init.c | 37 +++++++++++++++++++++++--------------
> drivers/scsi/qla2xxx/qla_inline.h | 2 +-
> drivers/scsi/qla2xxx/qla_iocb.c | 8 +++++---
> drivers/scsi/qla2xxx/qla_mbx.c | 8 ++++----
> drivers/scsi/qla2xxx/qla_mid.c | 2 +-
> drivers/scsi/qla2xxx/qla_mr.c | 5 +++--
> drivers/scsi/qla2xxx/qla_target.c | 2 +-
> 8 files changed, 45 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
> index 403fa096f8c8..fcde5ea203c0 100644
> --- a/drivers/scsi/qla2xxx/qla_gs.c
> +++ b/drivers/scsi/qla2xxx/qla_gs.c
> @@ -3796,6 +3796,7 @@ int qla24xx_async_gffid(scsi_qla_host_t *vha, fc_port_t
> *fcport)
> sp->gen1 = fcport->rscn_gen;
> sp->gen2 = fcport->login_gen;
>
> + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
>
> /* CT_IU preamble */
> @@ -3814,7 +3815,6 @@ int qla24xx_async_gffid(scsi_qla_host_t *vha, fc_port_t
> *fcport)
> sp->u.iocb_cmd.u.ctarg.rsp_size = GFF_ID_RSP_SIZE;
> sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS;
>
> - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> sp->done = qla24xx_async_gffid_sp_done;
>
> rval = qla2x00_start_sp(sp);
> @@ -4121,6 +4121,8 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha,
> struct srb *sp,
> sp->name = "gnnft";
> sp->gen1 = vha->hw->base_qpair->chip_reset;
> sp->gen2 = fc4_type;
> +
> + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
>
> memset(sp->u.iocb_cmd.u.ctarg.rsp, 0, sp->u.iocb_cmd.u.ctarg.rsp_size);
> @@ -4137,7 +4139,6 @@ static int qla24xx_async_gnnft(scsi_qla_host_t *vha,
> struct srb *sp,
> sp->u.iocb_cmd.u.ctarg.req_size = GNN_FT_REQ_SIZE;
> sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS;
>
> - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> sp->done = qla2x00_async_gpnft_gnnft_sp_done;
>
> rval = qla2x00_start_sp(sp);
> @@ -4210,6 +4211,8 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8
> fc4_type)
> sp->name = "gpnft";
> sp->gen1 = vha->hw->base_qpair->chip_reset;
> sp->gen2 = fc4_type;
> +
> + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
>
> sp->u.iocb_cmd.u.ctarg.req = dma_zalloc_coherent(&vha->hw->pdev->dev,
> @@ -4248,7 +4251,6 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8
> fc4_type)
> sp->u.iocb_cmd.u.ctarg.rsp_size = rspsz;
> sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS;
>
> - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> sp->done = qla2x00_async_gpnft_gnnft_sp_done;
>
> rval = qla2x00_start_sp(sp);
> @@ -4356,6 +4358,7 @@ int qla24xx_async_gnnid(scsi_qla_host_t *vha, fc_port_t
> *fcport)
> sp->gen1 = fcport->rscn_gen;
> sp->gen2 = fcport->login_gen;
>
> + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
>
> /* CT_IU preamble */
> @@ -4377,7 +4380,6 @@ int qla24xx_async_gnnid(scsi_qla_host_t *vha, fc_port_t
> *fcport)
> sp->u.iocb_cmd.u.ctarg.rsp_size = GNN_ID_RSP_SIZE;
> sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS;
>
> - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> sp->done = qla2x00_async_gnnid_sp_done;
>
> rval = qla2x00_start_sp(sp);
> @@ -4493,6 +4495,7 @@ int qla24xx_async_gfpnid(scsi_qla_host_t *vha,
> fc_port_t *fcport)
> sp->gen1 = fcport->rscn_gen;
> sp->gen2 = fcport->login_gen;
>
> + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
>
> /* CT_IU preamble */
> @@ -4514,7 +4517,6 @@ int qla24xx_async_gfpnid(scsi_qla_host_t *vha,
> fc_port_t *fcport)
> sp->u.iocb_cmd.u.ctarg.rsp_size = GFPN_ID_RSP_SIZE;
> sp->u.iocb_cmd.u.ctarg.nport_handle = NPH_SNS;
>
> - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> sp->done = qla2x00_async_gfpnid_sp_done;
>
> rval = qla2x00_start_sp(sp);
> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index b0aa8cc96f0f..45319119606a 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -183,10 +183,11 @@ qla2x00_async_login(struct scsi_qla_host *vha,
> fc_port_t *fcport,
> sp->name = "login";
> sp->gen1 = fcport->rscn_gen;
> sp->gen2 = fcport->login_gen;
> - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
>
> lio = &sp->u.iocb_cmd;
> lio->timeout = qla2x00_async_iocb_timeout;
> + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
> +
> sp->done = qla2x00_async_login_sp_done;
> lio->u.logio.flags |= SRB_LOGIN_COND_PLOGI;
>
> @@ -245,10 +246,11 @@ qla2x00_async_logout(struct scsi_qla_host *vha,
> fc_port_t *fcport)
>
> sp->type = SRB_LOGOUT_CMD;
> sp->name = "logout";
> - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
>
> lio = &sp->u.iocb_cmd;
> lio->timeout = qla2x00_async_iocb_timeout;
> + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
> +
> sp->done = qla2x00_async_logout_sp_done;
> rval = qla2x00_start_sp(sp);
> if (rval != QLA_SUCCESS)
> @@ -307,10 +309,11 @@ qla2x00_async_prlo(struct scsi_qla_host *vha, fc_port_t
> *fcport)
>
> sp->type = SRB_PRLO_CMD;
> sp->name = "prlo";
> - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
>
> lio = &sp->u.iocb_cmd;
> lio->timeout = qla2x00_async_iocb_timeout;
> + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
> +
> sp->done = qla2x00_async_prlo_sp_done;
> rval = qla2x00_start_sp(sp);
> if (rval != QLA_SUCCESS)
> @@ -412,10 +415,11 @@ qla2x00_async_adisc(struct scsi_qla_host *vha,
> fc_port_t *fcport,
>
> sp->type = SRB_ADISC_CMD;
> sp->name = "adisc";
> - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
>
> lio = &sp->u.iocb_cmd;
> lio->timeout = qla2x00_async_iocb_timeout;
> + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
> +
> sp->done = qla2x00_async_adisc_sp_done;
> if (data[1] & QLA_LOGIO_LOGIN_RETRIED)
> lio->u.logio.flags |= SRB_LOGIN_RETRIED;
> @@ -745,6 +749,8 @@ int qla24xx_async_gnl(struct scsi_qla_host *vha,
> fc_port_t *fcport)
> sp->gen1 = fcport->rscn_gen;
> sp->gen2 = fcport->login_gen;
>
> + mbx = &sp->u.iocb_cmd;
> + mbx->timeout = qla2x00_async_iocb_timeout;
> qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha)+2);
>
> mb = sp->u.iocb_cmd.u.mbx.out_mb;
> @@ -757,9 +763,6 @@ int qla24xx_async_gnl(struct scsi_qla_host *vha,
> fc_port_t *fcport)
> mb[8] = vha->gnl.size;
> mb[9] = vha->vp_idx;
>
> - mbx = &sp->u.iocb_cmd;
> - mbx->timeout = qla2x00_async_iocb_timeout;
> -
> sp->done = qla24xx_async_gnl_sp_done;
>
> rval = qla2x00_start_sp(sp);
> @@ -888,10 +891,11 @@ qla24xx_async_prli(struct scsi_qla_host *vha, fc_port_t
> *fcport)
>
> sp->type = SRB_PRLI_CMD;
> sp->name = "prli";
> - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
>
> lio = &sp->u.iocb_cmd;
> lio->timeout = qla2x00_async_iocb_timeout;
> + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
> +
> sp->done = qla2x00_async_prli_sp_done;
> lio->u.logio.flags = 0;
>
> @@ -956,6 +960,9 @@ int qla24xx_async_gpdb(struct scsi_qla_host *vha,
> fc_port_t *fcport, u8 opt)
> sp->name = "gpdb";
> sp->gen1 = fcport->rscn_gen;
> sp->gen2 = fcport->login_gen;
> +
> + mbx = &sp->u.iocb_cmd;
> + mbx->timeout = qla2x00_async_iocb_timeout;
> qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
>
> pd = dma_pool_zalloc(ha->s_dma_pool, GFP_KERNEL, &pd_dma);
> @@ -975,8 +982,6 @@ int qla24xx_async_gpdb(struct scsi_qla_host *vha,
> fc_port_t *fcport, u8 opt)
> mb[9] = vha->vp_idx;
> mb[10] = opt;
>
> - mbx = &sp->u.iocb_cmd;
> - mbx->timeout = qla2x00_async_iocb_timeout;
> mbx->u.mbx.in = (void *)pd;
> mbx->u.mbx.in_dma = pd_dma;
>
> @@ -1486,13 +1491,15 @@ qla2x00_async_tm_cmd(fc_port_t *fcport, uint32_t
> flags, uint32_t lun,
> tm_iocb = &sp->u.iocb_cmd;
> sp->type = SRB_TM_CMD;
> sp->name = "tmf";
> +
> + tm_iocb->timeout = qla2x00_tmf_iocb_timeout;
> + init_completion(&tm_iocb->u.tmf.comp);
> qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha));
> +
> tm_iocb->u.tmf.flags = flags;
> tm_iocb->u.tmf.lun = lun;
> tm_iocb->u.tmf.data = tag;
> sp->done = qla2x00_tmf_sp_done;
> - tm_iocb->timeout = qla2x00_tmf_iocb_timeout;
> - init_completion(&tm_iocb->u.tmf.comp);
>
> rval = qla2x00_start_sp(sp);
> if (rval != QLA_SUCCESS)
> @@ -1566,7 +1573,11 @@ qla24xx_async_abort_cmd(srb_t *cmd_sp)
> abt_iocb = &sp->u.iocb_cmd;
> sp->type = SRB_ABT_CMD;
> sp->name = "abort";
> +
> + abt_iocb->timeout = qla24xx_abort_iocb_timeout;
> + init_completion(&abt_iocb->u.abt.comp);
> qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha));
> +
> abt_iocb->u.abt.cmd_hndl = cmd_sp->handle;
>
> if (vha->flags.qpairs_available && cmd_sp->qpair)
> @@ -1576,8 +1587,6 @@ qla24xx_async_abort_cmd(srb_t *cmd_sp)
> abt_iocb->u.abt.req_que_no = cpu_to_le16(vha->req->id);
>
> sp->done = qla24xx_abort_sp_done;
> - abt_iocb->timeout = qla24xx_abort_iocb_timeout;
> - init_completion(&abt_iocb->u.abt.comp);
>
> rval = qla2x00_start_sp(sp);
> if (rval != QLA_SUCCESS)
> diff --git a/drivers/scsi/qla2xxx/qla_inline.h
> b/drivers/scsi/qla2xxx/qla_inline.h
> index 4d32426393c7..06c4a843e2ad 100644
> --- a/drivers/scsi/qla2xxx/qla_inline.h
> +++ b/drivers/scsi/qla2xxx/qla_inline.h
> @@ -271,13 +271,13 @@ qla2x00_init_timer(srb_t *sp, unsigned long tmo)
> {
> timer_setup(&sp->u.iocb_cmd.timer, qla2x00_sp_timeout, 0);
> sp->u.iocb_cmd.timer.expires = jiffies + tmo * HZ;
> - add_timer(&sp->u.iocb_cmd.timer);
> sp->free = qla2x00_sp_free;
> init_completion(&sp->comp);
> if (IS_QLAFX00(sp->vha->hw) && (sp->type == SRB_FXIOCB_DCMD))
> init_completion(&sp->u.iocb_cmd.u.fxiocb.fxiocb_comp);
> if (sp->type == SRB_ELS_DCMD)
> init_completion(&sp->u.iocb_cmd.u.els_logo.comp);
> + add_timer(&sp->u.iocb_cmd.timer);
> }
>
> static inline int
> diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
> index 8d00d559bd26..6a719c1f8af5 100644
> --- a/drivers/scsi/qla2xxx/qla_iocb.c
> +++ b/drivers/scsi/qla2xxx/qla_iocb.c
> @@ -2458,8 +2458,8 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int
> els_opcode,
> sp->type = SRB_ELS_DCMD;
> sp->name = "ELS_DCMD";
> sp->fcport = fcport;
> - qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT);
> elsio->timeout = qla2x00_els_dcmd_iocb_timeout;
> + qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT);
> sp->done = qla2x00_els_dcmd_sp_done;
> sp->free = qla2x00_els_dcmd_sp_free;
>
> @@ -2656,8 +2656,11 @@ qla24xx_els_dcmd2_iocb(scsi_qla_host_t *vha, int
> els_opcode,
> sp->type = SRB_ELS_DCMD;
> sp->name = "ELS_DCMD";
> sp->fcport = fcport;
> - qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT);
> +
> elsio->timeout = qla2x00_els_dcmd2_iocb_timeout;
> + init_completion(&elsio->u.els_plogi.comp);
> + qla2x00_init_timer(sp, ELS_DCMD_TIMEOUT);
> +
> sp->done = qla2x00_els_dcmd2_sp_done;
> sp->free = qla2x00_els_dcmd2_sp_free;
>
> @@ -2694,7 +2697,6 @@ qla24xx_els_dcmd2_iocb(scsi_qla_host_t *vha, int
> els_opcode,
> ql_dump_buffer(ql_dbg_io + ql_dbg_buffer, vha, 0x0109,
> (uint8_t *)elsio->u.els_plogi.els_plogi_pyld, 0x70);
>
> - init_completion(&elsio->u.els_plogi.comp);
> rval = qla2x00_start_sp(sp);
> if (rval != QLA_SUCCESS) {
> rval = QLA_FUNCTION_FAILED;
> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
> index 7397aeddd96c..4984570e210b 100644
> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -6006,14 +6006,14 @@ int qla24xx_send_mb_cmd(struct scsi_qla_host *vha,
> mbx_cmd_t *mcp)
> sp->type = SRB_MB_IOCB;
> sp->name = mb_to_str(mcp->mb[0]);
>
> - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
> -
> - memcpy(sp->u.iocb_cmd.u.mbx.out_mb, mcp->mb, SIZEOF_IOCB_MB_REG);
> -
> c = &sp->u.iocb_cmd;
> c->timeout = qla2x00_async_iocb_timeout;
> init_completion(&c->u.mbx.comp);
>
> + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
> +
> + memcpy(sp->u.iocb_cmd.u.mbx.out_mb, mcp->mb, SIZEOF_IOCB_MB_REG);
> +
> sp->done = qla2x00_async_mb_sp_done;
>
> rval = qla2x00_start_sp(sp);
> diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
> index e965b16f21e3..07ed3aaedb4a 100644
> --- a/drivers/scsi/qla2xxx/qla_mid.c
> +++ b/drivers/scsi/qla2xxx/qla_mid.c
> @@ -935,8 +935,8 @@ int qla24xx_control_vp(scsi_qla_host_t *vha, int cmd)
> sp->type = SRB_CTRL_VP;
> sp->name = "ctrl_vp";
> sp->done = qla_ctrlvp_sp_done;
> - qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
> sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> + qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha) + 2);
> sp->u.iocb_cmd.u.ctrlvp.cmd = cmd;
> sp->u.iocb_cmd.u.ctrlvp.vp_index = vp_index;
>
> diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
> index d5da3981cefe..134f2b8a49fe 100644
> --- a/drivers/scsi/qla2xxx/qla_mr.c
> +++ b/drivers/scsi/qla2xxx/qla_mr.c
> @@ -1821,9 +1821,11 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t
> *fcport, uint16_t fx_type)
>
> sp->type = SRB_FXIOCB_DCMD;
> sp->name = "fxdisc";
> - qla2x00_init_timer(sp, FXDISC_TIMEOUT);
>
> fdisc = &sp->u.iocb_cmd;
> + fdisc->timeout = qla2x00_fxdisc_iocb_timeout;
> + qla2x00_init_timer(sp, FXDISC_TIMEOUT);
> +
> switch (fx_type) {
> case FXDISC_GET_CONFIG_INFO:
> fdisc->u.fxiocb.flags =
> @@ -1924,7 +1926,6 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t
> *fcport, uint16_t fx_type)
> goto done_unmap_req;
> }
>
> - fdisc->timeout = qla2x00_fxdisc_iocb_timeout;
> fdisc->u.fxiocb.req_func_type = cpu_to_le16(fx_type);
> sp->done = qla2x00_fxdisc_sp_done;
>
> diff --git a/drivers/scsi/qla2xxx/qla_target.c
> b/drivers/scsi/qla2xxx/qla_target.c
> index b49ac85f3de2..84ca07356da1 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -664,10 +664,10 @@ int qla24xx_async_notify_ack(scsi_qla_host_t *vha,
> fc_port_t *fcport,
> sp->type = type;
> sp->name = "nack";
>
> + sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> qla2x00_init_timer(sp, qla2x00_get_async_timeout(vha)+2);
>
> sp->u.iocb_cmd.u.nack.ntfy = ntfy;
> - sp->u.iocb_cmd.timeout = qla2x00_async_iocb_timeout;
> sp->done = qla2x00_async_nack_sp_done;
>
> rval = qla2x00_start_sp(sp);
> --
> 2.15.0.rc0
>
What motivated this patch? are you debugging any crash which was helped by
moving code around?
What I see from this patch is that its moving iocb.cmd.timeout field before
qla2x00_init_timer(),
which are completely different from each other.
Thanks,
- Himanshu
