[PATCH RESEND for-next v6 0/4] RDMA/rxe: Fix no completion event issue

[PATCH RESEND for-next v6 0/4] RDMA/rxe: Fix no completion event issue

[Li Zhijian]

No change since v5, just resend it via another smtp instead of
Microsoft Exchange which made patches messed up.

It's observed that no more completion occurs after a few incorrect posts.
Actually, it will block the polling. we can easily reproduce it by the below
pattern.

a. post correct RDMA_WRITE
b. poll completion event
while true {
  c. post incorrect RDMA_WRITE(wrong rkey for example)
  d. poll completion event <<<< block after 2 incorrect RDMA_WRITE posts
}

V4 add new patch from Bob where it make requester stop executing qp
operation as soon as possible.

Both blktests and pyverbs tests are passed fine.

Bob Pearson (1):
  RDMA/rxe: Split qp state for requester and completer

Li Zhijian (3):
  RDMA/rxe: Update wqe_index for each wqe error completion
  RDMA/rxe: Generate error completion for error requester QP state
  RDMA/rxe: Fix typo in comment

 drivers/infiniband/sw/rxe/rxe_comp.c  |  6 +++---
 drivers/infiniband/sw/rxe/rxe_qp.c    |  5 +++++
 drivers/infiniband/sw/rxe/rxe_req.c   | 16 +++++++++++++++-
 drivers/infiniband/sw/rxe/rxe_task.c  |  2 +-
 drivers/infiniband/sw/rxe/rxe_verbs.h |  1 +
 5 files changed, 25 insertions(+), 5 deletions(-)

-- 
2.31.1

[PATCH RESEND for-next v6 1/4] RDMA/rxe: Update wqe_index for each wqe error completion

[Li Zhijian]

Previously, if user space keeps sending abnormal wqe, queue.index will
keep increasing while qp->req.wqe_index doesn't. Once
qp->req.wqe_index==queue.index in next round, req_next_wqe() will treat queue
as empty. In such case, no new completion would be generated.

Update wqe_index for each wqe completion so that req_next_wqe() can get
next wqe properly.

Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
 drivers/infiniband/sw/rxe/rxe_req.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index dec4ddaca70f..c187deeb6e6b 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -760,6 +760,8 @@ int rxe_requester(void *arg)
 	if (ah)
 		rxe_put(ah);
 err:
+	/* update wqe_index for each wqe completion */
+	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
 	wqe->state = wqe_state_error;
 	__rxe_do_task(&qp->comp.task);
 
-- 
2.31.1

[PATCH RESEND for-next v6 2/4] RDMA/rxe: Generate error completion for error requester QP state

[Li Zhijian]

As per IBTA specification, all subsequent WQEs while QP is in error
state should be completed with a flush error.

Here we check QP_STATE_ERROR after req_next_wqe() so that rxe_completer()
has chance to be called where it will set CQ state to FLUSH ERROR and the
completion can associate with its WQE.

Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
V5: parentheses issue # Cheng Xu
V4: check QP ERROR before QP RESET # Bob
V3: unlikely() optimization # Cheng Xu <chengyou@linux.alibaba.com>
    update commit log # Haakon Bugge <haakon.bugge@oracle.com>
---
 drivers/infiniband/sw/rxe/rxe_req.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index c187deeb6e6b..cbb2ce2d7b50 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -611,9 +611,20 @@ int rxe_requester(void *arg)
 		return -EAGAIN;
 
 next_wqe:
-	if (unlikely(!qp->valid || qp->req.state == QP_STATE_ERROR))
+	if (unlikely(!qp->valid))
 		goto exit;
 
