2021-02-03 20:10:44

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v3 0/6] RPC/RDMA client fixes

Changes since v2:
- Another minor optimization in rpcrdma_convert_kvec()
- Some patch description clarifications
- Add Reviewed-by (thanks Tom!)

Changes since v1:
- Respond to review comments
- Split "Remove FMR support" into three patches for clarity
- Fix implicit chunk roundup
- Improve Receive completion tracepoints

---

Chuck Lever (6):
xprtrdma: Remove FMR support in rpcrdma_convert_iovs()
xprtrdma: Simplify rpcrdma_convert_kvec() and frwr_map()
xprtrdma: Refactor invocations of offset_in_page()
rpcrdma: Fix comments about reverse-direction operation
xprtrdma: Pad optimization, revisited
rpcrdma: Capture bytes received in Receive completion tracepoints


include/trace/events/rpcrdma.h | 50 +++++++++++++++++++++-
net/sunrpc/xprtrdma/backchannel.c | 4 +-
net/sunrpc/xprtrdma/frwr_ops.c | 12 ++----
net/sunrpc/xprtrdma/rpc_rdma.c | 17 +++-----
net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 4 +-
net/sunrpc/xprtrdma/xprt_rdma.h | 15 ++++---
6 files changed, 68 insertions(+), 34 deletions(-)

--
Chuck Lever


2021-02-03 20:12:13

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v3 6/6] rpcrdma: Capture bytes received in Receive completion tracepoints

Make it easier to spot messages of an unusual size.

Signed-off-by: Chuck Lever <[email protected]>
Acked-by: Tom Talpey <[email protected]>
---
include/trace/events/rpcrdma.h | 50 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/include/trace/events/rpcrdma.h b/include/trace/events/rpcrdma.h
index 76e85e16854b..c838e7ac1c2d 100644
--- a/include/trace/events/rpcrdma.h
+++ b/include/trace/events/rpcrdma.h
@@ -60,6 +60,51 @@ DECLARE_EVENT_CLASS(rpcrdma_completion_class,
), \
TP_ARGS(wc, cid))

+DECLARE_EVENT_CLASS(rpcrdma_receive_completion_class,
+ TP_PROTO(
+ const struct ib_wc *wc,
+ const struct rpc_rdma_cid *cid
+ ),
+
+ TP_ARGS(wc, cid),
+
+ TP_STRUCT__entry(
+ __field(u32, cq_id)
+ __field(int, completion_id)
+ __field(u32, received)
+ __field(unsigned long, status)
+ __field(unsigned int, vendor_err)
+ ),
+
+ TP_fast_assign(
+ __entry->cq_id = cid->ci_queue_id;
+ __entry->completion_id = cid->ci_completion_id;
+ __entry->status = wc->status;
+ if (wc->status) {
+ __entry->received = 0;
+ __entry->vendor_err = wc->vendor_err;
+ } else {
+ __entry->received = wc->byte_len;
+ __entry->vendor_err = 0;
+ }
+ ),
+
+ TP_printk("cq.id=%u cid=%d status=%s (%lu/0x%x) received=%u",
+ __entry->cq_id, __entry->completion_id,
+ rdma_show_wc_status(__entry->status),
+ __entry->status, __entry->vendor_err,
+ __entry->received
+ )
+);
+
+#define DEFINE_RECEIVE_COMPLETION_EVENT(name) \
+ DEFINE_EVENT(rpcrdma_receive_completion_class, name, \
+ TP_PROTO( \
+ const struct ib_wc *wc, \
+ const struct rpc_rdma_cid *cid \
+ ), \
+ TP_ARGS(wc, cid))
+
DECLARE_EVENT_CLASS(xprtrdma_reply_class,
TP_PROTO(
const struct rpcrdma_rep *rep
@@ -838,7 +883,8 @@ TRACE_EVENT(xprtrdma_post_linv_err,
** Completion events
**/

-DEFINE_COMPLETION_EVENT(xprtrdma_wc_receive);
+DEFINE_RECEIVE_COMPLETION_EVENT(xprtrdma_wc_receive);
+
DEFINE_COMPLETION_EVENT(xprtrdma_wc_send);
DEFINE_COMPLETION_EVENT(xprtrdma_wc_fastreg);
DEFINE_COMPLETION_EVENT(xprtrdma_wc_li);
@@ -1790,7 +1836,7 @@ TRACE_EVENT(svcrdma_post_recv,
)
);

-DEFINE_COMPLETION_EVENT(svcrdma_wc_receive);
+DEFINE_RECEIVE_COMPLETION_EVENT(svcrdma_wc_receive);

TRACE_EVENT(svcrdma_rq_post_err,
TP_PROTO(


2021-02-03 21:28:43

by Tom Talpey

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] RPC/RDMA client fixes

On 2/3/2021 3:06 PM, Chuck Lever wrote:
> Changes since v2:
> - Another minor optimization in rpcrdma_convert_kvec()
> - Some patch description clarifications
> - Add Reviewed-by (thanks Tom!)
>
> Changes since v1:
> - Respond to review comments
> - Split "Remove FMR support" into three patches for clarity
> - Fix implicit chunk roundup
> - Improve Receive completion tracepoints
>
> ---
>
> Chuck Lever (6):
> xprtrdma: Remove FMR support in rpcrdma_convert_iovs()
> xprtrdma: Simplify rpcrdma_convert_kvec() and frwr_map()
> xprtrdma: Refactor invocations of offset_in_page()
> rpcrdma: Fix comments about reverse-direction operation
> xprtrdma: Pad optimization, revisited
> rpcrdma: Capture bytes received in Receive completion tracepoints
>
>
> include/trace/events/rpcrdma.h | 50 +++++++++++++++++++++-
> net/sunrpc/xprtrdma/backchannel.c | 4 +-
> net/sunrpc/xprtrdma/frwr_ops.c | 12 ++----
> net/sunrpc/xprtrdma/rpc_rdma.c | 17 +++-----

While reviewing the changes in rpc_rdma.c, I noticed a related minor
nit, which might be worth cleaning up for clarity.

Toward the end of rpcrdma_convert_iovs, there is no longer any
need to capture the returned value of rpcrdma_convert_kvec:

if (xdrbuf->tail[0].iov_len)
seg = rpcrdma_convert_kvec(&xdrbuf->tail[0], seg, &n);

^^^^^^ --> (void)?
out:
if (unlikely(n > RPCRDMA_MAX_SEGS))
return -EIO;
return n;

The two "goto out" statements just above it are getting kinda ugly
too, but...

Tom.


> net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 4 +-
> net/sunrpc/xprtrdma/xprt_rdma.h | 15 ++++---
> 6 files changed, 68 insertions(+), 34 deletions(-)
>
> --
> Chuck Lever
>
>