mlx5 has a common pattern implementing a device-global singleton resource
where it checks the resource pointer for !NULL and then skips obtaining
the lock.
This is not ordered properly as observing !NULL doesn't mean that all the
data under that pointer is also visible on this CPU when the lock is not
taken.
Use a release/acquire pairing to explicitly manage this.
Pointed out by sashiko, Codex found more cases.
Fixes: 5895e70f2e6e ("IB/mlx5: Allocate resources just before first QP/SRQ is
created")
Fixes: 638420115cc4 ("IB/mlx5: Create UMR QP just before first reg_mr occurs")
Link:
https://sashiko.dev/#/patchset/SYBPR01MB7881E1E0970268BD69C0BA75AF2B2%40SYBPR01MB7881.ausprd01.prod.outlook.com
Assisted-by: Codex:GPT-5.5
Signed-off-by: Jason Gunthorpe <[email protected]>
---
drivers/infiniband/hw/mlx5/main.c | 8 ++++----
drivers/infiniband/hw/mlx5/umr.c | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/main.c
b/drivers/infiniband/hw/mlx5/main.c
index 109661c2ac12b0..73fab8a376933d 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -3310,7 +3310,7 @@ int mlx5_ib_dev_res_cq_init(struct mlx5_ib_dev *dev)
* devr->c0 is set once, never changed until device unload.
* Avoid taking the mutex if initialization is already done.
*/
- if (devr->c0)
+ if (smp_load_acquire(&devr->c0))
return 0;
mutex_lock(&devr->cq_lock);
@@ -3336,7 +3336,7 @@ int mlx5_ib_dev_res_cq_init(struct mlx5_ib_dev *dev)
}
devr->p0 = pd;
- devr->c0 = cq;
+ smp_store_release(&devr->c0, cq);
unlock:
mutex_unlock(&devr->cq_lock);
@@ -3354,7 +3354,7 @@ int mlx5_ib_dev_res_srq_init(struct mlx5_ib_dev *dev)
* devr->s1 is set once, never changed until device unload.
* Avoid taking the mutex if initialization is already done.
*/
- if (devr->s1)
+ if (smp_load_acquire(&devr->s1))
return 0;
mutex_lock(&devr->srq_lock);
@@ -3395,7 +3395,7 @@ int mlx5_ib_dev_res_srq_init(struct mlx5_ib_dev *dev)
}
devr->s0 = s0;
- devr->s1 = s1;
+ smp_store_release(&devr->s1, s1);
unlock:
mutex_unlock(&devr->srq_lock);
diff --git a/drivers/infiniband/hw/mlx5/umr.c b/drivers/infiniband/hw/mlx5/umr.c
index 29488fba21a034..f2139474be3751 100644
--- a/drivers/infiniband/hw/mlx5/umr.c
+++ b/drivers/infiniband/hw/mlx5/umr.c
@@ -147,7 +147,7 @@ int mlx5r_umr_resource_init(struct mlx5_ib_dev *dev)
* UMR qp is set once, never changed until device unload.
* Avoid taking the mutex if initialization is already done.
*/
- if (dev->umrc.qp)
+ if (smp_load_acquire(&dev->umrc.qp))
return 0;
mutex_lock(&dev->umrc.init_lock);
@@ -185,7 +185,7 @@ int mlx5r_umr_resource_init(struct mlx5_ib_dev *dev)
sema_init(&dev->umrc.sem, MAX_UMR_WR);
mutex_init(&dev->umrc.lock);
dev->umrc.state = MLX5_UMR_STATE_ACTIVE;
- dev->umrc.qp = qp;
+ smp_store_release(&dev->umrc.qp, qp);
mutex_unlock(&dev->umrc.init_lock);
return 0;
--
2.43.0
