[PATCH 0/2] Two fixes for this merge window

[PATCH 0/2] Two fixes for this merge window

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Hi,

We are in merge window so I didn't know if I should put -next or -rc,
but they are better to go this cycle.

Thanks

Danit Goldberg (2):
  IB/mlx5: Free page from the beginning of the allocation
  IB/mlx5: Free mpi in mp_slave mode

 drivers/infiniband/hw/mlx5/main.c | 1 +
 drivers/infiniband/hw/mlx5/odp.c  | 9 +++++----
 2 files changed, 6 insertions(+), 4 deletions(-)

--
2.20.1

[PATCH 1/2] IB/mlx5: Free page from the beginning of the allocation

[Leon Romanovsky]

From: Danit Goldberg <danitg@mellanox.com>

After the allocation of a page for wqe pointer, the pointer is
shifted. Therefore it is necessary to keep the original address, and
free it at the end.

Fixes: 0f51427bd097 ("RDMA/mlx5: Cleanup WQE page fault handler")
Signed-off-by: Danit Goldberg <danitg@mellanox.com>
Reviewed-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx5/odp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/odp.c b/drivers/infiniband/hw/mlx5/odp.c
index 0a59912a4cef..416e750808eb 100644
--- a/drivers/infiniband/hw/mlx5/odp.c
+++ b/drivers/infiniband/hw/mlx5/odp.c
@@ -1205,7 +1205,7 @@ static void mlx5_ib_mr_wqe_pfault_handler(struct mlx5_ib_dev *dev,
 {
 	bool sq = pfault->type & MLX5_PFAULT_REQUESTOR;
 	u16 wqe_index = pfault->wqe.wqe_index;
-	void *wqe = NULL, *wqe_end = NULL;
+	void *wqe, *wqe_start = NULL, *wqe_end = NULL;
 	u32 bytes_mapped, total_wqe_bytes;
 	struct mlx5_core_rsc_common *res;
 	int resume_with_error = 1;
@@ -1226,12 +1226,13 @@ static void mlx5_ib_mr_wqe_pfault_handler(struct mlx5_ib_dev *dev,
 		goto resolve_page_fault;
 	}

-	wqe = (void *)__get_free_page(GFP_KERNEL);
-	if (!wqe) {
+	wqe_start = (void *)__get_free_page(GFP_KERNEL);
+	if (!wqe_start) {
 		mlx5_ib_err(dev, "Error allocating memory for IO page fault handling.\n");
 		goto resolve_page_fault;
 	}

+	wqe = wqe_start;
 	qp = (res->res == MLX5_RES_QP) ? res_to_qp(res) : NULL;
 	if (qp && sq) {
 		ret = mlx5_ib_read_user_wqe_sq(qp, wqe_index, wqe, PAGE_SIZE,
@@ -1286,7 +1287,7 @@ static void mlx5_ib_mr_wqe_pfault_handler(struct mlx5_ib_dev *dev,
 		    pfault->wqe.wq_num, resume_with_error,
 		    pfault->type);
 	mlx5_core_res_put(res);
-	free_page((unsigned long)wqe);
+	free_page((unsigned long)wqe_start);
 }

 static int pages_in_range(u64 address, u32 length)
--
2.20.1

[PATCH 2/2] IB/mlx5: Free mpi in mp_slave mode

[Leon Romanovsky]

From: Danit Goldberg <danitg@mellanox.com>

ib_add_slave_port() allocates a multiport struct but never frees it.
Don't leak memory, free the allocated mpi struct during driver unload.

Fixes: 32f69e4be269 ("{net, IB}/mlx5: Manage port association for multiport RoCE")
Signed-off-by: Danit Goldberg <danitg@mellanox.com>
---
 drivers/infiniband/hw/mlx5/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 0569bcab02d4..14807ea8dc3f 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -6959,6 +6959,7 @@ static void mlx5_ib_remove(struct mlx5_core_dev *mdev, void *context)
 			mlx5_ib_unbind_slave_port(mpi->ibdev, mpi);
 		list_del(&mpi->list);
 		mutex_unlock(&mlx5_ib_multiport_mutex);
+		kfree(mpi);
 		return;
 	}

--
2.20.1

Re: [PATCH 0/2] Two fixes for this merge window

[Jason Gunthorpe]

On Mon, Sep 16, 2019 at 09:48:16AM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> Hi,
> 
> We are in merge window so I didn't know if I should put -next or -rc,
> but they are better to go this cycle.
> 
> Thanks
> 
> Danit Goldberg (2):
>   IB/mlx5: Free page from the beginning of the allocation
>   IB/mlx5: Free mpi in mp_slave mode

Applied to for-next

Thanks,
Jason

Re: [PATCH 2/2] IB/mlx5: Free mpi in mp_slave mode

[Jason Gunthorpe]

On Mon, Sep 16, 2019 at 09:48:18AM +0300, Leon Romanovsky wrote:
> From: Danit Goldberg <danitg@mellanox.com>
> 
> ib_add_slave_port() allocates a multiport struct but never frees it.
> Don't leak memory, free the allocated mpi struct during driver unload.
> 
> Fixes: 32f69e4be269 ("{net, IB}/mlx5: Manage port association for multiport RoCE")
> Signed-off-by: Danit Goldberg <danitg@mellanox.com>
>  drivers/infiniband/hw/mlx5/main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> index 0569bcab02d4..14807ea8dc3f 100644
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -6959,6 +6959,7 @@ static void mlx5_ib_remove(struct mlx5_core_dev *mdev, void *context)
>  			mlx5_ib_unbind_slave_port(mpi->ibdev, mpi);
>  		list_del(&mpi->list);
>  		mutex_unlock(&mlx5_ib_multiport_mutex);
> +		kfree(mpi);
>  		return;
>  	}

Personally I think the way this code was written to try to share the
struct mlx5_interface between two completely different usages, with
totally different opaque structs is really obtuse. 

Two interfaces callback blocks for the two parallel uses would have
been cleaner and might have made this missing kfree clearer.

Jason