Hi Trond,
On Thu, Oct 1, 2020 at 1:39 PM Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote:
>
> On Mon, 2020-09-28 at 13:09 -0400, schumaker.anna@xxxxxxxxx wrote:
> > From: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
> >
> > This patch adds the ability to "read a hole" into a set of XDR data
> > pages by taking the following steps:
> >
> > 1) Shift all data after the current xdr->p to the right, possibly
> > into
> > the tail,
> > 2) Zero the specified range, and
> > 3) Update xdr->p to point beyond the hole.
> >
> > Signed-off-by: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
> > ---
> > include/linux/sunrpc/xdr.h | 1 +
> > net/sunrpc/xdr.c | 73
> > ++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 74 insertions(+)
> >
> > diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
> > index 026edbd041d5..36a81c29542e 100644
> > --- a/include/linux/sunrpc/xdr.h
> > +++ b/include/linux/sunrpc/xdr.h
> > @@ -252,6 +252,7 @@ extern __be32 *xdr_inline_decode(struct
> > xdr_stream *xdr, size_t nbytes);
> > extern unsigned int xdr_read_pages(struct xdr_stream *xdr, unsigned
> > int len);
> > extern void xdr_enter_page(struct xdr_stream *xdr, unsigned int
> > len);
> > extern int xdr_process_buf(struct xdr_buf *buf, unsigned int offset,
> > unsigned int len, int (*actor)(struct scatterlist *, void *), void
> > *data);
> > +extern uint64_t xdr_expand_hole(struct xdr_stream *, uint64_t,
> > uint64_t);
> >
> > /**
> > * xdr_stream_remaining - Return the number of bytes remaining in
> > the stream
> > diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
> > index d8c9555c6f2b..24baf052e6e6 100644
> > --- a/net/sunrpc/xdr.c
> > +++ b/net/sunrpc/xdr.c
> > @@ -390,6 +390,42 @@ _copy_from_pages(char *p, struct page **pages,
> > size_t pgbase, size_t len)
> > }
> > EXPORT_SYMBOL_GPL(_copy_from_pages);
> >
> > +/**
> > + * _zero_pages
> > + * @pages: array of pages
> > + * @pgbase: beginning page vector address
> > + * @len: length
> > + */
> > +static void
> > +_zero_pages(struct page **pages, size_t pgbase, size_t len)
> > +{
> > + struct page **page;
> > + char *vpage;
> > + size_t zero;
> > +
> > + page = pages + (pgbase >> PAGE_SHIFT);
> > + pgbase &= ~PAGE_MASK;
> > +
> > + do {
> > + zero = PAGE_SIZE - pgbase;
> > + if (zero > len)
> > + zero = len;
> > +
> > + vpage = kmap_atomic(*page);
> > + memset(vpage + pgbase, 0, zero);
> > + kunmap_atomic(vpage);
> > +
> > + pgbase += zero;
> > + if (pgbase == PAGE_SIZE) {
>
> Hmm.... Do we need to make this conditional? After all, if pgbase !=
> PAGE_SIZE in this case, then zero == len, in which case we still want
> to flush_dcache_page(*page) and then exit.
I removed the condition, and everything works just the same. So I
guess it's not necessary. Good catch!
>
> > + flush_dcache_page(*page);
> > + pgbase = 0;
> > + page++;
> > + }
> > +
> > + } while ((len -= zero) != 0);
> > + flush_dcache_page(*page);
> > +}
> > +
> > /**
> > * xdr_shrink_bufhead
> > * @buf: xdr_buf
> > @@ -1141,6 +1177,43 @@ unsigned int xdr_read_pages(struct xdr_stream
> > *xdr, unsigned int len)
> > }
> > EXPORT_SYMBOL_GPL(xdr_read_pages);
> >
> > +uint64_t xdr_expand_hole(struct xdr_stream *xdr, uint64_t offset,
> > uint64_t length)
> > +{
> > + struct xdr_buf *buf = xdr->buf;
> > + unsigned int bytes;
> > + unsigned int from;
> > + unsigned int truncated = 0;
> > +
> > + if ((offset + length) < offset ||
> > + (offset + length) > buf->page_len)
> > + length = buf->page_len - offset;
> > +
> > + xdr_realign_pages(xdr);
> > + from = xdr_page_pos(xdr);
> > + bytes = xdr->nwords << 2;
>
> Hmm... Won't this mean that we also include the padding at the end of
> the read buffer in 'bytes'?
I guess it would, assuming the server adds padding to the end of the
buffer. Should the server be adding padding here? Because it doesn't
the way I have it coded now (and RFC 7862 doesn't say anything about
padding at the end of a READ_PLUS reply).
Anna
>
> > +
> > + if (offset + length + bytes > buf->page_len) {
> > + unsigned int shift = (offset + length + bytes) - buf-
> > >page_len;
> > + unsigned int res = _shift_data_right_tail(buf, from +
> > bytes - shift, shift);
> > + truncated = shift - res;
> > + xdr->nwords -= XDR_QUADLEN(truncated);
> > + bytes -= shift;
> > + }
> > +
> > + /* Now move the page data over and zero pages */
> > + if (bytes > 0)
> > + _shift_data_right_pages(buf->pages,
> > + buf->page_base + offset +
> > length,
> > + buf->page_base + from,
> > + bytes);
> > + _zero_pages(buf->pages, buf->page_base + offset, length);
> > +
> > + buf->len += length - (from - offset) - truncated;
> > + xdr_set_page(xdr, offset + length, PAGE_SIZE);
> > + return length;
> > +}
> > +EXPORT_SYMBOL_GPL(xdr_expand_hole);
> > +
> > /**
> > * xdr_enter_page - decode data from the XDR page
> > * @xdr: pointer to xdr_stream struct
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@xxxxxxxxxxxxxxx
>
>
