Hi Anna-
> On Aug 3, 2020, at 12:59 PM, schumaker.anna@xxxxxxxxx wrote:
>
> From: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
>
> Reserving space for a large READ payload requires special handling when
> reserving space in the xdr buffer pages. One problem we can have is use
> of the scratch buffer, which is used to get a pointer to a contiguous
> region of data up to PAGE_SIZE. When using the scratch buffer, calls to
> xdr_commit_encode() shift the data to it's proper alignment in the xdr
> buffer. If we've reserved several pages in a vector, then this could
> potentially invalidate earlier pointers and result in incorrect READ
> data being sent to the client.
>
> I get around this by looking at the amount of space left in the current
> page, and never reserve more than that for each entry in the read
> vector. This lets us place data directly where it needs to go in the
> buffer pages.
Nit: This appears to be a refactoring change that should be squashed
together with 2/6.
> Signed-off-by: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
> ---
> include/linux/sunrpc/xdr.h | 2 ++
> net/sunrpc/xdr.c | 45 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 47 insertions(+)
>
> diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
> index 22c207b2425f..bac459584dd0 100644
> --- a/include/linux/sunrpc/xdr.h
> +++ b/include/linux/sunrpc/xdr.h
> @@ -234,6 +234,8 @@ typedef int (*kxdrdproc_t)(struct rpc_rqst *rqstp, struct xdr_stream *xdr,
> extern void xdr_init_encode(struct xdr_stream *xdr, struct xdr_buf *buf,
> __be32 *p, struct rpc_rqst *rqst);
> extern __be32 *xdr_reserve_space(struct xdr_stream *xdr, size_t nbytes);
> +extern int xdr_reserve_space_vec(struct xdr_stream *xdr, struct kvec *vec,
> + size_t nbytes);
> extern void xdr_commit_encode(struct xdr_stream *xdr);
> extern void xdr_truncate_encode(struct xdr_stream *xdr, size_t len);
> extern int xdr_restrict_buflen(struct xdr_stream *xdr, int newbuflen);
> diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
> index be11d672b5b9..6dfe5dc8b35f 100644
> --- a/net/sunrpc/xdr.c
> +++ b/net/sunrpc/xdr.c
> @@ -648,6 +648,51 @@ __be32 * xdr_reserve_space(struct xdr_stream *xdr, size_t nbytes)
> }
> EXPORT_SYMBOL_GPL(xdr_reserve_space);
>
> +
> +/**
> + * xdr_reserve_space_vec - Reserves a large amount of buffer space for sending
> + * @xdr: pointer to xdr_stream
> + * @vec: pointer to a kvec array
> + * @nbytes: number of bytes to reserve
> + *
> + * Reserves enough buffer space to encode 'nbytes' of data and stores the
> + * pointers in 'vec'. The size argument passed to xdr_reserve_space() is
> + * determined based on the number of bytes remaining in the current page to
> + * avoid invalidating iov_base pointers when xdr_commit_encode() is called.
> + */
> +int xdr_reserve_space_vec(struct xdr_stream *xdr, struct kvec *vec, size_t nbytes)
> +{
> + int thislen;
> + int v = 0;
> + __be32 *p;
> +
> + /*
> + * svcrdma requires every READ payload to start somewhere
> + * in xdr->pages.
> + */
> + if (xdr->iov == xdr->buf->head) {
> + xdr->iov = NULL;
> + xdr->end = xdr->p;
> + }
> +
> + while (nbytes) {
> + thislen = xdr->buf->page_len % PAGE_SIZE;
> + thislen = min_t(size_t, nbytes, PAGE_SIZE - thislen);
> +
> + p = xdr_reserve_space(xdr, thislen);
> + if (!p)
> + return -EIO;
> +
> + vec[v].iov_base = p;
> + vec[v].iov_len = thislen;
> + v++;
> + nbytes -= thislen;
> + }
> +
> + return v;
> +}
> +EXPORT_SYMBOL_GPL(xdr_reserve_space_vec);
> +
> /**
> * xdr_truncate_encode - truncate an encode buffer
> * @xdr: pointer to xdr_stream
> --
> 2.27.0
>
--
Chuck Lever
