2020-01-31 20:42:15

by Chuck Lever III

Subject: [PATCH RFC2 2/3] SUNRPC: Track current encode position in struct xdr_stream

Maintain a byte-offset cursor in struct xdr_stream. This will be
used in a subsequent patch.

Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4proc.c | 1 +
include/linux/sunrpc/xdr.h | 1 +
net/sunrpc/xdr.c | 2 ++
3 files changed, 4 insertions(+)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4798667af647..9cd610485f7d 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1904,6 +1904,7 @@ static void svcxdr_init_encode(struct svc_rqst *rqstp,
xdr->iov = head;
xdr->p = head->iov_base + head->iov_len;
xdr->end = head->iov_base + PAGE_SIZE - rqstp->rq_auth_slack;
+ xdr->pos = head->iov_len;
/* Tail and page_len should be zero at this point: */
buf->len = buf->head[0].iov_len;
xdr->scratch.iov_len = 0;
diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index b41f34977995..2ca479b0708e 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -232,6 +232,7 @@ struct xdr_stream {
struct kvec *iov; /* pointer to the current kvec */
struct kvec scratch; /* Scratch buffer */
struct page **page_ptr; /* pointer to the current page */
+ unsigned int pos; /* current encode position */
unsigned int nwords; /* Remaining decode buffer length */

struct rpc_rqst *rqst; /* For debugging */
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index f3104be8ff5d..b8b78876a9bd 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -542,6 +542,7 @@ void xdr_init_encode(struct xdr_stream *xdr, struct xdr_buf *buf, __be32 *p,
buf->len += len;
iov->iov_len += len;
}
+ xdr->pos = iov->iov_len;
xdr->rqst = rqst;
}
EXPORT_SYMBOL_GPL(xdr_init_encode);
@@ -644,6 +645,7 @@ __be32 * xdr_reserve_space(struct xdr_stream *xdr, size_t nbytes)
else
xdr->buf->page_len += nbytes;
xdr->buf->len += nbytes;
+ xdr->pos += nbytes;
return p;
}
EXPORT_SYMBOL_GPL(xdr_reserve_space);

2020-01-31 21:20:08

by Trond Myklebust

Subject: Re: [PATCH RFC2 2/3] SUNRPC: Track current encode position in struct xdr_stream

On Fri, 2020-01-31 at 15:41 -0500, Chuck Lever wrote:
> Maintain a byte-offset cursor in struct xdr_stream. This will be
> used in a subsequent patch.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> fs/nfsd/nfs4proc.c | 1 +
> include/linux/sunrpc/xdr.h | 1 +
> net/sunrpc/xdr.c | 2 ++
> 3 files changed, 4 insertions(+)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 4798667af647..9cd610485f7d 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1904,6 +1904,7 @@ static void svcxdr_init_encode(struct svc_rqst
> *rqstp,
> xdr->iov = head;
> xdr->p = head->iov_base + head->iov_len;
> xdr->end = head->iov_base + PAGE_SIZE - rqstp->rq_auth_slack;
> + xdr->pos = head->iov_len;
> /* Tail and page_len should be zero at this point: */
> buf->len = buf->head[0].iov_len;
> xdr->scratch.iov_len = 0;
> diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
> index b41f34977995..2ca479b0708e 100644
> --- a/include/linux/sunrpc/xdr.h
> +++ b/include/linux/sunrpc/xdr.h
> @@ -232,6 +232,7 @@ struct xdr_stream {
> struct kvec *iov; /* pointer to the current kvec */
> struct kvec scratch; /* Scratch buffer */
> struct page **page_ptr; /* pointer to the current page */
> + unsigned int pos; /* current encode position */
> unsigned int nwords; /* Remaining decode buffer length */

Is there any reason not to make this an anonymous union? 'pos' is used
only when encoding, and 'nwords' is only when decoding. Never the twain
shall meet...

>
> struct rpc_rqst *rqst; /* For debugging */
> diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
> index f3104be8ff5d..b8b78876a9bd 100644
> --- a/net/sunrpc/xdr.c
> +++ b/net/sunrpc/xdr.c
> @@ -542,6 +542,7 @@ void xdr_init_encode(struct xdr_stream *xdr,
> struct xdr_buf *buf, __be32 *p,
> buf->len += len;
> iov->iov_len += len;
> }
> + xdr->pos = iov->iov_len;
> xdr->rqst = rqst;
> }
> EXPORT_SYMBOL_GPL(xdr_init_encode);
> @@ -644,6 +645,7 @@ __be32 * xdr_reserve_space(struct xdr_stream
> *xdr, size_t nbytes)
> else
> xdr->buf->page_len += nbytes;
> xdr->buf->len += nbytes;
> + xdr->pos += nbytes;
> return p;
> }
> EXPORT_SYMBOL_GPL(xdr_reserve_space);
>

You should fix up xdr_encode_write() as well for consistency.


Now with all that said, why do we need both xdr->pos and xdr->buf->len?
They should always hold the same value.

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]