2021-08-30 16:53:40

by Olga Kornievskaia

[permalink] [raw]
Subject: [RFC 0/2] revisit RMDA XDR padding management

From: Olga Kornievskaia <[email protected]>

We have previously revisited how XDR padding management was done
for the RDMA read chunks.

This patch series attempts to do the same for the RDMA read chunks
and altogether remove the options of doing an implicit roundup.

According to the RFC 8166 client "SHOULD NOT" include additional
space for XDR roundup padding. Furthermore, server MUST NOT write
XDR padding into the a write chunk. Given those two statement and
a patch "NFS: Always provide aligned buffers to the RPC read layers",
there is no reason for the client to look at the tail and assume
there is any padding data for which it needs to allocate space for.

The only operation that this patch series effects is an NFS read.
All non-read ops that might use a write chunk are setup to use
reply chunk if reply is larger than inline threshold, not a write
chunk.



*** SUBJECT HERE ***

*** BLURB HERE ***

Olga Kornievskaia (2):
xprtrdma: xdr pad optimization revisted again
xprtrdma: remove re_implicit_roundup xprt_rdma_pad_optimize

net/sunrpc/xprtrdma/rpc_rdma.c | 15 ---------------
net/sunrpc/xprtrdma/transport.c | 8 --------
net/sunrpc/xprtrdma/verbs.c | 2 --
net/sunrpc/xprtrdma/xprt_rdma.h | 6 ------
4 files changed, 31 deletions(-)

--
2.27.0


2021-08-30 16:53:43

by Olga Kornievskaia

[permalink] [raw]
Subject: [RFC 2/2] xprtrdma: remove re_implicit_roundup xprt_rdma_pad_optimize

From: Olga Kornievskaia <[email protected]>

Since RPC over RDMA layer no longer needs to manage XDR padding,
remove existing code that managed whether or not client included
extra space for the XDR padding.

Signed-off-by: Olga Kornievskaia <[email protected]>
---
net/sunrpc/xprtrdma/transport.c | 8 --------
net/sunrpc/xprtrdma/verbs.c | 2 --
net/sunrpc/xprtrdma/xprt_rdma.h | 6 ------
3 files changed, 16 deletions(-)

diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
index 16e5696314a4..e7b9d88f4483 100644
--- a/net/sunrpc/xprtrdma/transport.c
+++ b/net/sunrpc/xprtrdma/transport.c
@@ -72,7 +72,6 @@ static unsigned int xprt_rdma_slot_table_entries = RPCRDMA_DEF_SLOT_TABLE;
unsigned int xprt_rdma_max_inline_read = RPCRDMA_DEF_INLINE;
unsigned int xprt_rdma_max_inline_write = RPCRDMA_DEF_INLINE;
unsigned int xprt_rdma_memreg_strategy = RPCRDMA_FRWR;
-int xprt_rdma_pad_optimize;
static struct xprt_class xprt_rdma;

#if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
@@ -134,13 +133,6 @@ static struct ctl_table xr_tunables_table[] = {
.extra1 = &min_memreg,
.extra2 = &max_memreg,
},
- {
- .procname = "rdma_pad_optimize",
- .data = &xprt_rdma_pad_optimize,
- .maxlen = sizeof(unsigned int),
- .mode = 0644,
- .proc_handler = proc_dointvec,
- },
{ },
};

diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index aaec3c9be8db..d8650a3563ef 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -205,14 +205,12 @@ static void rpcrdma_update_cm_private(struct rpcrdma_ep *ep,
unsigned int rsize, wsize;

/* Default settings for RPC-over-RDMA Version One */
- ep->re_implicit_roundup = xprt_rdma_pad_optimize;
rsize = RPCRDMA_V1_DEF_INLINE_SIZE;
wsize = RPCRDMA_V1_DEF_INLINE_SIZE;

if (pmsg &&
pmsg->cp_magic == rpcrdma_cmp_magic &&
pmsg->cp_version == RPCRDMA_CMP_VERSION) {
- ep->re_implicit_roundup = true;
rsize = rpcrdma_decode_buffer_size(pmsg->cp_send_size);
wsize = rpcrdma_decode_buffer_size(pmsg->cp_recv_size);
}
diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
index d91f54eae00b..137866a83a3a 100644
--- a/net/sunrpc/xprtrdma/xprt_rdma.h
+++ b/net/sunrpc/xprtrdma/xprt_rdma.h
@@ -74,7 +74,6 @@ struct rpcrdma_ep {
struct ib_pd *re_pd;
unsigned int re_max_rdma_segs;
unsigned int re_max_fr_depth;
- bool re_implicit_roundup;
enum ib_mr_type re_mrtype;
struct completion re_done;
unsigned int re_send_count;
@@ -441,11 +440,6 @@ rpcrdma_portstr(const struct rpcrdma_xprt *r_xprt)
return r_xprt->rx_xprt.address_strings[RPC_DISPLAY_PORT];
}

-/* Setting this to 0 ensures interoperability with early servers.
- * Setting this to 1 enhances certain unaligned read/write performance.
- * Default is 0, see sysctl entry and rpc_rdma.c rpcrdma_convert_iovs() */
-extern int xprt_rdma_pad_optimize;
-
/* This setting controls the hunt for a supported memory
* registration strategy.
*/
--
2.27.0

2021-08-30 16:56:22

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [RFC 0/2] revisit RMDA XDR padding management

On Mon, Aug 30, 2021 at 12:53 PM Olga Kornievskaia
<[email protected]> wrote:
>
> From: Olga Kornievskaia <[email protected]>
>
> We have previously revisited how XDR padding management was done
> for the RDMA read chunks.
>
> This patch series attempts to do the same for the RDMA read chunks
> and altogether remove the options of doing an implicit roundup.

Apologies for typos but this is about RDMA write chunks.

> According to the RFC 8166 client "SHOULD NOT" include additional
> space for XDR roundup padding. Furthermore, server MUST NOT write
> XDR padding into the a write chunk. Given those two statement and
> a patch "NFS: Always provide aligned buffers to the RPC read layers",
> there is no reason for the client to look at the tail and assume
> there is any padding data for which it needs to allocate space for.
>
> The only operation that this patch series effects is an NFS read.
> All non-read ops that might use a write chunk are setup to use
> reply chunk if reply is larger than inline threshold, not a write
> chunk.
>
>
>
> *** SUBJECT HERE ***
>
> *** BLURB HERE ***
>
> Olga Kornievskaia (2):
> xprtrdma: xdr pad optimization revisted again
> xprtrdma: remove re_implicit_roundup xprt_rdma_pad_optimize
>
> net/sunrpc/xprtrdma/rpc_rdma.c | 15 ---------------
> net/sunrpc/xprtrdma/transport.c | 8 --------
> net/sunrpc/xprtrdma/verbs.c | 2 --
> net/sunrpc/xprtrdma/xprt_rdma.h | 6 ------
> 4 files changed, 31 deletions(-)
>
> --
> 2.27.0
>

2021-08-30 16:57:49

by Chuck Lever

[permalink] [raw]
Subject: Re: [RFC 2/2] xprtrdma: remove re_implicit_roundup xprt_rdma_pad_optimize



> On Aug 30, 2021, at 12:53 PM, Olga Kornievskaia <[email protected]> wrote:
>
> From: Olga Kornievskaia <[email protected]>
>
> Since RPC over RDMA layer no longer needs to manage XDR padding,
> remove existing code that managed whether or not client included
> extra space for the XDR padding.
>
> Signed-off-by: Olga Kornievskaia <[email protected]>
> ---
> net/sunrpc/xprtrdma/transport.c | 8 --------
> net/sunrpc/xprtrdma/verbs.c | 2 --
> net/sunrpc/xprtrdma/xprt_rdma.h | 6 ------
> 3 files changed, 16 deletions(-)
>
> diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
> index 16e5696314a4..e7b9d88f4483 100644
> --- a/net/sunrpc/xprtrdma/transport.c
> +++ b/net/sunrpc/xprtrdma/transport.c
> @@ -72,7 +72,6 @@ static unsigned int xprt_rdma_slot_table_entries = RPCRDMA_DEF_SLOT_TABLE;
> unsigned int xprt_rdma_max_inline_read = RPCRDMA_DEF_INLINE;
> unsigned int xprt_rdma_max_inline_write = RPCRDMA_DEF_INLINE;
> unsigned int xprt_rdma_memreg_strategy = RPCRDMA_FRWR;
> -int xprt_rdma_pad_optimize;
> static struct xprt_class xprt_rdma;
>
> #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
> @@ -134,13 +133,6 @@ static struct ctl_table xr_tunables_table[] = {
> .extra1 = &min_memreg,
> .extra2 = &max_memreg,
> },
> - {
> - .procname = "rdma_pad_optimize",
> - .data = &xprt_rdma_pad_optimize,
> - .maxlen = sizeof(unsigned int),
> - .mode = 0644,
> - .proc_handler = proc_dointvec,
> - },
> { },
> };

I have to NACK at least this hunk: my understanding is we
can't just remove /proc interfaces.


> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index aaec3c9be8db..d8650a3563ef 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -205,14 +205,12 @@ static void rpcrdma_update_cm_private(struct rpcrdma_ep *ep,
> unsigned int rsize, wsize;
>
> /* Default settings for RPC-over-RDMA Version One */
> - ep->re_implicit_roundup = xprt_rdma_pad_optimize;
> rsize = RPCRDMA_V1_DEF_INLINE_SIZE;
> wsize = RPCRDMA_V1_DEF_INLINE_SIZE;
>
> if (pmsg &&
> pmsg->cp_magic == rpcrdma_cmp_magic &&
> pmsg->cp_version == RPCRDMA_CMP_VERSION) {
> - ep->re_implicit_roundup = true;
> rsize = rpcrdma_decode_buffer_size(pmsg->cp_send_size);
> wsize = rpcrdma_decode_buffer_size(pmsg->cp_recv_size);
> }
> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
> index d91f54eae00b..137866a83a3a 100644
> --- a/net/sunrpc/xprtrdma/xprt_rdma.h
> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h
> @@ -74,7 +74,6 @@ struct rpcrdma_ep {
> struct ib_pd *re_pd;
> unsigned int re_max_rdma_segs;
> unsigned int re_max_fr_depth;
> - bool re_implicit_roundup;
> enum ib_mr_type re_mrtype;
> struct completion re_done;
> unsigned int re_send_count;
> @@ -441,11 +440,6 @@ rpcrdma_portstr(const struct rpcrdma_xprt *r_xprt)
> return r_xprt->rx_xprt.address_strings[RPC_DISPLAY_PORT];
> }
>
> -/* Setting this to 0 ensures interoperability with early servers.
> - * Setting this to 1 enhances certain unaligned read/write performance.
> - * Default is 0, see sysctl entry and rpc_rdma.c rpcrdma_convert_iovs() */
> -extern int xprt_rdma_pad_optimize;
> -
> /* This setting controls the hunt for a supported memory
> * registration strategy.
> */
> --
> 2.27.0
>

--
Chuck Lever