[PATCH for-rc 0/2] RDMA/hns: Fix memory leaks

[PATCH for-rc 0/2] RDMA/hns: Fix memory leaks

[Weihang Li]

Fix memory leaks about mr and eq in hip08.

Lijun Ou (2):
  RDMA/hns: Fix memory leaks about mr
  RDMA/hns: Prevent memory leaks about eq

 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 8 +++++---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 6 +++---
 2 files changed, 8 insertions(+), 6 deletions(-)

-- 
2.8.1

[PATCH for-rc 1/2] RDMA/hns: Fix memory leaks about mr

[Weihang Li]

From: Lijun Ou <oulijun@huawei.com>

In hns_roce_v1_dereg_mr(), 'mr_work' is not freed in some cases, for
example, try_wait_for_completion() runs fail, which will cause memory
leaks.

Fixes: bfcc681bd09d ("IB/hns: Fix the bug when free mr")
Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Weihang Li <liweihang@hisilicon.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 5f74bf5..88c1cd9 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -1094,7 +1094,6 @@ static void hns_roce_v1_mr_free_work_fn(struct work_struct *work)
 free_work:
 	if (mr_work->comp_flag)
 		complete(mr_work->comp);
-	kfree(mr_work);
 }
 
 static int hns_roce_v1_dereg_mr(struct hns_roce_dev *hr_dev,
@@ -1137,18 +1136,21 @@ static int hns_roce_v1_dereg_mr(struct hns_roce_dev *hr_dev,
 
 	while (end > 0) {
 		if (try_wait_for_completion(&comp))
-			goto free_mr;
+			goto err;
 		msleep(HNS_ROCE_V1_FREE_MR_WAIT_VALUE);
 		end -= HNS_ROCE_V1_FREE_MR_WAIT_VALUE;
 	}
 
 	mr_work->comp_flag = 0;
 	if (try_wait_for_completion(&comp))
-		goto free_mr;
+		goto err;
 
 	dev_warn(dev, "Free mr work 0x%x over 50s and failed!\n", mr->key);
 	ret = -ETIMEDOUT;
 
+err:
+	kfree(mr_work);
+
 free_mr:
 	dev_dbg(dev, "Free mr 0x%x use 0x%x us.\n",
 		mr->key, jiffies_to_usecs(jiffies) - jiffies_to_usecs(start));
-- 
2.8.1

[PATCH for-rc 2/2] RDMA/hns: Prevent memory leaks about eq

[Weihang Li]

From: Lijun Ou <oulijun@huawei.com>

eq->buf_list->buf and eq->buf_list should also be freed when eqe_hop_num
is set to 0, or there will be memory leaks.

Fixes: a5073d6054f7 ("RDMA/hns: Add eq support of hip08")
Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Weihang Li <liweihang@hisilicon.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 7a89d66..e82567f 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -5389,9 +5389,9 @@ static void hns_roce_v2_free_eq(struct hns_roce_dev *hr_dev,
 		return;
 	}
 
-	if (eq->buf_list)
-		dma_free_coherent(hr_dev->dev, buf_chk_sz,
-				  eq->buf_list->buf, eq->buf_list->map);
+	dma_free_coherent(hr_dev->dev, buf_chk_sz, eq->buf_list->buf,
+			  eq->buf_list->map);
+	kfree(eq->buf_list);
 }
 
 static void hns_roce_config_eqc(struct hns_roce_dev *hr_dev,
-- 
2.8.1

Re: [PATCH for-rc 1/2] RDMA/hns: Fix memory leaks about mr

[Jason Gunthorpe]

On Sat, Oct 26, 2019 at 02:56:34PM +0800, Weihang Li wrote:
> From: Lijun Ou <oulijun@huawei.com>
> 
> In hns_roce_v1_dereg_mr(), 'mr_work' is not freed in some cases, for
> example, try_wait_for_completion() runs fail, which will cause memory
> leaks.
> 
> Fixes: bfcc681bd09d ("IB/hns: Fix the bug when free mr")
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> Signed-off-by: Weihang Li <liweihang@hisilicon.com>
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 5f74bf5..88c1cd9 100644
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -1094,7 +1094,6 @@ static void hns_roce_v1_mr_free_work_fn(struct work_struct *work)
>  free_work:
>  	if (mr_work->comp_flag)
>  		complete(mr_work->comp);
> -	kfree(mr_work);
>  }
>  
>  static int hns_roce_v1_dereg_mr(struct hns_roce_dev *hr_dev,
> @@ -1137,18 +1136,21 @@ static int hns_roce_v1_dereg_mr(struct hns_roce_dev *hr_dev,
>  
>  	while (end > 0) {
>  		if (try_wait_for_completion(&comp))
> -			goto free_mr;
> +			goto err;
>  		msleep(HNS_ROCE_V1_FREE_MR_WAIT_VALUE);
>  		end -= HNS_ROCE_V1_FREE_MR_WAIT_VALUE;
>  	}
>  
>  	mr_work->comp_flag = 0;
>  	if (try_wait_for_completion(&comp))
> -		goto free_mr;
> +		goto err;
>  
>  	dev_warn(dev, "Free mr work 0x%x over 50s and failed!\n", mr->key);
>  	ret = -ETIMEDOUT;
>  
> +err:
> +	kfree(mr_work);

This whole thing makes absolutely no sense.

Why is work being pushed onto a WQ and then the same routine turns
around and does wait_for_completion??

Further, trying to make this 'non blocking' by sleep spinning on
'try_wait_for_completion' is utterly insane.

Then going on to uncondionally free memory that we don't even know if
the work is finished with or not is just wrong.

Doug, you took this spin loop stuff, you get to review the fixes for
it! :)

Jason

Re: [PATCH for-rc 2/2] RDMA/hns: Prevent memory leaks about eq

[Jason Gunthorpe]

On Sat, Oct 26, 2019 at 02:56:35PM +0800, Weihang Li wrote:
> From: Lijun Ou <oulijun@huawei.com>
> 
> eq->buf_list->buf and eq->buf_list should also be freed when eqe_hop_num
> is set to 0, or there will be memory leaks.
> 
> Fixes: a5073d6054f7 ("RDMA/hns: Add eq support of hip08")
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> Signed-off-by: Weihang Li <liweihang@hisilicon.com>
> ---
>  drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Applies to for-rc

Thanks,
Jason