RDMA/mlx5: Prevent prefetch from racing with implicit destruction
Subsystems (2)
PATCH 1/1 RDMA/mlx5: Prevent prefetch from racing with implicit destruction
From: Jason Gunthorpe <[email protected]>
Prefetch work in mlx5_ib_prefetch_mr_work can be queued and able to run concurrently with destruction of the implicit MR. The num_deferred_work was intended to serialize this, but there is a race:
CPU0 CPU1
mlx5_ib_free_implicit_mr()
xa_erase(odp_mkeys)
synchronize_srcu()
__xa_erase(implicit_children)
mlx5_ib_prefetch_mr_work()
pagefault_mr()
pagefault_implicit_mr()
implicit_get_child_mr()
xa_cmpxchg()
atomic_dec_and_test(num_deferred_mr)
wait_event(imr->q_deferred_work)
ib_umem_odp_release(odp_imr)
kfree(odp_imr)At this point in mlx5_ib_free_implicit_mr() the implicit_children list is supposed to be empty forever so that destroy_unused_implicit_child_mr() and related are not and will not be running.
Since it is not empty the destroy_unused_implicit_child_mr() flow ends up touching deallocated memory as mlx5_ib_free_implicit_mr() already tore down the imr parent.
The solution is to flush out the prefetch wq by driving num_deferred_work to zero after creation of new prefetch work is blocked.
--- drivers/infiniband/hw/mlx5/odp.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
drivers/infiniband/hw/mlx5/odp.c +19 -3
@@ -601,6 +601,23 @@ void mlx5_ib_free_implicit_mr(struct mlx5_ib_mr *imr)*/ synchronize_srcu (&dev->odp_srcu); +/* +* All work on the prefetch list must be completed, xa_erase() prevented +* new work from being created. +*/ +wait_event (imr->q_deferred_work, !atomic_read (&imr->num_deferred_work)); + +/* +* At this point it is forbidden for any other thread to enter +* pagefault_mr() on this imr. It is already forbidden to call +* pagefault_mr() on an implicit child. Due to this additions to +* implicit_children are prevented. +*/ + +/* +* Block destroy_unused_implicit_child_mr() from incrementing +* num_deferred_work. +*/ xa_lock (&imr->implicit_children); xa_for_each (&imr->implicit_children, idx, mtt) {__xa_erase (&imr->implicit_children, idx);
@@ -609,9 +626,8 @@ void mlx5_ib_free_implicit_mr(struct mlx5_ib_mr *imr)xa_unlock (&imr->implicit_children);/* -* num_deferred_work can only be incremented inside the odp_srcu, or -* under xa_lock while the child is in the xarray. Thus at this point -* it is only decreasing, and all work holding it is now on the wq. +* Wait for any concurrent destroy_unused_implicit_child_mr() to +* complete. */ wait_event (imr->q_deferred_work, !atomic_read (&imr->num_deferred_work));
Signature · 1 line
2.26.2
Quoted 33 lines · Leon Romanovsky
From: Jason Gunthorpe <[email protected]>
Prefetch work in
mlx5_ib_prefetch_mr_workcan be queued and able to run concurrently with destruction of the implicit MR. Thenum_deferred_workwas intended to serialize this, but there is a race:CPU0 CPU1 mlx5_ib_free_implicit_mr() xa_erase(odp_mkeys) synchronize_srcu() __xa_erase(implicit_children) mlx5_ib_prefetch_mr_work() pagefault_mr() pagefault_implicit_mr() implicit_get_child_mr() xa_cmpxchg() atomic_dec_and_test(num_deferred_mr) wait_event(imr->q_deferred_work) ib_umem_odp_release(odp_imr) kfree(odp_imr)At this point in
mlx5_ib_free_implicit_mr()theimplicit_childrenlist is supposed to be empty forever so thatdestroy_unused_implicit_child_mr()and related are not and will not be running.Since it is not empty the
destroy_unused_implicit_child_mr()flow ends up touching deallocated memory asmlx5_ib_free_implicit_mr()already tore down the imr parent.The solution is to flush out the prefetch wq by driving
num_deferred_workto zero after creation of new prefetch work is blocked.
Quoted 6 lines
Fixes 5256edcb98a1 ("RDMA/mlx5: Rework implicit ODP destroy")Signed-off-by Jason Gunthorpe <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>--- drivers/infiniband/hw/mlx5/odp.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
Applied to for-rc, thanks
Jason