Skip to content

Commit aa60381

Browse files
committed
RDMA/mlx5: Put live in the correct place for ODP MRs
live is used to signal to the pagefault thread that the MR is initialized and ready for use. It should be after the umem is assigned and all other setup is completed. This prevents races (at least) of the form: CPU0 CPU1 mlx5_ib_alloc_implicit_mr() implicit_mr_alloc() live = 1 imr->umem = umem num_pending_prefetch_inc() if (live) atomic_inc(num_pending_prefetch) atomic_set(num_pending_prefetch,0) // Overwrites other thread's store Further, live is being used with SRCU as the 'update' in an acquire/release fashion, so it can not be read and written raw. Move all live = 1's to after MR initialization is completed and use smp_store_release/smp_load_acquire() for manipulating it. Add a missing live = 0 when an implicit MR child is deleted, before queuing work to do synchronize_srcu(). The barriers in update_odp_mr() were some broken attempt to create a acquire/release, but were not even applied consistently and missed the point, delete it as well. Fixes: 6aec21f ("IB/mlx5: Page faults handling infrastructure") Link: https://lore.kernel.org/r/[email protected] Reviewed-by: Artemy Kovalyov <[email protected]> Signed-off-by: Jason Gunthorpe <[email protected]>
1 parent aa116b8 commit aa60381

File tree

3 files changed

+14
-38
lines changed

3 files changed

+14
-38
lines changed

drivers/infiniband/hw/mlx5/mlx5_ib.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ struct mlx5_ib_mr {
606606
struct mlx5_ib_dev *dev;
607607
u32 out[MLX5_ST_SZ_DW(create_mkey_out)];
608608
struct mlx5_core_sig_ctx *sig;
609-
int live;
609+
unsigned int live;
610610
void *descs_alloc;
611611
int access_flags; /* Needed for rereg MR */
612612

drivers/infiniband/hw/mlx5/mr.c

Lines changed: 5 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -84,32 +84,6 @@ static bool use_umr_mtt_update(struct mlx5_ib_mr *mr, u64 start, u64 length)
8484
length + (start & (MLX5_ADAPTER_PAGE_SIZE - 1));
8585
}
8686

87-
static void update_odp_mr(struct mlx5_ib_mr *mr)
88-
{
89-
if (is_odp_mr(mr)) {
90-
/*
91-
* This barrier prevents the compiler from moving the
92-
* setting of umem->odp_data->private to point to our
93-
* MR, before reg_umr finished, to ensure that the MR
94-
* initialization have finished before starting to
95-
* handle invalidations.
96-
*/
97-
smp_wmb();
98-
to_ib_umem_odp(mr->umem)->private = mr;
99-
/*
100-
* Make sure we will see the new
101-
* umem->odp_data->private value in the invalidation
102-
* routines, before we can get page faults on the
103-
* MR. Page faults can happen once we put the MR in
104-
* the tree, below this line. Without the barrier,
105-
* there can be a fault handling and an invalidation
106-
* before umem->odp_data->private == mr is visible to
107-
* the invalidation handler.
108-
*/
109-
smp_wmb();
110-
}
111-
}
112-
11387
static void reg_mr_callback(int status, struct mlx5_async_work *context)
11488
{
11589
struct mlx5_ib_mr *mr =
@@ -1346,8 +1320,6 @@ struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
13461320
mr->umem = umem;
13471321
set_mr_fields(dev, mr, npages, length, access_flags);
13481322

1349-
update_odp_mr(mr);
1350-
13511323
if (use_umr) {
13521324
int update_xlt_flags = MLX5_IB_UPD_XLT_ENABLE;
13531325

@@ -1363,10 +1335,12 @@ struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
13631335
}
13641336
}
13651337

1366-
if (IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING)) {
1367-
mr->live = 1;
1338+
if (is_odp_mr(mr)) {
1339+
to_ib_umem_odp(mr->umem)->private = mr;
13681340
atomic_set(&mr->num_pending_prefetch, 0);
13691341
}
1342+
if (IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING))
1343+
smp_store_release(&mr->live, 1);
13701344

