diff mbox series

[v6,07/10] SUNRPC: Add the ability to expand holes in data pages

Message ID 20200928170919.707641-8-Anna.Schumaker@Netapp.com (mailing list archive)
State New, archived
Headers show
Series NFS: Add support for the v4.2 READ_PLUS operation | expand

Commit Message

Anna Schumaker Sept. 28, 2020, 5:09 p.m. UTC 
From: Anna Schumaker <Anna.Schumaker@Netapp.com>

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@Netapp.com>
---
 include/linux/sunrpc/xdr.h |  1 +
 net/sunrpc/xdr.c           | 73 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+)

Comments

Trond Myklebust Oct. 1, 2020, 5:39 p.m. UTC | #1 
On Mon, 2020-09-28 at 13:09 -0400, schumaker.anna@gmail.com wrote:
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> 
> 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@Netapp.com>
> ---
>  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.

> +			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'?

> +
> +	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
Anna Schumaker Oct. 2, 2020, 7:37 p.m. UTC | #2 
Hi Trond,

On Thu, Oct 1, 2020 at 1:39 PM Trond Myklebust <trondmy@hammerspace.com> wrote:
>
> On Mon, 2020-09-28 at 13:09 -0400, schumaker.anna@gmail.com wrote:
> > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> >
> > 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@Netapp.com>
> > ---
> >  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@hammerspace.com
>
>
diff mbox series

Patch

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) {
+			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;
+
+	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