* [PATCH v2 0/8] Refactor path that sends error responses
@ 2020-06-29 14:49 Chuck Lever
2020-06-29 14:50 ` [PATCH v2 1/8] svcrdma: Fix page leak in svc_rdma_recv_read_chunk() Chuck Lever
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: Chuck Lever @ 2020-06-29 14:49 UTC (permalink / raw)
To: linux-nfs, linux-rdma
There are currently two paths in the server's RPC/RDMA implementation
for sending error responses. De-duplicate these two into one set of
helpers.
I mistakenly did not send the full set last week. The only change
since v1 of this series is two additional patches at the end of the
series.
---
Chuck Lever (8):
svcrdma: Fix page leak in svc_rdma_recv_read_chunk()
svcrdma: Remove save_io_pages() call from send_error_msg()
svcrdma: Add @rctxt parameter to svc_rdma_send_error() functions
svcrdma: Add a @status parameter to svc_rdma_send_error_msg()
svcrdma: Eliminate return value for svc_rdma_send_error_msg()
svcrdma: Make svc_rdma_send_error_msg() a global function
svcrdma: Consolidate send_error helper functions
svcrdma: Clean up trace_svcrdma_send_failed() tracepoint
include/linux/sunrpc/svc_rdma.h | 4 ++
include/trace/events/rpcrdma.h | 7 +-
net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 59 +++--------------
net/sunrpc/xprtrdma/svc_rdma_sendto.c | 87 +++++++++++++++++--------
4 files changed, 75 insertions(+), 82 deletions(-)
--
Chuck Lever
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/8] svcrdma: Fix page leak in svc_rdma_recv_read_chunk()
2020-06-29 14:49 [PATCH v2 0/8] Refactor path that sends error responses Chuck Lever
@ 2020-06-29 14:50 ` Chuck Lever
2020-06-29 14:50 ` [PATCH v2 2/8] svcrdma: Remove save_io_pages() call from send_error_msg() Chuck Lever
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-06-29 14:50 UTC (permalink / raw)
To: linux-nfs, linux-rdma
Commit 07d0ff3b0cd2 ("svcrdma: Clean up Read chunk path") moved the
page saver logic so that it gets executed event when an error occurs.
In that case, the I/O is never posted, and those pages are then
leaked. Errors in this path, however, are quite rare.
Fixes: 07d0ff3b0cd2 ("svcrdma: Clean up Read chunk path")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
net/sunrpc/xprtrdma/svc_rdma_rw.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/net/sunrpc/xprtrdma/svc_rdma_rw.c b/net/sunrpc/xprtrdma/svc_rdma_rw.c
index 5eb35309ecef..83806fa94def 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_rw.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_rw.c
@@ -684,7 +684,6 @@ static int svc_rdma_build_read_chunk(struct svc_rqst *rqstp,
struct svc_rdma_read_info *info,
__be32 *p)
{
- unsigned int i;
int ret;
ret = -EINVAL;
@@ -707,12 +706,6 @@ static int svc_rdma_build_read_chunk(struct svc_rqst *rqstp,
info->ri_chunklen += rs_length;
}
- /* Pages under I/O have been copied to head->rc_pages.
- * Prevent their premature release by svc_xprt_release() .
- */
- for (i = 0; i < info->ri_readctxt->rc_page_count; i++)
- rqstp->rq_pages[i] = NULL;
-
return ret;
}
@@ -807,6 +800,26 @@ static int svc_rdma_build_pz_read_chunk(struct svc_rqst *rqstp,
return ret;
}
+/* Pages under I/O have been copied to head->rc_pages. Ensure they
+ * are not released by svc_xprt_release() until the I/O is complete.
+ *
+ * This has to be done after all Read WRs are constructed to properly
+ * handle a page that is part of I/O on behalf of two different RDMA
+ * segments.
+ *
+ * Do this only if I/O has been posted. Otherwise, we do indeed want
+ * svc_xprt_release() to clean things up properly.
+ */
+static void svc_rdma_save_io_pages(struct svc_rqst *rqstp,
+ const unsigned int start,
+ const unsigned int num_pages)
+{
+ unsigned int i;
+
+ for (i = start; i < num_pages + start; i++)
+ rqstp->rq_pages[i] = NULL;
+}
+
/**
* svc_rdma_recv_read_chunk - Pull a Read chunk from the client
* @rdma: controlling RDMA transport
@@ -860,6 +873,7 @@ int svc_rdma_recv_read_chunk(struct svcxprt_rdma *rdma, struct svc_rqst *rqstp,
ret = svc_rdma_post_chunk_ctxt(&info->ri_cc);
if (ret < 0)
goto out_err;
+ svc_rdma_save_io_pages(rqstp, 0, head->rc_page_count);
return 0;
out_err:
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/8] svcrdma: Remove save_io_pages() call from send_error_msg()
2020-06-29 14:49 [PATCH v2 0/8] Refactor path that sends error responses Chuck Lever
2020-06-29 14:50 ` [PATCH v2 1/8] svcrdma: Fix page leak in svc_rdma_recv_read_chunk() Chuck Lever
@ 2020-06-29 14:50 ` Chuck Lever
2020-06-29 14:50 ` [PATCH v2 3/8] svcrdma: Add @rctxt parameter to svc_rdma_send_error() functions Chuck Lever
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-06-29 14:50 UTC (permalink / raw)
To: linux-nfs, linux-rdma
Commit 4757d90b15d8 ("svcrdma: Report Write/Reply chunk overruns")
made an effort to preserve I/O pages until RDMA Write completion.
In a subsequent patch, I intend to de-duplicate the two functions
that send ERR_CHUNK responses. Pull the save_io_pages() call out of
svc_rdma_send_error_msg() to make it more like
svc_rdma_send_error().
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
net/sunrpc/xprtrdma/svc_rdma_sendto.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
index 38e7c3c8c4a9..2f88d01e8d27 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
@@ -806,8 +806,7 @@ static int svc_rdma_send_reply_msg(struct svcxprt_rdma *rdma,
/* Given the client-provided Write and Reply chunks, the server was not
* able to form a complete reply. Return an RDMA_ERROR message so the
- * client can retire this RPC transaction. As above, the Send completion
- * routine releases payload pages that were part of a previous RDMA Write.
+ * client can retire this RPC transaction.
*
* Remote Invalidation is skipped for simplicity.
*/
@@ -834,8 +833,6 @@ static int svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
*p = err_chunk;
trace_svcrdma_err_chunk(*rdma_argp);
- svc_rdma_save_io_pages(rqstp, ctxt);
-
ctxt->sc_send_wr.num_sge = 1;
ctxt->sc_send_wr.opcode = IB_WR_SEND;
ctxt->sc_sges[0].length = ctxt->sc_hdrbuf.len;
@@ -930,6 +927,10 @@ int svc_rdma_sendto(struct svc_rqst *rqstp)
if (ret != -E2BIG && ret != -EINVAL)
goto err1;
+ /* Send completion releases payload pages that were part
+ * of previously posted RDMA Writes.
+ */
+ svc_rdma_save_io_pages(rqstp, sctxt);
ret = svc_rdma_send_error_msg(rdma, sctxt, rqstp);
if (ret < 0)
goto err1;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/8] svcrdma: Add @rctxt parameter to svc_rdma_send_error() functions
2020-06-29 14:49 [PATCH v2 0/8] Refactor path that sends error responses Chuck Lever
2020-06-29 14:50 ` [PATCH v2 1/8] svcrdma: Fix page leak in svc_rdma_recv_read_chunk() Chuck Lever
2020-06-29 14:50 ` [PATCH v2 2/8] svcrdma: Remove save_io_pages() call from send_error_msg() Chuck Lever
@ 2020-06-29 14:50 ` Chuck Lever
2020-06-29 14:50 ` [PATCH v2 4/8] svcrdma: Add a @status parameter to svc_rdma_send_error_msg() Chuck Lever
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-06-29 14:50 UTC (permalink / raw)
To: linux-nfs, linux-rdma
Another step towards making svc_rdma_send_error_msg() and
svc_rdma_send_error() similar enough to eliminate one of them.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 9 +++++----
net/sunrpc/xprtrdma/svc_rdma_sendto.c | 23 +++++++++++------------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
index e426fedb9524..60d855116ae7 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
@@ -715,10 +715,11 @@ static void rdma_read_complete(struct svc_rqst *rqstp,
}
static void svc_rdma_send_error(struct svcxprt_rdma *xprt,
- __be32 *rdma_argp, int status)
+ struct svc_rdma_recv_ctxt *rctxt,
+ int status)
{
+ __be32 *p, *rdma_argp = rctxt->rc_recv_buf;
struct svc_rdma_send_ctxt *ctxt;
- __be32 *p;
int ret;
ctxt = svc_rdma_send_ctxt_get(xprt);
@@ -900,13 +901,13 @@ int svc_rdma_recvfrom(struct svc_rqst *rqstp)
return 0;
out_err:
- svc_rdma_send_error(rdma_xprt, p, ret);
+ svc_rdma_send_error(rdma_xprt, ctxt, ret);
svc_rdma_recv_ctxt_put(rdma_xprt, ctxt);
return 0;
out_postfail:
if (ret == -EINVAL)
- svc_rdma_send_error(rdma_xprt, p, ret);
+ svc_rdma_send_error(rdma_xprt, ctxt, ret);
svc_rdma_recv_ctxt_put(rdma_xprt, ctxt);
return ret;
diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
index 2f88d01e8d27..47ada61411c3 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
@@ -811,18 +811,17 @@ static int svc_rdma_send_reply_msg(struct svcxprt_rdma *rdma,
* Remote Invalidation is skipped for simplicity.
*/
static int svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
- struct svc_rdma_send_ctxt *ctxt,
- struct svc_rqst *rqstp)
+ struct svc_rdma_send_ctxt *sctxt,
+ struct svc_rdma_recv_ctxt *rctxt)
{
- struct svc_rdma_recv_ctxt *rctxt = rqstp->rq_xprt_ctxt;
__be32 *rdma_argp = rctxt->rc_recv_buf;
__be32 *p;
- rpcrdma_set_xdrlen(&ctxt->sc_hdrbuf, 0);
- xdr_init_encode(&ctxt->sc_stream, &ctxt->sc_hdrbuf, ctxt->sc_xprt_buf,
- NULL);
+ rpcrdma_set_xdrlen(&sctxt->sc_hdrbuf, 0);
+ xdr_init_encode(&sctxt->sc_stream, &sctxt->sc_hdrbuf,
+ sctxt->sc_xprt_buf, NULL);
- p = xdr_reserve_space(&ctxt->sc_stream, RPCRDMA_HDRLEN_ERR);
+ p = xdr_reserve_space(&sctxt->sc_stream, RPCRDMA_HDRLEN_ERR);
if (!p)
return -ENOMSG;
@@ -833,10 +832,10 @@ static int svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
*p = err_chunk;
trace_svcrdma_err_chunk(*rdma_argp);
- ctxt->sc_send_wr.num_sge = 1;
- ctxt->sc_send_wr.opcode = IB_WR_SEND;
- ctxt->sc_sges[0].length = ctxt->sc_hdrbuf.len;
- return svc_rdma_send(rdma, &ctxt->sc_send_wr);
+ sctxt->sc_send_wr.num_sge = 1;
+ sctxt->sc_send_wr.opcode = IB_WR_SEND;
+ sctxt->sc_sges[0].length = sctxt->sc_hdrbuf.len;
+ return svc_rdma_send(rdma, &sctxt->sc_send_wr);
}
/**
@@ -931,7 +930,7 @@ int svc_rdma_sendto(struct svc_rqst *rqstp)
* of previously posted RDMA Writes.
*/
svc_rdma_save_io_pages(rqstp, sctxt);
- ret = svc_rdma_send_error_msg(rdma, sctxt, rqstp);
+ ret = svc_rdma_send_error_msg(rdma, sctxt, rctxt);
if (ret < 0)
goto err1;
return 0;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 4/8] svcrdma: Add a @status parameter to svc_rdma_send_error_msg()
2020-06-29 14:49 [PATCH v2 0/8] Refactor path that sends error responses Chuck Lever
` (2 preceding siblings ...)
2020-06-29 14:50 ` [PATCH v2 3/8] svcrdma: Add @rctxt parameter to svc_rdma_send_error() functions Chuck Lever
@ 2020-06-29 14:50 ` Chuck Lever
2020-06-29 14:50 ` [PATCH v2 5/8] svcrdma: Eliminate return value for svc_rdma_send_error_msg() Chuck Lever
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-06-29 14:50 UTC (permalink / raw)
To: linux-nfs, linux-rdma
The common "send RDMA_ERR" function should be in svc_rdma_sendto.c,
since that is where the other Send-related functions are located.
So from here, I will beef up svc_rdma_send_error_msg() and deprecate
svc_rdma_send_error().
A generic svc_rdma_send_error_msg() will need to handle both
ERR_CHUNK and ERR_VERS. Copy that logic from svc_rdma_send_error()
to svc_rdma_send_error_msg().
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
net/sunrpc/xprtrdma/svc_rdma_sendto.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
index 47ada61411c3..73fe7a213169 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
@@ -812,7 +812,8 @@ static int svc_rdma_send_reply_msg(struct svcxprt_rdma *rdma,
*/
static int svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
struct svc_rdma_send_ctxt *sctxt,
- struct svc_rdma_recv_ctxt *rctxt)
+ struct svc_rdma_recv_ctxt *rctxt,
+ int status)
{
__be32 *rdma_argp = rctxt->rc_recv_buf;
__be32 *p;
@@ -821,16 +822,35 @@ static int svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
xdr_init_encode(&sctxt->sc_stream, &sctxt->sc_hdrbuf,
sctxt->sc_xprt_buf, NULL);
- p = xdr_reserve_space(&sctxt->sc_stream, RPCRDMA_HDRLEN_ERR);
+ p = xdr_reserve_space(&sctxt->sc_stream,
+ rpcrdma_fixed_maxsz * sizeof(*p));
if (!p)
return -ENOMSG;
*p++ = *rdma_argp;
*p++ = *(rdma_argp + 1);
*p++ = rdma->sc_fc_credits;
- *p++ = rdma_error;
- *p = err_chunk;
- trace_svcrdma_err_chunk(*rdma_argp);
+ *p = rdma_error;
+
+ switch (status) {
+ case -EPROTONOSUPPORT:
+ p = xdr_reserve_space(&sctxt->sc_stream, 3 * sizeof(*p));
+ if (!p)
+ return -ENOMSG;
+
+ *p++ = err_vers;
+ *p++ = rpcrdma_version;
+ *p = rpcrdma_version;
+ trace_svcrdma_err_vers(*rdma_argp);
+ break;
+ default:
+ p = xdr_reserve_space(&sctxt->sc_stream, sizeof(*p));
+ if (!p)
+ return -ENOMSG;
+
+ *p = err_chunk;
+ trace_svcrdma_err_chunk(*rdma_argp);
+ }
sctxt->sc_send_wr.num_sge = 1;
sctxt->sc_send_wr.opcode = IB_WR_SEND;
@@ -930,7 +950,7 @@ int svc_rdma_sendto(struct svc_rqst *rqstp)
* of previously posted RDMA Writes.
*/
svc_rdma_save_io_pages(rqstp, sctxt);
- ret = svc_rdma_send_error_msg(rdma, sctxt, rctxt);
+ ret = svc_rdma_send_error_msg(rdma, sctxt, rctxt, ret);
if (ret < 0)
goto err1;
return 0;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 5/8] svcrdma: Eliminate return value for svc_rdma_send_error_msg()
2020-06-29 14:49 [PATCH v2 0/8] Refactor path that sends error responses Chuck Lever
` (3 preceding siblings ...)
2020-06-29 14:50 ` [PATCH v2 4/8] svcrdma: Add a @status parameter to svc_rdma_send_error_msg() Chuck Lever
@ 2020-06-29 14:50 ` Chuck Lever
2020-06-29 14:50 ` [PATCH v2 6/8] svcrdma: Make svc_rdma_send_error_msg() a global function Chuck Lever
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-06-29 14:50 UTC (permalink / raw)
To: linux-nfs, linux-rdma
Like svc_rdma_send_error(), have svc_rdma_send_error_msg() handle
any error conditions internally, rather than duplicating that
recovery logic at every call site.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
net/sunrpc/xprtrdma/svc_rdma_sendto.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
index 73fe7a213169..fb548b548c4b 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
@@ -810,10 +810,10 @@ static int svc_rdma_send_reply_msg(struct svcxprt_rdma *rdma,
*
* Remote Invalidation is skipped for simplicity.
*/
-static int svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
- struct svc_rdma_send_ctxt *sctxt,
- struct svc_rdma_recv_ctxt *rctxt,
- int status)
+static void svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
+ struct svc_rdma_send_ctxt *sctxt,
+ struct svc_rdma_recv_ctxt *rctxt,
+ int status)
{
__be32 *rdma_argp = rctxt->rc_recv_buf;
__be32 *p;
@@ -825,7 +825,7 @@ static int svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
p = xdr_reserve_space(&sctxt->sc_stream,
rpcrdma_fixed_maxsz * sizeof(*p));
if (!p)
- return -ENOMSG;
+ goto put_ctxt;
*p++ = *rdma_argp;
*p++ = *(rdma_argp + 1);
@@ -836,7 +836,7 @@ static int svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
case -EPROTONOSUPPORT:
p = xdr_reserve_space(&sctxt->sc_stream, 3 * sizeof(*p));
if (!p)
- return -ENOMSG;
+ goto put_ctxt;
*p++ = err_vers;
*p++ = rpcrdma_version;
@@ -846,7 +846,7 @@ static int svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
default:
p = xdr_reserve_space(&sctxt->sc_stream, sizeof(*p));
if (!p)
- return -ENOMSG;
+ goto put_ctxt;
*p = err_chunk;
trace_svcrdma_err_chunk(*rdma_argp);
@@ -855,7 +855,12 @@ static int svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
sctxt->sc_send_wr.num_sge = 1;
sctxt->sc_send_wr.opcode = IB_WR_SEND;
sctxt->sc_sges[0].length = sctxt->sc_hdrbuf.len;
- return svc_rdma_send(rdma, &sctxt->sc_send_wr);
+ if (svc_rdma_send(rdma, &sctxt->sc_send_wr))
+ goto put_ctxt;
+ return;
+
+put_ctxt:
+ svc_rdma_send_ctxt_put(rdma, sctxt);
}
/**
@@ -950,9 +955,7 @@ int svc_rdma_sendto(struct svc_rqst *rqstp)
* of previously posted RDMA Writes.
*/
svc_rdma_save_io_pages(rqstp, sctxt);
- ret = svc_rdma_send_error_msg(rdma, sctxt, rctxt, ret);
- if (ret < 0)
- goto err1;
+ svc_rdma_send_error_msg(rdma, sctxt, rctxt, ret);
return 0;
err1:
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 6/8] svcrdma: Make svc_rdma_send_error_msg() a global function
2020-06-29 14:49 [PATCH v2 0/8] Refactor path that sends error responses Chuck Lever
` (4 preceding siblings ...)
2020-06-29 14:50 ` [PATCH v2 5/8] svcrdma: Eliminate return value for svc_rdma_send_error_msg() Chuck Lever
@ 2020-06-29 14:50 ` Chuck Lever
2020-06-29 14:50 ` [PATCH v2 7/8] svcrdma: Consolidate send_error helper functions Chuck Lever
2020-06-29 14:50 ` [PATCH v2 8/8] svcrdma: Clean up trace_svcrdma_send_failed() tracepoint Chuck Lever
7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-06-29 14:50 UTC (permalink / raw)
To: linux-nfs, linux-rdma
Prepare for svc_rdma_send_error_msg() to be invoked from another
source file.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
include/linux/sunrpc/svc_rdma.h | 4 ++++
net/sunrpc/xprtrdma/svc_rdma_sendto.c | 28 +++++++++++++++++++---------
2 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
index 7ed82625dc0b..1579f7a14ab4 100644
--- a/include/linux/sunrpc/svc_rdma.h
+++ b/include/linux/sunrpc/svc_rdma.h
@@ -195,6 +195,10 @@ extern int svc_rdma_map_reply_msg(struct svcxprt_rdma *rdma,
struct svc_rdma_send_ctxt *sctxt,
const struct svc_rdma_recv_ctxt *rctxt,
struct xdr_buf *xdr);
+extern void svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
+ struct svc_rdma_send_ctxt *sctxt,
+ struct svc_rdma_recv_ctxt *rctxt,
+ int status);
extern int svc_rdma_sendto(struct svc_rqst *);
extern int svc_rdma_read_payload(struct svc_rqst *rqstp, unsigned int offset,
unsigned int length);
diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
index fb548b548c4b..57041298fe4f 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
@@ -804,16 +804,25 @@ static int svc_rdma_send_reply_msg(struct svcxprt_rdma *rdma,
return svc_rdma_send(rdma, &sctxt->sc_send_wr);
}
-/* Given the client-provided Write and Reply chunks, the server was not
- * able to form a complete reply. Return an RDMA_ERROR message so the
- * client can retire this RPC transaction.
- *
- * Remote Invalidation is skipped for simplicity.
+/**
+ * svc_rdma_send_error_msg - Send an RPC/RDMA v1 error response
+ * @rdma: controlling transport context
+ * @sctxt: Send context for the response
+ * @rctxt: Receive context for incoming bad message
+ * @status: negative errno indicating error that occurred
+ *
+ * Given the client-provided Read, Write, and Reply chunks, the
+ * server was not able to parse the Call or form a complete Reply.
+ * Return an RDMA_ERROR message so the client can retire the RPC
+ * transaction.
+ *
+ * The caller does not have to release @sctxt. It is released by
+ * Send completion, or by this function on error.
*/
-static void svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
- struct svc_rdma_send_ctxt *sctxt,
- struct svc_rdma_recv_ctxt *rctxt,
- int status)
+void svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
+ struct svc_rdma_send_ctxt *sctxt,
+ struct svc_rdma_recv_ctxt *rctxt,
+ int status)
{
__be32 *rdma_argp = rctxt->rc_recv_buf;
__be32 *p;
@@ -852,6 +861,7 @@ static void svc_rdma_send_error_msg(struct svcxprt_rdma *rdma,
trace_svcrdma_err_chunk(*rdma_argp);
}
+ /* Remote Invalidation is skipped for simplicity. */
sctxt->sc_send_wr.num_sge = 1;
sctxt->sc_send_wr.opcode = IB_WR_SEND;
sctxt->sc_sges[0].length = sctxt->sc_hdrbuf.len;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 7/8] svcrdma: Consolidate send_error helper functions
2020-06-29 14:49 [PATCH v2 0/8] Refactor path that sends error responses Chuck Lever
` (5 preceding siblings ...)
2020-06-29 14:50 ` [PATCH v2 6/8] svcrdma: Make svc_rdma_send_error_msg() a global function Chuck Lever
@ 2020-06-29 14:50 ` Chuck Lever
2020-06-29 14:50 ` [PATCH v2 8/8] svcrdma: Clean up trace_svcrdma_send_failed() tracepoint Chuck Lever
7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-06-29 14:50 UTC (permalink / raw)
To: linux-nfs, linux-rdma
Final refactor: Replace internals of svc_rdma_send_error() with a
simple call to svc_rdma_send_error_msg().
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 52 +++----------------------------
1 file changed, 5 insertions(+), 47 deletions(-)
diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
index 60d855116ae7..c072ce61b393 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
@@ -714,58 +714,16 @@ static void rdma_read_complete(struct svc_rqst *rqstp,
rqstp->rq_arg.buflen = head->rc_arg.buflen;
}
-static void svc_rdma_send_error(struct svcxprt_rdma *xprt,
+static void svc_rdma_send_error(struct svcxprt_rdma *rdma,
struct svc_rdma_recv_ctxt *rctxt,
int status)
{
- __be32 *p, *rdma_argp = rctxt->rc_recv_buf;
- struct svc_rdma_send_ctxt *ctxt;
- int ret;
+ struct svc_rdma_send_ctxt *sctxt;
- ctxt = svc_rdma_send_ctxt_get(xprt);
- if (!ctxt)
+ sctxt = svc_rdma_send_ctxt_get(rdma);
+ if (!sctxt)
return;
-
- p = xdr_reserve_space(&ctxt->sc_stream,
- rpcrdma_fixed_maxsz * sizeof(*p));
- if (!p)
- goto put_ctxt;
-
- *p++ = *rdma_argp;
- *p++ = *(rdma_argp + 1);
- *p++ = xprt->sc_fc_credits;
- *p = rdma_error;
-
- switch (status) {
- case -EPROTONOSUPPORT:
- p = xdr_reserve_space(&ctxt->sc_stream, 3 * sizeof(*p));
- if (!p)
- goto put_ctxt;
-
- *p++ = err_vers;
- *p++ = rpcrdma_version;
- *p = rpcrdma_version;
- trace_svcrdma_err_vers(*rdma_argp);
- break;
- default:
- p = xdr_reserve_space(&ctxt->sc_stream, sizeof(*p));
- if (!p)
- goto put_ctxt;
-
- *p = err_chunk;
- trace_svcrdma_err_chunk(*rdma_argp);
- }
-
- ctxt->sc_send_wr.num_sge = 1;
- ctxt->sc_send_wr.opcode = IB_WR_SEND;
- ctxt->sc_sges[0].length = ctxt->sc_hdrbuf.len;
- ret = svc_rdma_send(xprt, &ctxt->sc_send_wr);
- if (ret)
- goto put_ctxt;
- return;
-
-put_ctxt:
- svc_rdma_send_ctxt_put(xprt, ctxt);
+ svc_rdma_send_error_msg(rdma, sctxt, rctxt, status);
}
/* By convention, backchannel calls arrive via rdma_msg type
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 8/8] svcrdma: Clean up trace_svcrdma_send_failed() tracepoint
2020-06-29 14:49 [PATCH v2 0/8] Refactor path that sends error responses Chuck Lever
` (6 preceding siblings ...)
2020-06-29 14:50 ` [PATCH v2 7/8] svcrdma: Consolidate send_error helper functions Chuck Lever
@ 2020-06-29 14:50 ` Chuck Lever
7 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-06-29 14:50 UTC (permalink / raw)
To: linux-nfs, linux-rdma
- Use the _err naming convention instead
- Remove display of kernel memory address of the controlling xprt
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
include/trace/events/rpcrdma.h | 7 ++-----
net/sunrpc/xprtrdma/svc_rdma_sendto.c | 2 +-
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/include/trace/events/rpcrdma.h b/include/trace/events/rpcrdma.h
index 0f05a6e2b9cb..0eff80dee066 100644
--- a/include/trace/events/rpcrdma.h
+++ b/include/trace/events/rpcrdma.h
@@ -1716,7 +1716,7 @@ TRACE_EVENT(svcrdma_send_pullup,
TP_printk("len=%u", __entry->len)
);
-TRACE_EVENT(svcrdma_send_failed,
+TRACE_EVENT(svcrdma_send_err,
TP_PROTO(
const struct svc_rqst *rqst,
int status
@@ -1727,19 +1727,16 @@ TRACE_EVENT(svcrdma_send_failed,
TP_STRUCT__entry(
__field(int, status)
__field(u32, xid)
- __field(const void *, xprt)
__string(addr, rqst->rq_xprt->xpt_remotebuf)
),
TP_fast_assign(
__entry->status = status;
__entry->xid = __be32_to_cpu(rqst->rq_xid);
- __entry->xprt = rqst->rq_xprt;
__assign_str(addr, rqst->rq_xprt->xpt_remotebuf);
),
- TP_printk("xprt=%p addr=%s xid=0x%08x status=%d",
- __entry->xprt, __get_str(addr),
+ TP_printk("addr=%s xid=0x%08x status=%d", __get_str(addr),
__entry->xid, __entry->status
)
);
diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
index 57041298fe4f..f985f548346a 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c
@@ -971,7 +971,7 @@ int svc_rdma_sendto(struct svc_rqst *rqstp)
err1:
svc_rdma_send_ctxt_put(rdma, sctxt);
err0:
- trace_svcrdma_send_failed(rqstp, ret);
+ trace_svcrdma_send_err(rqstp, ret);
set_bit(XPT_CLOSE, &xprt->xpt_flags);
return -ENOTCONN;
}
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-06-29 19:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-29 14:49 [PATCH v2 0/8] Refactor path that sends error responses Chuck Lever
2020-06-29 14:50 ` [PATCH v2 1/8] svcrdma: Fix page leak in svc_rdma_recv_read_chunk() Chuck Lever
2020-06-29 14:50 ` [PATCH v2 2/8] svcrdma: Remove save_io_pages() call from send_error_msg() Chuck Lever
2020-06-29 14:50 ` [PATCH v2 3/8] svcrdma: Add @rctxt parameter to svc_rdma_send_error() functions Chuck Lever
2020-06-29 14:50 ` [PATCH v2 4/8] svcrdma: Add a @status parameter to svc_rdma_send_error_msg() Chuck Lever
2020-06-29 14:50 ` [PATCH v2 5/8] svcrdma: Eliminate return value for svc_rdma_send_error_msg() Chuck Lever
2020-06-29 14:50 ` [PATCH v2 6/8] svcrdma: Make svc_rdma_send_error_msg() a global function Chuck Lever
2020-06-29 14:50 ` [PATCH v2 7/8] svcrdma: Consolidate send_error helper functions Chuck Lever
2020-06-29 14:50 ` [PATCH v2 8/8] svcrdma: Clean up trace_svcrdma_send_failed() tracepoint Chuck Lever
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).