The completion path fills cqp_request->compl_info before marking
request_done and waking waiters. irdma_wait_event() waits for
request_done before reading compl_info.

READ_ONCE()/WRITE_ONCE() do not order these accesses, so a waiter can
observe request_done while still seeing stale completion information.

Use release/acquire helpers for request_done to publish compl_info to the
waiting thread.

Clearing request_done does not publish completion information. It only
reinitializes a private request before returning it to cqp_avail_reqs under
req_lock, so WRITE_ONCE() is enough for the false transition.

Fixes: 44d9e52977a1 ("RDMA/irdma: Implement device initialization definitions")
Fixes: 915cc7ac0f8e ("RDMA/irdma: Add miscellaneous utility definitions")
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
Found by auditing READ_ONCE() used for synchronization.
A similar fix can be found in 8df672bfe3ec.
---
 drivers/infiniband/hw/irdma/hw.c    |  2 +-
 drivers/infiniband/hw/irdma/main.h  | 22 +++++++++++++++++++++-
 drivers/infiniband/hw/irdma/utils.c |  6 +++---
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/irdma/hw.c b/drivers/infiniband/hw/irdma/hw.c
index f9be467d137f..96fecb28af97 100644
--- a/drivers/infiniband/hw/irdma/hw.c
+++ b/drivers/infiniband/hw/irdma/hw.c
@@ -235,7 +235,7 @@ static void irdma_complete_cqp_request(struct irdma_cqp *cqp,
 				       struct irdma_cqp_request *cqp_request)
 {
 	if (cqp_request->waiting) {
-		WRITE_ONCE(cqp_request->request_done, true);
+		irdma_cqp_request_mark_done(cqp_request);
 		wake_up(&cqp_request->waitq);
 	} else if (cqp_request->callback_fcn) {
 		cqp_request->callback_fcn(cqp_request);
diff --git a/drivers/infiniband/hw/irdma/main.h b/drivers/infiniband/hw/irdma/main.h
index 3d49bd57bae7..57102278ed64 100644
--- a/drivers/infiniband/hw/irdma/main.h
+++ b/drivers/infiniband/hw/irdma/main.h
@@ -167,12 +167,32 @@ struct irdma_cqp_request {
 	void (*callback_fcn)(struct irdma_cqp_request *cqp_request);
 	void *param;
 	struct irdma_cqp_compl_info compl_info;
-	bool request_done; /* READ/WRITE_ONCE macros operate on it */
+	bool request_done; /* Use irdma_cqp_request_*() helpers. */
 	bool waiting:1;
 	bool dynamic:1;
 	bool pending:1;
 };
 
+static inline void
+irdma_cqp_request_clear_done(struct irdma_cqp_request *cqp_request)
+{
+	WRITE_ONCE(cqp_request->request_done, false);
+}
+
+static inline void
+irdma_cqp_request_mark_done(struct irdma_cqp_request *cqp_request)
+{
+	/* Publish compl_info before waking the waiter. */
+	smp_store_release(&cqp_request->request_done, true);
+}
+
+static inline bool
+irdma_cqp_request_is_done(struct irdma_cqp_request *cqp_request)
+{
+	/* Pair with irdma_cqp_request_mark_done(). */
+	return smp_load_acquire(&cqp_request->request_done);
+}
+
 struct irdma_cqp {
 	struct irdma_sc_cqp sc_cqp;
 	spinlock_t req_lock; /* protect CQP request list */
diff --git a/drivers/infiniband/hw/irdma/utils.c b/drivers/infiniband/hw/irdma/utils.c
index 495e5daff4b4..566f690a4e58 100644
--- a/drivers/infiniband/hw/irdma/utils.c
+++ b/drivers/infiniband/hw/irdma/utils.c
@@ -480,7 +480,7 @@ void irdma_free_cqp_request(struct irdma_cqp *cqp,
 	if (cqp_request->dynamic) {
 		kfree(cqp_request);
 	} else {
-		WRITE_ONCE(cqp_request->request_done, false);
+		irdma_cqp_request_clear_done(cqp_request);
 		cqp_request->callback_fcn = NULL;
 		cqp_request->waiting = false;
 		cqp_request->pending = false;
@@ -515,7 +515,7 @@ irdma_free_pending_cqp_request(struct irdma_cqp *cqp,
 {
 	if (cqp_request->waiting) {
 		cqp_request->compl_info.error = true;
-		WRITE_ONCE(cqp_request->request_done, true);
+		irdma_cqp_request_mark_done(cqp_request);
 		wake_up(&cqp_request->waitq);
 	}
 	wait_event_timeout(cqp->remove_wq,
@@ -610,7 +610,7 @@ static int irdma_wait_event(struct irdma_pci_f *rf,
 	do {
 		irdma_cqp_ce_handler(rf, &rf->ccq.sc_cq);
 		if (wait_event_timeout(cqp_request->waitq,
-				       READ_ONCE(cqp_request->request_done),
+				       irdma_cqp_request_is_done(cqp_request),
 				       msecs_to_jiffies(CQP_COMPL_WAIT_TIME_MS)))
 			break;
 
-- 
2.34.1

Makes sense to me.

That said, I wonder if we should just get rid of the wait queue + explicit
flag and replace it with a completion?

- Jake

On Thu, Jun 04, 2026 at 09:41:24AM -0400, Jacob Moroni wrote:
> Makes sense to me.
> 
> That said, I wonder if we should just get rid of the wait queue + explicit
> flag and replace it with a completion?

Yeah, then you don't have to worry about this.. IMHO the driver was
following a fairly common pattern for wait event that is indeed
probably incomplete.

Jason