Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
* [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	[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	[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	[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	[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	[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	[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	[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	[flat|nested] 9+ messages in thread

end of thread, back to index

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

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org
	public-inbox-index linux-nfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git