2021-03-31 19:37:24

by Chuck Lever III

[permalink] [raw]
Subject: [PATCH v1 1/8] xprtrdma: Avoid Receive Queue wrapping

Commit e340c2d6ef2a ("xprtrdma: Reduce the doorbell rate (Receive)")
increased the number of Receive WRs that are posted by the client,
but did not increase the size of the Receive Queue allocated during
transport set-up.

This is usually not an issue because RPCRDMA_BACKWARD_WRS is defined
as (32) when SUNRPC_BACKCHANNEL is defined. In cases where it isn't,
there is a real risk of Receive Queue wrapping.

Fixes: e340c2d6ef2a ("xprtrdma: Reduce the doorbell rate (Receive)")
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/xprtrdma/frwr_ops.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c
index 766a1048a48a..132df9b59ab4 100644
--- a/net/sunrpc/xprtrdma/frwr_ops.c
+++ b/net/sunrpc/xprtrdma/frwr_ops.c
@@ -257,6 +257,7 @@ int frwr_query_device(struct rpcrdma_ep *ep, const struct ib_device *device)
ep->re_attr.cap.max_send_wr += 1; /* for ib_drain_sq */
ep->re_attr.cap.max_recv_wr = ep->re_max_requests;
ep->re_attr.cap.max_recv_wr += RPCRDMA_BACKWARD_WRS;
+ ep->re_attr.cap.max_recv_wr += RPCRDMA_MAX_RECV_BATCH;
ep->re_attr.cap.max_recv_wr += 1; /* for ib_drain_rq */

ep->re_max_rdma_segs =



2021-03-31 20:07:35

by Tom Talpey

[permalink] [raw]
Subject: Re: [PATCH v1 1/8] xprtrdma: Avoid Receive Queue wrapping

On 3/31/2021 3:36 PM, Chuck Lever wrote:
> Commit e340c2d6ef2a ("xprtrdma: Reduce the doorbell rate (Receive)")
> increased the number of Receive WRs that are posted by the client,
> but did not increase the size of the Receive Queue allocated during
> transport set-up.
>
> This is usually not an issue because RPCRDMA_BACKWARD_WRS is defined
> as (32) when SUNRPC_BACKCHANNEL is defined. In cases where it isn't,
> there is a real risk of Receive Queue wrapping.
>
> Fixes: e340c2d6ef2a ("xprtrdma: Reduce the doorbell rate (Receive)")
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> net/sunrpc/xprtrdma/frwr_ops.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c
> index 766a1048a48a..132df9b59ab4 100644
> --- a/net/sunrpc/xprtrdma/frwr_ops.c
> +++ b/net/sunrpc/xprtrdma/frwr_ops.c
> @@ -257,6 +257,7 @@ int frwr_query_device(struct rpcrdma_ep *ep, const struct ib_device *device)
> ep->re_attr.cap.max_send_wr += 1; /* for ib_drain_sq */
> ep->re_attr.cap.max_recv_wr = ep->re_max_requests;
> ep->re_attr.cap.max_recv_wr += RPCRDMA_BACKWARD_WRS;
> + ep->re_attr.cap.max_recv_wr += RPCRDMA_MAX_RECV_BATCH;

Naively, it seems to me this should be max(BACKWARD, BATCH).
But, extra WR slots are cheap and safe.

Reviewed-By: Tom Talpey <[email protected]>

> ep->re_attr.cap.max_recv_wr += 1; /* for ib_drain_rq */
>
> ep->re_max_rdma_segs =
>
>
>