Skip to contentSkip to conversation

Two fixes for this merge window

PATCH Applied Leon Romanovsky 2 participants 5 messages Sep 16, 2019

Subsystems (2)
InfiniBand Mellanox MLX5 IB driver
lore
Share & tools
lore

Leon Romanovsky

author
#0 lore raw

cover Two fixes for this merge window

From: Leon Romanovsky <[email protected]>

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

author
#1 lore raw

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

From: Danit Goldberg <[email protected]>

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 <[email protected]>
Reviewed-by Yishai Hadas <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/hw/mlx5/odp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/infiniband/hw/mlx5/odp.c +5 -4
index 0a59912a4cef..416e750808eb 100644
@@ -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

author
#2 lore raw

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

In reply to Leon Romanovsky

From: Danit Goldberg <[email protected]>

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 <[email protected]>
---
 drivers/infiniband/hw/mlx5/main.c | 1 +
 1 file changed, 1 insertion(+)
drivers/infiniband/hw/mlx5/main.c +1 -0
index 0569bcab02d4..14807ea8dc3f 100644
@@ -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

#3 lore raw

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

Quoted 14 lines · Leon Romanovsky

From: Danit Goldberg <[email protected]>

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 <[email protected]>
 drivers/infiniband/hw/mlx5/main.c | 1 +
 1 file changed, 1 insertion(+)
drivers/infiniband/hw/mlx5/main.c +1 -0
index 0569bcab02d4..14807ea8dc3f 100644
+++ b/drivers/infiniband/hw/mlx5/main.c

drivers/infiniband/hw/mlx5/main.c static void mlx5_ib_remove(struct mlx5_core_dev *mdev, void *context) · line 6964

Quoted 6 lines
drivers/infiniband/hw/mlx5/main.c +1 -0
@@ -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

Applied
#4 lore raw
In reply to Leon Romanovsky
Quoted 4 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Hi,

Quoted 8 lines

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