[PATCH v2 1/2] RDMA/rw: fix error flow during RDMA context initialization

[PATCH v2 1/2] RDMA/rw: fix error flow during RDMA context initialization

[Max Gurtovoy]

In case the SGL was mapped for P2P DMA operation, we must unmap it using
pci_p2pdma_unmap_sg.

Fixes: 7f73eac3a713 ("PCI/P2PDMA: Introduce pci_p2pdma_unmap_sg()")
Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
---
 drivers/infiniband/core/rw.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c
index 4fad732f9b3c..69513b484507 100644
--- a/drivers/infiniband/core/rw.c
+++ b/drivers/infiniband/core/rw.c
@@ -273,6 +273,24 @@ static int rdma_rw_init_single_wr(struct rdma_rw_ctx *ctx, struct ib_qp *qp,
 	return 1;
 }
 
+static void rdma_rw_unmap_sg(struct ib_device *dev, struct scatterlist *sg,
+		u32 sg_cnt, enum dma_data_direction dir)
+{
+	if (is_pci_p2pdma_page(sg_page(sg)))
+		pci_p2pdma_unmap_sg(dev->dma_device, sg, sg_cnt, dir);
+	else
+		ib_dma_unmap_sg(dev, sg, sg_cnt, dir);
+}
+
+static int rdma_rw_map_sg(struct ib_device *dev, struct scatterlist *sg,
+		u32 sg_cnt, enum dma_data_direction dir)
+{
+	if (is_pci_p2pdma_page(sg_page(sg)))
+		return pci_p2pdma_map_sg(dev->dma_device, sg, sg_cnt, dir);
+	else
+		return ib_dma_map_sg(dev, sg, sg_cnt, dir);
+}
+
 /**
  * rdma_rw_ctx_init - initialize a RDMA READ/WRITE context
  * @ctx:	context to initialize
@@ -295,11 +313,7 @@ int rdma_rw_ctx_init(struct rdma_rw_ctx *ctx, struct ib_qp *qp, u8 port_num,
 	struct ib_device *dev = qp->pd->device;
 	int ret;
 
-	if (is_pci_p2pdma_page(sg_page(sg)))
-		ret = pci_p2pdma_map_sg(dev->dma_device, sg, sg_cnt, dir);
-	else
-		ret = ib_dma_map_sg(dev, sg, sg_cnt, dir);
-
+	ret = rdma_rw_map_sg(dev, sg, sg_cnt, dir);
 	if (!ret)
 		return -ENOMEM;
 	sg_cnt = ret;
@@ -338,7 +352,7 @@ int rdma_rw_ctx_init(struct rdma_rw_ctx *ctx, struct ib_qp *qp, u8 port_num,
 	return ret;
 
 out_unmap_sg:
-	ib_dma_unmap_sg(dev, sg, sg_cnt, dir);
+	rdma_rw_unmap_sg(dev, sg, sg_cnt, dir);
 	return ret;
 }
 EXPORT_SYMBOL(rdma_rw_ctx_init);
@@ -588,11 +602,7 @@ void rdma_rw_ctx_destroy(struct rdma_rw_ctx *ctx, struct ib_qp *qp, u8 port_num,
 		break;
 	}
 
-	if (is_pci_p2pdma_page(sg_page(sg)))
-		pci_p2pdma_unmap_sg(qp->pd->device->dma_device, sg,
-				    sg_cnt, dir);
-	else
-		ib_dma_unmap_sg(qp->pd->device, sg, sg_cnt, dir);
+	rdma_rw_unmap_sg(qp->pd->device, sg, sg_cnt, dir);
 }
 EXPORT_SYMBOL(rdma_rw_ctx_destroy);
 
-- 
2.20.1

[PATCH v2 2/2] RDMA/rw: map P2P memory correctly for signature operations

[Max Gurtovoy]

Since RDMA rw API support operations with P2P memory sg list, make sure
to map/unmap the scatter list for signature operation correctly.

Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
---
 drivers/infiniband/core/rw.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c
index 69513b484507..6eba8453f206 100644
--- a/drivers/infiniband/core/rw.c
+++ b/drivers/infiniband/core/rw.c
@@ -392,13 +392,13 @@ int rdma_rw_ctx_signature_init(struct rdma_rw_ctx *ctx, struct ib_qp *qp,
 		return -EINVAL;
 	}
 
-	ret = ib_dma_map_sg(dev, sg, sg_cnt, dir);
+	ret = rdma_rw_map_sg(dev, sg, sg_cnt, dir);
 	if (!ret)
 		return -ENOMEM;
 	sg_cnt = ret;
 
 	if (prot_sg_cnt) {
-		ret = ib_dma_map_sg(dev, prot_sg, prot_sg_cnt, dir);
+		ret = rdma_rw_map_sg(dev, prot_sg, prot_sg_cnt, dir);
 		if (!ret) {
 			ret = -ENOMEM;
 			goto out_unmap_sg;
@@ -467,9 +467,9 @@ int rdma_rw_ctx_signature_init(struct rdma_rw_ctx *ctx, struct ib_qp *qp,
 	kfree(ctx->reg);
 out_unmap_prot_sg:
 	if (prot_sg_cnt)
-		ib_dma_unmap_sg(dev, prot_sg, prot_sg_cnt, dir);
+		rdma_rw_unmap_sg(dev, prot_sg, prot_sg_cnt, dir);
 out_unmap_sg:
-	ib_dma_unmap_sg(dev, sg, sg_cnt, dir);
+	rdma_rw_unmap_sg(dev, sg, sg_cnt, dir);
 	return ret;
 }
 EXPORT_SYMBOL(rdma_rw_ctx_signature_init);
@@ -629,9 +629,9 @@ void rdma_rw_ctx_destroy_signature(struct rdma_rw_ctx *ctx, struct ib_qp *qp,
 	ib_mr_pool_put(qp, &qp->sig_mrs, ctx->reg->mr);
 	kfree(ctx->reg);
 
-	ib_dma_unmap_sg(qp->pd->device, sg, sg_cnt, dir);
 	if (prot_sg_cnt)
-		ib_dma_unmap_sg(qp->pd->device, prot_sg, prot_sg_cnt, dir);
+		rdma_rw_unmap_sg(qp->pd->device, prot_sg, prot_sg_cnt, dir);
+	rdma_rw_unmap_sg(qp->pd->device, sg, sg_cnt, dir);
 }
 EXPORT_SYMBOL(rdma_rw_ctx_destroy_signature);
 
-- 
2.20.1

Re: [PATCH v2 1/2] RDMA/rw: fix error flow during RDMA context initialization

[Leon Romanovsky]

On Thu, Feb 20, 2020 at 12:08:18PM +0200, Max Gurtovoy wrote:
> In case the SGL was mapped for P2P DMA operation, we must unmap it using
> pci_p2pdma_unmap_sg.
>
> Fixes: 7f73eac3a713 ("PCI/P2PDMA: Introduce pci_p2pdma_unmap_sg()")
> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> ---
>  drivers/infiniband/core/rw.c | 32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c
> index 4fad732f9b3c..69513b484507 100644
> --- a/drivers/infiniband/core/rw.c
> +++ b/drivers/infiniband/core/rw.c
> @@ -273,6 +273,24 @@ static int rdma_rw_init_single_wr(struct rdma_rw_ctx *ctx, struct ib_qp *qp,
>  	return 1;
>  }
>
> +static void rdma_rw_unmap_sg(struct ib_device *dev, struct scatterlist *sg,
> +		u32 sg_cnt, enum dma_data_direction dir)
> +{
> +	if (is_pci_p2pdma_page(sg_page(sg)))
> +		pci_p2pdma_unmap_sg(dev->dma_device, sg, sg_cnt, dir);
> +	else
> +		ib_dma_unmap_sg(dev, sg, sg_cnt, dir);
> +}
> +
> +static int rdma_rw_map_sg(struct ib_device *dev, struct scatterlist *sg,
> +		u32 sg_cnt, enum dma_data_direction dir)
> +{
> +	if (is_pci_p2pdma_page(sg_page(sg)))
> +		return pci_p2pdma_map_sg(dev->dma_device, sg, sg_cnt, dir);
> +	else

This "else" is not needed.

> +		return ib_dma_map_sg(dev, sg, sg_cnt, dir);
> +}> +
>  /**

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

Re: [PATCH v2 1/2] RDMA/rw: fix error flow during RDMA context initialization

[Logan Gunthorpe]

On 2020-02-20 3:08 a.m., Max Gurtovoy wrote:
> In case the SGL was mapped for P2P DMA operation, we must unmap it using
> pci_p2pdma_unmap_sg.
> 
> Fixes: 7f73eac3a713 ("PCI/P2PDMA: Introduce pci_p2pdma_unmap_sg()")
> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>

Makes sense to me.

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Thanks!

Logan

Re: [PATCH v2 2/2] RDMA/rw: map P2P memory correctly for signature operations

[Logan Gunthorpe]

On 2020-02-20 3:08 a.m., Max Gurtovoy wrote:
> Since RDMA rw API support operations with P2P memory sg list, make sure
> to map/unmap the scatter list for signature operation correctly.

Does anyone actually use P2P pages with rdma_rw_ctx_signature_init()?

Logan

> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> ---
>  drivers/infiniband/core/rw.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c
> index 69513b484507..6eba8453f206 100644
> --- a/drivers/infiniband/core/rw.c
> +++ b/drivers/infiniband/core/rw.c
> @@ -392,13 +392,13 @@ int rdma_rw_ctx_signature_init(struct rdma_rw_ctx *ctx, struct ib_qp *qp,
>  		return -EINVAL;
>  	}
>  
> -	ret = ib_dma_map_sg(dev, sg, sg_cnt, dir);
> +	ret = rdma_rw_map_sg(dev, sg, sg_cnt, dir);
>  	if (!ret)
>  		return -ENOMEM;
>  	sg_cnt = ret;
>  
>  	if (prot_sg_cnt) {
> -		ret = ib_dma_map_sg(dev, prot_sg, prot_sg_cnt, dir);
> +		ret = rdma_rw_map_sg(dev, prot_sg, prot_sg_cnt, dir);
>  		if (!ret) {
>  			ret = -ENOMEM;
>  			goto out_unmap_sg;
> @@ -467,9 +467,9 @@ int rdma_rw_ctx_signature_init(struct rdma_rw_ctx *ctx, struct ib_qp *qp,
>  	kfree(ctx->reg);
>  out_unmap_prot_sg:
>  	if (prot_sg_cnt)
> -		ib_dma_unmap_sg(dev, prot_sg, prot_sg_cnt, dir);
> +		rdma_rw_unmap_sg(dev, prot_sg, prot_sg_cnt, dir);
>  out_unmap_sg:
> -	ib_dma_unmap_sg(dev, sg, sg_cnt, dir);
> +	rdma_rw_unmap_sg(dev, sg, sg_cnt, dir);
>  	return ret;
>  }
>  EXPORT_SYMBOL(rdma_rw_ctx_signature_init);
> @@ -629,9 +629,9 @@ void rdma_rw_ctx_destroy_signature(struct rdma_rw_ctx *ctx, struct ib_qp *qp,
>  	ib_mr_pool_put(qp, &qp->sig_mrs, ctx->reg->mr);
>  	kfree(ctx->reg);
>  
> -	ib_dma_unmap_sg(qp->pd->device, sg, sg_cnt, dir);
>  	if (prot_sg_cnt)
> -		ib_dma_unmap_sg(qp->pd->device, prot_sg, prot_sg_cnt, dir);
> +		rdma_rw_unmap_sg(qp->pd->device, prot_sg, prot_sg_cnt, dir);
> +	rdma_rw_unmap_sg(qp->pd->device, sg, sg_cnt, dir);
>  }
>  EXPORT_SYMBOL(rdma_rw_ctx_destroy_signature);
>  
> 

Re: [PATCH v2 2/2] RDMA/rw: map P2P memory correctly for signature operations

[Max Gurtovoy]

On 2/20/2020 6:51 PM, Logan Gunthorpe wrote:
>
> On 2020-02-20 3:08 a.m., Max Gurtovoy wrote:
>> Since RDMA rw API support operations with P2P memory sg list, make sure
>> to map/unmap the scatter list for signature operation correctly.
> Does anyone actually use P2P pages with rdma_rw_ctx_signature_init()?

We're adding support for NVMf/target in these days.

Nevertheless, the RW API defined as an API that might use P2P pages.

>
> Logan
>
>> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
>> ---
>>   drivers/infiniband/core/rw.c | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c
>> index 69513b484507..6eba8453f206 100644
>> --- a/drivers/infiniband/core/rw.c
>> +++ b/drivers/infiniband/core/rw.c
>> @@ -392,13 +392,13 @@ int rdma_rw_ctx_signature_init(struct rdma_rw_ctx *ctx, struct ib_qp *qp,
>>   		return -EINVAL;
>>   	}
>>   
>> -	ret = ib_dma_map_sg(dev, sg, sg_cnt, dir);
>> +	ret = rdma_rw_map_sg(dev, sg, sg_cnt, dir);
>>   	if (!ret)
>>   		return -ENOMEM;
>>   	sg_cnt = ret;
>>   
>>   	if (prot_sg_cnt) {
>> -		ret = ib_dma_map_sg(dev, prot_sg, prot_sg_cnt, dir);
>> +		ret = rdma_rw_map_sg(dev, prot_sg, prot_sg_cnt, dir);
>>   		if (!ret) {
>>   			ret = -ENOMEM;
>>   			goto out_unmap_sg;
>> @@ -467,9 +467,9 @@ int rdma_rw_ctx_signature_init(struct rdma_rw_ctx *ctx, struct ib_qp *qp,
>>   	kfree(ctx->reg);
>>   out_unmap_prot_sg:
>>   	if (prot_sg_cnt)
>> -		ib_dma_unmap_sg(dev, prot_sg, prot_sg_cnt, dir);
>> +		rdma_rw_unmap_sg(dev, prot_sg, prot_sg_cnt, dir);
>>   out_unmap_sg:
>> -	ib_dma_unmap_sg(dev, sg, sg_cnt, dir);
>> +	rdma_rw_unmap_sg(dev, sg, sg_cnt, dir);
>>   	return ret;
>>   }
>>   EXPORT_SYMBOL(rdma_rw_ctx_signature_init);
>> @@ -629,9 +629,9 @@ void rdma_rw_ctx_destroy_signature(struct rdma_rw_ctx *ctx, struct ib_qp *qp,
>>   	ib_mr_pool_put(qp, &qp->sig_mrs, ctx->reg->mr);
>>   	kfree(ctx->reg);
>>   
>> -	ib_dma_unmap_sg(qp->pd->device, sg, sg_cnt, dir);
>>   	if (prot_sg_cnt)
>> -		ib_dma_unmap_sg(qp->pd->device, prot_sg, prot_sg_cnt, dir);
>> +		rdma_rw_unmap_sg(qp->pd->device, prot_sg, prot_sg_cnt, dir);
>> +	rdma_rw_unmap_sg(qp->pd->device, sg, sg_cnt, dir);
>>   }
>>   EXPORT_SYMBOL(rdma_rw_ctx_destroy_signature);
>>   
>>

Re: [PATCH v2 1/2] RDMA/rw: fix error flow during RDMA context initialization

[Jason Gunthorpe]

On Thu, Feb 20, 2020 at 12:08:18PM +0200, Max Gurtovoy wrote:
> In case the SGL was mapped for P2P DMA operation, we must unmap it using
> pci_p2pdma_unmap_sg.
> 
> Fixes: 7f73eac3a713 ("PCI/P2PDMA: Introduce pci_p2pdma_unmap_sg()")
> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  drivers/infiniband/core/rw.c | 32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)

Applied to for-rc. The other one can go to -next in a bit as there is
no current bug

Jason

Re: [PATCH v2 2/2] RDMA/rw: map P2P memory correctly for signature operations

[Jason Gunthorpe]

On Thu, Feb 20, 2020 at 12:08:19PM +0200, Max Gurtovoy wrote:
> Since RDMA rw API support operations with P2P memory sg list, make sure
> to map/unmap the scatter list for signature operation correctly.
> 
> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> ---
>  drivers/infiniband/core/rw.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Applied to for-next

Thanks,
Jason