On 8/13/19 10:31 PM, Martin Wilck wrote:
> From: Martin Wilck <mwilck@xxxxxxxx>
>
> Reset ha->rce, ha->eft and the respective dma fields if
> the buffers aren't mapped for some reason. Also, treat
> both failure cases (allocation and initialization failure)
> equally. The next patch modifies the failure behavior
> slightly again.
>
> Fixes: ad0a0b01f088 "scsi: qla2xxx: Fix Firmware dump size for Extended
> login and Exchange Offload"
> Fixes: a28d9e4ef997 "scsi: qla2xxx: Add support for multiple fwdump
> templates/segments"
> Cc: Joe Carnuccio <joe.carnuccio@xxxxxxxxxx>
> Cc: Quinn Tran <qutran@xxxxxxxxxxx>
> Cc: Himanshu Madhani <hmadhani@xxxxxxxxxxx>
> Cc: Bart Van Assche <bvanassche@xxxxxxx>
> Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> ---
> drivers/scsi/qla2xxx/qla_init.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index 6dd68be..ca9c3f3 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -3063,6 +3063,8 @@ qla2x00_alloc_offload_mem(scsi_qla_host_t *vha)
> ql_log(ql_log_warn, vha, 0x00be,
> "Unable to allocate (%d KB) for FCE.\n",
> FCE_SIZE / 1024);
> + ha->fce_dma = 0;
> + ha->fce = NULL;
> goto try_eft;
> }
>
Actually, I would set this earlier here:
if (ha->fce)
dma_free_coherent(&ha->pdev->dev,
FCE_SIZE, ha->fce, ha->fce_dma);
which is the logical place to clear 'ha->fce' and 'ha->fce_dma' IMO.
Alsoe there is this call later on:
rval = qla2x00_enable_fce_trace(vha, tc_dma, FCE_NUM_BUFFERS,
ha->fce_mb, &ha->fce_bufs);
if (rval) {
ql_log(ql_log_warn, vha, 0x00bf,
"Unable to initialize FCE (%d).\n", rval);
dma_free_coherent(&ha->pdev->dev, FCE_SIZE, tc,
tc_dma);
ha->flags.fce_enabled = 0;
goto try_eft;
}
which also needs to be protected.
> @@ -3111,9 +3113,12 @@ qla2x00_alloc_offload_mem(scsi_qla_host_t *vha)
>
> ha->eft_dma = tc_dma;
> ha->eft = tc;
> + return;
> }
>
> eft_err:
> + ha->eft = NULL;
> + ha->eft_dma = 0;
> return;
> }
>
I wonder why this is even there.
Right at the start we have:
if (ha->eft) {
ql_dbg(ql_dbg_init, vha, 0x00bd,
"%s: Offload Mem is already allocated.\n",
__func__);
return;
}
IE the second half of this function really should be unreachable code.
Himanshu?
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
