Re: [PATCH] scsi: fix dma_unmap_sg() parameter in some drivers

[John Garry <john.garry@xxxxxxxxxx>]

On 21/12/2017 08:15, chenxiang wrote:
> For function dma_unmap_sg(), the <nents> parameter should be number of
> elements in the scatterlist prior to the mapping, not after the mapping.
> So fix this usage in ibmvscsi_tgt/isci/mvsas/pm8001.

Hi chenxiang,

I think that it may be better to rewrite the series with a patch per 
driver, so that we can add specific "fixes" ID tag. Please also include 
"CC:" tag per patch, cc'ing the respective driver maintainer (if any).

BTW, I guess that there are many more instances of this issue throughout 
the kernel...

Much appreciated,
John

> Signed-off-by: Xiang Chen <chenxiang66@xxxxxxxxxxxxx>
> ---
>  drivers/scsi/ibmvscsi_tgt/libsrp.c | 6 ++++--
>  drivers/scsi/isci/request.c        | 2 +-
>  drivers/scsi/mvsas/mv_sas.c        | 4 ++--
>  drivers/scsi/pm8001/pm8001_sas.c   | 2 +-
>  4 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/ibmvscsi_tgt/libsrp.c b/drivers/scsi/ibmvscsi_tgt/libsrp.c
> index 5a4cc28..16054c0 100644
> --- a/drivers/scsi/ibmvscsi_tgt/libsrp.c
> +++ b/drivers/scsi/ibmvscsi_tgt/libsrp.c
> @@ -193,7 +193,8 @@ static int srp_direct_data(struct ibmvscsis_cmd *cmd, struct srp_direct_buf *md,
>  	err = rdma_io(cmd, sg, nsg, md, 1, dir, len);
>
>  	if (dma_map)
> -		dma_unmap_sg(iue->target->dev, sg, nsg, DMA_BIDIRECTIONAL);
> +		dma_unmap_sg(iue->target->dev, sg, cmd->se_cmd.t_data_nents,
> +				DMA_BIDIRECTIONAL);
>
>  	return err;
>  }
> @@ -265,7 +266,8 @@ static int srp_indirect_data(struct ibmvscsis_cmd *cmd, struct srp_cmd *srp_cmd,
>  	err = rdma_io(cmd, sg, nsg, md, nmd, dir, len);
>
>  	if (dma_map)
> -		dma_unmap_sg(iue->target->dev, sg, nsg, DMA_BIDIRECTIONAL);
> +		dma_unmap_sg(iue->target->dev, sg, cmd->se_cmd.t_data_nents,
> +				DMA_BIDIRECTIONAL);
>
>  free_mem:
>  	if (token && dma_map) {
> diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
> index ed197bc..225d947 100644
> --- a/drivers/scsi/isci/request.c
> +++ b/drivers/scsi/isci/request.c
> @@ -2914,7 +2914,7 @@ static void isci_request_io_request_complete(struct isci_host *ihost,
>  					 task->total_xfer_len, task->data_dir);
>  		else  /* unmap the sgl dma addresses */
>  			dma_unmap_sg(&ihost->pdev->dev, task->scatter,
> -				     request->num_sg_entries, task->data_dir);
> +				     task->num_scatter, task->data_dir);
>  		break;
>  	case SAS_PROTOCOL_SMP: {
>  		struct scatterlist *sg = &task->smp_task.smp_req;
> diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
> index cff43bd..4b2cf36 100644
> --- a/drivers/scsi/mvsas/mv_sas.c
> +++ b/drivers/scsi/mvsas/mv_sas.c
> @@ -848,7 +848,7 @@ static int mvs_task_prep(struct sas_task *task, struct mvs_info *mvi, int is_tmf
>  	dev_printk(KERN_ERR, mvi->dev, "mvsas prep failed[%d]!\n", rc);
>  	if (!sas_protocol_ata(task->task_proto))
>  		if (n_elem)
> -			dma_unmap_sg(mvi->dev, task->scatter, n_elem,
> +			dma_unmap_sg(mvi->dev, task->scatter, task->num_scatter,
>  				     task->data_dir);
>  prep_out:
>  	return rc;
> @@ -899,7 +899,7 @@ static void mvs_slot_task_free(struct mvs_info *mvi, struct sas_task *task,
>  	if (!sas_protocol_ata(task->task_proto))
>  		if (slot->n_elem)
>  			dma_unmap_sg(mvi->dev, task->scatter,
> -				     slot->n_elem, task->data_dir);
> +				     task->num_scatter, task->data_dir);
>
>  	switch (task->task_proto) {
>  	case SAS_PROTOCOL_SMP:
> diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
> index 947d601..576a0f0 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.c
> +++ b/drivers/scsi/pm8001/pm8001_sas.c
> @@ -466,7 +466,7 @@ static int pm8001_task_exec(struct sas_task *task,
>  	dev_printk(KERN_ERR, pm8001_ha->dev, "pm8001 exec failed[%d]!\n", rc);
>  	if (!sas_protocol_ata(t->task_proto))
>  		if (n_elem)
> -			dma_unmap_sg(pm8001_ha->dev, t->scatter, n_elem,
> +			dma_unmap_sg(pm8001_ha->dev, t->scatter, t->num_scatter,
>  				t->data_dir);
>  out_done:
>  	spin_unlock_irqrestore(&pm8001_ha->lock, flags);