Hi Ewan,
On 3/18/19, 6:36 AM, "linux-scsi-owner@xxxxxxxxxxxxxxx on behalf of Ewan D. Milne" <linux-scsi-owner@xxxxxxxxxxxxxxx on behalf of emilne@xxxxxxxxxx> wrote:
On Fri, 2019-03-15 at 15:04 -0700, Himanshu Madhani wrote:
> This patch fixes crash due to NULL pointer derefrence because
> CPU pointer is not set and used by driver. Instead, driver is
> passes CPU as tag via ha->isp_ops->{lun_reset|target_reset}
>
...
>
> Fixes: 9cf2bab630765 ("block: kill request ->cpu member")
> Signed-off-by: Himanshu Madhani <hmadhani@xxxxxxxxxxx>
> ---
> drivers/scsi/qla2xxx/qla_os.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 677f82fdf56f..91f576d743fe 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -1517,7 +1517,7 @@ __qla2xxx_eh_generic_reset(char *name, enum nexus_wait_type type,
> goto eh_reset_failed;
> }
> err = 2;
> - if (do_reset(fcport, cmd->device->lun, blk_mq_rq_cpu(cmd->request) + 1)
> + if (do_reset(fcport, cmd->device->lun, 1)
> != QLA_SUCCESS) {
> ql_log(ql_log_warn, vha, 0x800c,
> "do_reset failed for cmd=%p.\n", cmd);
Hi Himanshu-
The 3rd parameter to do_reset() may end up being passed to the HBA in the TM
IOCB in qla2x00_async_tm_cmd(). Can you explain why the CPU number was used
previously and why passing a constant here is sufficient? Was this not needed
in the original implementation or is this a functional change?
-Ewan
In earlier BLK-MQ implementation 3rd field must have some meaning and so driver must be setting that value.
(I could not find history on old implementation). However, with latest updates to BLK-MQ, 3rd parameter is
passed as tag in the driver and it's not being used in driver.
In qla2x00_async_tm_cmd(), 3rd parameter is passed as tag to and its value is saved in tm_iocb->u.tmf.data but
not used in the driver anywhere else, so passing value of 1 is safe here since return value from qla2x00_start_sp()
will override this value.
Thanks,
Himanshu
