[PATCH v2 0/4] fix dma_unmap_sg() parameter in some scsi drivers

[PATCH v2 0/4] fix dma_unmap_sg() parameter in some scsi drivers

[chenxiang]

According to Documentation/DMA-API.txt, all the parameters of dma_unmap_sg()
must be the same as those and passed in to the scatter/gather mapping API.
But in scsi drivers such as ibmscsi_tgt/iscsi/mvsas/pm8001, the <nents>
parameter of dma_unmap_sg() is number of elements after mapping. So fix them.

Part of the document is as follows:

	void
	dma_unmap_sg(struct device *dev, struct scatterlist *sg,
			int nents, enum dma_data_direction direction)
				
Unmap the previously mapped scatter/gather list.  All the parameters
must be the same as those and passed in to the scatter/gather mapping
API.
				
Note: <nents> must be the number you passed in, *not* the number of
	DMA address entries returned.

Chang Log:
v1 -> v2:
	Split the patch into small patchset, and one patch per driver;

chenxiang (4):
  scsi: ibmvscsis: fix dma_unmap_sg() parameter
  scsi: isci: fix dma_unmap_sg() parameter
  scsi: mvsas: fix dma_unmap_sg() parameter
  scsi: pm8001: fix dma_unmap_sg() parameter

 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(-)

-- 
1.9.1

[PATCH v2 1/4] scsi: ibmvscsis: fix dma_unmap_sg() parameter

[chenxiang]

For function dma_unmap_sg(), the <nents> parameter should be number of
elements in the scatterlist prior to the mapping, not after the mapping.
Fix this usage.

Cc: Bryant G. Ly <bryantly@linux.vnet.ibm.com>
Cc: Michael Cyr <mikecyr@linux.vnet.ibm.com>
Fixes: 88a678bb("ibmscsis: Initial commit of IBM VSCSI Tgt Driver")
Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
---
 drivers/scsi/ibmvscsi_tgt/libsrp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 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) {
-- 
1.9.1

[PATCH v2 2/4] scsi: isci: fix dma_unmap_sg() parameter

[chenxiang]

For function dma_unmap_sg(), the <nents> parameter should be number of
elements in the scatterlist prior to the mapping, not after the mapping.
Fix this usage.

Cc: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Fixes: d9dcb4ba7("isci: unify isci_host and scic_sds_controller")
Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
---
 drivers/scsi/isci/request.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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;
-- 
1.9.1

[PATCH v2 3/4] scsi: mvsas: fix dma_unmap_sg() parameter

[chenxiang]

For function dma_unmap_sg(), the <nents> parameter should be number of
elements in the scatterlist prior to the mapping, not after the mapping.
Fix this usage.

Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Jeff Garzik <jgarzik@pobox.com>
Fixes: 0b15fb1fd("[SCSI]mvsas: add support for Task collector mode and fixed relative
bugs")
Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
---
 drivers/scsi/mvsas/mv_sas.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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:
-- 
1.9.1

[PATCH v2 4/4] scsi: pm8001: fix dma_unmap_sg() parameter

[chenxiang]

For function dma_unmap_sg(), the <nents> parameter should be number of
elements in the scatterlist prior to the mapping, not after the mapping.
Fix this usage.

Cc: Jack Wang <jack_wang@usish.com>
Cc: lindar_liu@usish.com
Fixes: dbf9bfe6("[SCSI]pm8001: add SAS/SATA/HBA driver")
Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
---
 drivers/scsi/pm8001/pm8001_sas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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);
-- 
1.9.1

Re: [PATCH v2 4/4] scsi: pm8001: fix dma_unmap_sg() parameter

[chenxiang (M)]

+cc Jack Wang <xjtuwjp@gmail.com>

在 2018/1/4 10:36, chenxiang 写道:
> For function dma_unmap_sg(), the <nents> parameter should be number of
> elements in the scatterlist prior to the mapping, not after the mapping.
> Fix this usage.
>
> Cc: Jack Wang <jack_wang@usish.com>
> Cc: lindar_liu@usish.com
> Fixes: dbf9bfe6("[SCSI]pm8001: add SAS/SATA/HBA driver")
> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> ---
>   drivers/scsi/pm8001/pm8001_sas.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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);

Re: [PATCH v2 0/4] fix dma_unmap_sg() parameter in some scsi drivers

[chenxiang (M)]

Hi, does anyone notice and review this issue?

在 2018/1/4 10:36, chenxiang 写道:
> According to Documentation/DMA-API.txt, all the parameters of dma_unmap_sg()
> must be the same as those and passed in to the scatter/gather mapping API.
> But in scsi drivers such as ibmscsi_tgt/iscsi/mvsas/pm8001, the <nents>
> parameter of dma_unmap_sg() is number of elements after mapping. So fix them.
>
> Part of the document is as follows:
>
> 	void
> 	dma_unmap_sg(struct device *dev, struct scatterlist *sg,
> 			int nents, enum dma_data_direction direction)
> 				
> Unmap the previously mapped scatter/gather list.  All the parameters
> must be the same as those and passed in to the scatter/gather mapping
> API.
> 				
> Note: <nents> must be the number you passed in, *not* the number of
> 	DMA address entries returned.
>
> Chang Log:
> v1 -> v2:
> 	Split the patch into small patchset, and one patch per driver;
>
> chenxiang (4):
>    scsi: ibmvscsis: fix dma_unmap_sg() parameter
>    scsi: isci: fix dma_unmap_sg() parameter
>    scsi: mvsas: fix dma_unmap_sg() parameter
>    scsi: pm8001: fix dma_unmap_sg() parameter
>
>   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(-)
>

Re: [PATCH v2 0/4] fix dma_unmap_sg() parameter in some scsi drivers

[Martin K. Petersen]

Chenxiang,

> does anyone notice and review this issue?

It's a pretty crappy interface that has to have explanatory "notes"
throughout the documentation saying "Don't use this the obvious way, do
this instead". As evidenced by 5 drivers messing it up.

Your change looks good to me. But before I apply I'd like somebody from
the IBM camp to verify that fixing this doesn't unintentionally break
stuff on power.

-- 
Martin K. Petersen	Oracle Linux Engineering