13711345
return &mr->ibmr;
13721346
error:
@@ -1607,7 +1581,7 @@ static void dereg_mr(struct mlx5_ib_dev *dev, struct mlx5_ib_mr *mr)
16071581
/* Prevent new page faults and
16081582
* prefetch requests from succeeding
16091583
*/
1610-
mr->live = 0;
1584+
WRITE_ONCE(mr->live, 0);
16111585

16121586
/* Wait for all running page-fault handlers to finish. */
16131587
synchronize_srcu(&dev->mr_srcu);

drivers/infiniband/hw/mlx5/odp.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ static void mr_leaf_free_action(struct work_struct *work)
231231
mr->parent = NULL;
232232
synchronize_srcu(&mr->dev->mr_srcu);
233233

234-
if (imr->live) {
234+
if (smp_load_acquire(&imr->live)) {
235235
srcu_key = srcu_read_lock(&mr->dev->mr_srcu);
236236
mutex_lock(&odp_imr->umem_mutex);
237237
mlx5_ib_update_xlt(imr, idx, 1, 0,
@@ -318,6 +318,7 @@ void mlx5_ib_invalidate_range(struct ib_umem_odp *umem_odp, unsigned long start,
318318

319319
if (unlikely(!umem_odp->npages && mr->parent &&
320320
!umem_odp->dying)) {
321+
WRITE_ONCE(mr->live, 0);
321322
umem_odp->dying = 1;
322323
atomic_inc(&mr->parent->num_leaf_free);
323324
schedule_work(&umem_odp->work);
@@ -459,8 +460,6 @@ static struct mlx5_ib_mr *implicit_mr_alloc(struct ib_pd *pd,
459460
mr->ibmr.lkey = mr->mmkey.key;
460461
mr->ibmr.rkey = mr->mmkey.key;
461462

462-
mr->live = 1;
463-
464463
mlx5_ib_dbg(dev, "key %x dev %p mr %p\n",
465464
mr->mmkey.key, dev->mdev, mr);
466465

@@ -514,6 +513,8 @@ static struct ib_umem_odp *implicit_mr_get_data(struct mlx5_ib_mr *mr,
514513
mtt->parent = mr;
515514
INIT_WORK(&odp->work, mr_leaf_free_action);
516515

516+
smp_store_release(&mtt->live, 1);
517+
517518
if (!nentries)
518519
start_idx = addr >> MLX5_IMR_MTT_SHIFT;
519520
nentries++;
@@ -566,6 +567,7 @@ struct mlx5_ib_mr *mlx5_ib_alloc_implicit_mr(struct mlx5_ib_pd *pd,
566567
init_waitqueue_head(&imr->q_leaf_free);
567568
atomic_set(&imr->num_leaf_free, 0);
568569
atomic_set(&imr->num_pending_prefetch, 0);
570+
smp_store_release(&imr->live, 1);
569571

570572
return imr;
571573
}
@@ -807,7 +809,7 @@ static int pagefault_single_data_segment(struct mlx5_ib_dev *dev,
807809
switch (mmkey->type) {
808810
case MLX5_MKEY_MR:
809811
mr = container_of(mmkey, struct mlx5_ib_mr, mmkey);
810-
if (!mr->live || !mr->ibmr.pd) {
812+
if (!smp_load_acquire(&mr->live) || !mr->ibmr.pd) {
811813
mlx5_ib_dbg(dev, "got dead MR\n");
812814
ret = -EFAULT;
813815
goto srcu_unlock;
@@ -1675,12 +1677,12 @@ static bool num_pending_prefetch_inc(struct ib_pd *pd,
16751677

16761678
mr = container_of(mmkey, struct mlx5_ib_mr, mmkey);
16771679

1678-
if (mr->ibmr.pd != pd) {
1680+
if (!smp_load_acquire(&mr->live)) {
16791681
ret = false;
16801682
break;
16811683
}
16821684

1683-
if (!mr->live) {
1685+
if (mr->ibmr.pd != pd) {
16841686
ret = false;
16851687
break;
16861688
}

0 commit comments

Comments
 (0)