+	if (unlikely(qp->req.state == QP_STATE_ERROR)) {
+		wqe = req_next_wqe(qp);
+		if (wqe)
+			/*
+			 * Generate an error completion for error qp state
+			 */
+			goto err;
+		else
+			goto exit;
+	}
+
 	if (unlikely(qp->req.state == QP_STATE_RESET)) {
 		qp->req.wqe_index = queue_get_consumer(q,
 						QUEUE_TYPE_FROM_CLIENT);
-- 
2.31.1

[PATCH RESEND for-next v6 3/4] RDMA/rxe: Split qp state for requester and completer

[Li Zhijian]

From: Bob Pearson <rpearsonhpe@gmail.com>

Currently the requester can continue to process send wqes after
an local qp operation error is detected because the setting of
the qp state to the error state is deferred until later. This
patch splits the qp state for the completer and requester into
two separate states and sets qp->req.state = QP_STATE_ERROR as
soon as the error is detected before another wqe can be executed.

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
V4: new patch
---
 drivers/infiniband/sw/rxe/rxe_comp.c  | 6 +++---
 drivers/infiniband/sw/rxe/rxe_qp.c    | 5 +++++
 drivers/infiniband/sw/rxe/rxe_req.c   | 1 +
 drivers/infiniband/sw/rxe/rxe_verbs.h | 1 +
 4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index da3a398053b8..0b68630a3e49 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -565,10 +565,10 @@ int rxe_completer(void *arg)
 	if (!rxe_get(qp))
 		return -EAGAIN;
 
-	if (!qp->valid || qp->req.state == QP_STATE_ERROR ||
-	    qp->req.state == QP_STATE_RESET) {
+	if (!qp->valid || qp->comp.state == QP_STATE_ERROR ||
+	    qp->comp.state == QP_STATE_RESET) {
 		rxe_drain_resp_pkts(qp, qp->valid &&
-				    qp->req.state == QP_STATE_ERROR);
+				    qp->comp.state == QP_STATE_ERROR);
 		ret = -EAGAIN;
 		goto done;
 	}
diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
index f69e3ff095f8..becdcfdfc49b 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -228,6 +228,7 @@ static int rxe_qp_init_req(struct rxe_dev *rxe, struct rxe_qp *qp,
 					       QUEUE_TYPE_FROM_CLIENT);
 
 	qp->req.state		= QP_STATE_RESET;
+	qp->comp.state		= QP_STATE_RESET;
 	qp->req.opcode		= -1;
 	qp->comp.opcode		= -1;
 
@@ -488,6 +489,7 @@ static void rxe_qp_reset(struct rxe_qp *qp)
 
 	/* move qp to the reset state */
 	qp->req.state = QP_STATE_RESET;
+	qp->comp.state = QP_STATE_RESET;
 	qp->resp.state = QP_STATE_RESET;
 
 	/* let state machines reset themselves drain work and packet queues
@@ -550,6 +552,7 @@ void rxe_qp_error(struct rxe_qp *qp)
 {
 	qp->req.state = QP_STATE_ERROR;
 	qp->resp.state = QP_STATE_ERROR;
+	qp->comp.state = QP_STATE_ERROR;
 	qp->attr.qp_state = IB_QPS_ERR;
 
 	/* drain work and packet queues */
@@ -687,6 +690,7 @@ int rxe_qp_from_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask,
 			pr_debug("qp#%d state -> INIT\n", qp_num(qp));
 			qp->req.state = QP_STATE_INIT;
 			qp->resp.state = QP_STATE_INIT;
+			qp->comp.state = QP_STATE_INIT;
 			break;
 
 		case IB_QPS_RTR:
@@ -697,6 +701,7 @@ int rxe_qp_from_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask,
 		case IB_QPS_RTS:
 			pr_debug("qp#%d state -> RTS\n", qp_num(qp));
 			qp->req.state = QP_STATE_READY;
+			qp->comp.state = QP_STATE_READY;
 			break;
 
 		case IB_QPS_SQD:
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index cbb2ce2d7b50..35a249727435 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -774,6 +774,7 @@ int rxe_requester(void *arg)
 	/* update wqe_index for each wqe completion */
 	qp->req.wqe_index = queue_next_index(qp->sq.queue, qp->req.wqe_index);
 	wqe->state = wqe_state_error;
+	qp->req.state = QP_STATE_ERROR;
 	__rxe_do_task(&qp->comp.task);
 
 exit:
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index 628e40c1714b..ab899aee72e4 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -128,6 +128,7 @@ struct rxe_req_info {
 };
 
 struct rxe_comp_info {
+	enum rxe_qp_state	state;
 	u32			psn;
 	int			opcode;
 	int			timeout;
-- 
2.31.1

[PATCH RESEND for-next v6 4/4] RDMA/rxe: Fix typo in comment

[Li Zhijian]

Fix a spelling mistake

Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_task.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_task.c b/drivers/infiniband/sw/rxe/rxe_task.c
index 0c4db5bb17d7..c9b80410cd5b 100644
--- a/drivers/infiniband/sw/rxe/rxe_task.c
+++ b/drivers/infiniband/sw/rxe/rxe_task.c
@@ -67,7 +67,7 @@ void rxe_do_task(struct tasklet_struct *t)
 				cont = 1;
 			break;
 
-		/* soneone tried to run the task since the last time we called
+		/* someone tried to run the task since the last time we called
 		 * func, so we will call one more time regardless of the
 		 * return value
 		 */
-- 
2.31.1

Re: [PATCH RESEND for-next v6 0/4] RDMA/rxe: Fix no completion event issue

[Jason Gunthorpe]

On Wed, Jul 20, 2022 at 04:56:04AM -0400, Li Zhijian wrote:
> No change since v5, just resend it via another smtp instead of
> Microsoft Exchange which made patches messed up.
> 
> It's observed that no more completion occurs after a few incorrect posts.
> Actually, it will block the polling. we can easily reproduce it by the below
> pattern.
> 
> a. post correct RDMA_WRITE
> b. poll completion event
> while true {
>   c. post incorrect RDMA_WRITE(wrong rkey for example)
>   d. poll completion event <<<< block after 2 incorrect RDMA_WRITE posts
> }
> 
> V4 add new patch from Bob where it make requester stop executing qp
> operation as soon as possible.
> 
> Both blktests and pyverbs tests are passed fine.
>
> Bob Pearson (1):
>   RDMA/rxe: Split qp state for requester and completer
> 
> Li Zhijian (3):
>   RDMA/rxe: Update wqe_index for each wqe error completion
>   RDMA/rxe: Generate error completion for error requester QP state
>   RDMA/rxe: Fix typo in comment

Bob are you Ok with these?

Jason

Re: [PATCH RESEND for-next v6 0/4] RDMA/rxe: Fix no completion event issue

[Bob Pearson]

On 7/29/22 14:48, Jason Gunthorpe wrote:
> On Wed, Jul 20, 2022 at 04:56:04AM -0400, Li Zhijian wrote:
>> No change since v5, just resend it via another smtp instead of
>> Microsoft Exchange which made patches messed up.
>>
>> It's observed that no more completion occurs after a few incorrect posts.
>> Actually, it will block the polling. we can easily reproduce it by the below
>> pattern.
>>
>> a. post correct RDMA_WRITE
>> b. poll completion event
>> while true {
>>   c. post incorrect RDMA_WRITE(wrong rkey for example)
>>   d. poll completion event <<<< block after 2 incorrect RDMA_WRITE posts
>> }
>>
>> V4 add new patch from Bob where it make requester stop executing qp
>> operation as soon as possible.
>>
>> Both blktests and pyverbs tests are passed fine.
>>
>> Bob Pearson (1):
>>   RDMA/rxe: Split qp state for requester and completer
>>
>> Li Zhijian (3):
>>   RDMA/rxe: Update wqe_index for each wqe error completion
>>   RDMA/rxe: Generate error completion for error requester QP state
>>   RDMA/rxe: Fix typo in comment
> 
> Bob are you Ok with these?
> 
> Jason

yes. I had reviewed these a while ago and suggested a change which he included.
I'm fine with this.

Bob

Re: [PATCH RESEND for-next v6 0/4] RDMA/rxe: Fix no completion event issue

[Jason Gunthorpe]

On Wed, Jul 20, 2022 at 04:56:04AM -0400, Li Zhijian wrote:
> No change since v5, just resend it via another smtp instead of
> Microsoft Exchange which made patches messed up.
> 
> It's observed that no more completion occurs after a few incorrect posts.
> Actually, it will block the polling. we can easily reproduce it by the below
> pattern.
> 
> a. post correct RDMA_WRITE
> b. poll completion event
> while true {
>   c. post incorrect RDMA_WRITE(wrong rkey for example)
>   d. poll completion event <<<< block after 2 incorrect RDMA_WRITE posts
> }
> 
> V4 add new patch from Bob where it make requester stop executing qp
> operation as soon as possible.
> 
> Both blktests and pyverbs tests are passed fine.
> 
> Bob Pearson (1):
>   RDMA/rxe: Split qp state for requester and completer
> 
> Li Zhijian (3):
>   RDMA/rxe: Update wqe_index for each wqe error completion
>   RDMA/rxe: Generate error completion for error requester QP state
>   RDMA/rxe: Fix typo in comment

Applied to for-next, thanks

Jason