From: Jason Gunthorpe <jgg@mellanox.com>
Date: Wed, 9 Oct 2019 13:09:30 -0300
Subject: RDMA/mlx5: Reduce locking in implicit_mr_get_data()
Patch-mainline: v5.5-rc1
Git-commit: 3389baa831b6a09e3c96e2a6283a1b952be2f0cd
References: jsc#SLE-8446
Now that the child MRs are stored in an xarray we can rely on the SRCU
lock to protect the xa_load and use xa_cmpxchg on the slow allocation path
to resolve races with concurrent page fault.
This reduces the scope of the critical section of umem_mutex for implicit
MRs to only cover mlx5_ib_update_xlt, and avoids taking a lock at all if
the child MR is already in the xarray. This makes it consistent with the
normal ODP MR critical section for umem_lock, and the locking approach
used for destroying an unusued implicit child MR.
The MLX5_IB_UPD_XLT_ATOMIC is no longer needed in implicit_get_child_mr()
since it is no longer called with any locks.
Link: https://lore.kernel.org/r/20191009160934.3143-11-jgg@ziepe.ca
Reviewed-by: Artemy Kovalyov <artemyko@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Acked-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
drivers/infiniband/hw/mlx5/odp.c | 38 ++++++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 12 deletions(-)
--- a/drivers/infiniband/hw/mlx5/odp.c
+++ b/drivers/infiniband/hw/mlx5/odp.c
@@ -386,8 +386,7 @@ static struct mlx5_ib_mr *implicit_get_c
MLX5_IMR_MTT_ENTRIES,
PAGE_SHIFT,
MLX5_IB_UPD_XLT_ZAP |
- MLX5_IB_UPD_XLT_ENABLE |
- MLX5_IB_UPD_XLT_ATOMIC);
+ MLX5_IB_UPD_XLT_ENABLE);
if (err) {
ret = ERR_PTR(err);
goto out_release;
@@ -397,9 +396,16 @@ static struct mlx5_ib_mr *implicit_get_c
* Once the store to either xarray completes any error unwind has to
* use synchronize_srcu(). Avoid this with xa_reserve()
*/
- err = xa_err(xa_store(&imr->implicit_children, idx, mr, GFP_KERNEL));
- if (err) {
- ret = ERR_PTR(err);
+ ret = xa_cmpxchg(&imr->implicit_children, idx, NULL, mr, GFP_KERNEL);
+ if (unlikely(ret)) {
+ if (xa_is_err(ret)) {
+ ret = ERR_PTR(xa_err(ret));
+ goto out_release;
+ }
+ /*
+ * Another thread beat us to creating the child mr, use
+ * theirs.
+ */
goto out_release;
}
@@ -429,7 +435,8 @@ static struct mlx5_ib_mr *implicit_mr_ge
struct mlx5_ib_mr *result = NULL;
int ret;
- mutex_lock(&odp_imr->umem_mutex);
+ lockdep_assert_held(&imr->dev->odp_srcu);
+
for (idx = idx; idx <= end_idx; idx++) {
struct mlx5_ib_mr *mtt = xa_load(&imr->implicit_children, idx);
@@ -455,20 +462,27 @@ static struct mlx5_ib_mr *implicit_mr_ge
*/
out:
if (likely(!inv_len))
- goto out_unlock;
+ return result;
+ /*
+ * Notice this is not strictly ordered right, the KSM is updated after
+ * the implicit_leaves is updated, so a parallel page fault could see
+ * a MR that is not yet visible in the KSM. This is similar to a
+ * parallel page fault seeing a MR that is being concurrently removed
+ * from the KSM. Both of these improbable situations are resolved
+ * safely by resuming the HW and then taking another page fault. The
+ * next pagefault handler will see the new information.
+ */
+ mutex_lock(&odp_imr->umem_mutex);
ret = mlx5_ib_update_xlt(imr, inv_start_idx, inv_len, 0,
MLX5_IB_UPD_XLT_INDIRECT |
MLX5_IB_UPD_XLT_ATOMIC);
+ mutex_unlock(&odp_imr->umem_mutex);
if (ret) {
mlx5_ib_err(to_mdev(imr->ibmr.pd->device),
"Failed to update PAS\n");
- result = ERR_PTR(ret);
- goto out_unlock;
+ return ERR_PTR(ret);
}
-
-out_unlock:
- mutex_unlock(&odp_imr->umem_mutex);
return result;
}