Skip to contentSkip to conversation

RDMA/irdma: Replace waitqueue and flag with completion

PATCH Applied Jacob Moroni 2 participants 2 messages Jun 16

Subsystems (2)
InfiniBand Intel Ethernet protocol driver for RDMA
lore
Share & tools
lore

Jacob Moroni

author
#0 lore raw

PATCH 1/1 RDMA/irdma: Replace waitqueue and flag with completion

The driver previously used a waitqueue along with an explicit request_done flag, but without proper barriers around request_done.

An earlier patch by Gui-Dong Han <[email protected]> attempted to fix this by adding the missing memory barriers. Rather than adding the barriers, this patch replaces the waitqueue+flag with a completion, which is designed for this exact purpose.

Link https://lore.kernel.org/linux-rdma/[email protected]/T/#t open here
Fixes 44d9e52977a1 ("RDMA/irdma: Implement device initialization definitions")
Fixes 915cc7ac0f8e ("RDMA/irdma: Add miscellaneous utility definitions")
Signed-off-by Jacob Moroni <[email protected]>
---
 drivers/infiniband/hw/irdma/hw.c    |  7 +++----
 drivers/infiniband/hw/irdma/main.h  |  3 +--
 drivers/infiniband/hw/irdma/utils.c | 12 +++++-------
 3 files changed, 9 insertions(+), 13 deletions(-)
drivers/infiniband/hw/irdma/hw.c +3 -4
index f9be467d13..c345cc6542 100644
@@ -235,8 +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);
-		wake_up(&cqp_request->waitq);
+		complete_all(&cqp_request->comp);
 	} else if (cqp_request->callback_fcn) {
 		cqp_request->callback_fcn(cqp_request);
 	}

@@ -1107,9 +1106,9 @@ static int irdma_create_cqp(struct irdma_pci_f *rf)
 	INIT_LIST_HEAD(&cqp->cqp_avail_reqs);
 	INIT_LIST_HEAD(&cqp->cqp_pending_reqs);
 
-	/* init the waitqueue of the cqp_requests and add them to the list */
+	/* init the completion of the cqp_requests and add them to the list */
 	for (i = 0; i < sqsize; i++) {
-		init_waitqueue_head(&cqp->cqp_requests[i].waitq);
+		init_completion(&cqp->cqp_requests[i].comp);
 		list_add_tail(&cqp->cqp_requests[i].list, &cqp->cqp_avail_reqs);
 	}
 	init_waitqueue_head(&cqp->remove_wq);
drivers/infiniband/hw/irdma/main.h +1 -2
index 3d49bd57ba..8c17a201c1 100644
@@ -161,13 +161,12 @@ struct irdma_cqp_compl_info {
 
 struct irdma_cqp_request {
 	struct cqp_cmds_info info;
-	wait_queue_head_t waitq;
+	struct completion comp;
 	struct list_head list;
 	refcount_t refcnt;
 	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 waiting:1;
 	bool dynamic:1;
 	bool pending:1;
drivers/infiniband/hw/irdma/utils.c +5 -7
index 7cc7b107db..e4037d5ef8 100644
@@ -442,7 +442,7 @@ struct irdma_cqp_request *irdma_alloc_and_get_cqp_request(struct irdma_cqp *cqp,
 		if (cqp_request) {
 			cqp_request->dynamic = true;
 			if (wait)
-				init_waitqueue_head(&cqp_request->waitq);
+				init_completion(&cqp_request->comp);
 		}
 	}
 	if (!cqp_request) {

@@ -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);
+		reinit_completion(&cqp_request->comp);
 		cqp_request->callback_fcn = NULL;
 		cqp_request->waiting = false;
 		cqp_request->pending = false;

@@ -515,8 +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);
-		wake_up(&cqp_request->waitq);
+		complete_all(&cqp_request->comp);
 	}
 	wait_event_timeout(cqp->remove_wq,
 			   refcount_read(&cqp_request->refcnt) == 1, 1000);

@@ -609,9 +608,8 @@ static int irdma_wait_event(struct irdma_pci_f *rf,
 	cqp_timeout.compl_cqp_cmds = atomic64_read(&rf->sc_dev.cqp->completed_ops);
 	do {
 		irdma_cqp_ce_handler(rf, &rf->ccq.sc_cq);
-		if (wait_event_timeout(cqp_request->waitq,
-				       READ_ONCE(cqp_request->request_done),
-				       msecs_to_jiffies(CQP_COMPL_WAIT_TIME_MS)))
+		if (wait_for_completion_timeout(&cqp_request->comp,
+					msecs_to_jiffies(CQP_COMPL_WAIT_TIME_MS)))
 			break;
 
 		if (cqp_request->pending)
Signature · 1 line
2.54.0.1136.gdb2ca164c4-goog
maintainer Applied
#1 lore raw
Quoted 12 lines · Jacob Moroni

The driver previously used a waitqueue along with an explicit request_done flag, but without proper barriers around request_done.

An earlier patch by Gui-Dong Han <[email protected]> attempted to fix this by adding the missing memory barriers. Rather than adding the barriers, this patch replaces the waitqueue+flag with a completion, which is designed for this exact purpose.

Link https://lore.kernel.org/linux-rdma/[email protected]/T/#t open here
Fixes 44d9e52977a1 ("RDMA/irdma: Implement device initialization definitions")
Fixes 915cc7ac0f8e ("RDMA/irdma: Add miscellaneous utility definitions")
Signed-off-by Jacob Moroni <[email protected]>
Quoted 5 lines
---
 drivers/infiniband/hw/irdma/hw.c    |  7 +++----
 drivers/infiniband/hw/irdma/main.h  |  3 +--
 drivers/infiniband/hw/irdma/utils.c | 12 +++++-------
 3 files changed, 9 insertions(+), 13 deletions(-)

Applied to for-next, thanks

Jason