* [PATCH v2 0/2] Fixes for READ_PLUS @ 2020-12-08 20:29 schumaker.anna 2020-12-08 20:29 ` [PATCH v2 1/2] SUNRPC: Keep buf->len in sync with xdr->nwords when expanding holes schumaker.anna 2020-12-08 20:29 ` [PATCH v2 2/2] SUNRPC: Check if the buffer has fewer bytes than requested schumaker.anna 0 siblings, 2 replies; 6+ messages in thread From: schumaker.anna @ 2020-12-08 20:29 UTC (permalink / raw) To: linux-nfs; +Cc: Anna.Schumaker From: Anna Schumaker <Anna.Schumaker@Netapp.com> These patches fix up hole and data segment decoding for READ_PLUS. It turns out I wasn't handling data getting truncated off the end of the message properly. These patches fix it up, and now xfstests generic/091 and generic/263 pass when run against servers exporting ext4 and btrfs. These tests also pass against servers exporting xfs when the clone operation is disabled, so it seems like there is something going on inside the xfs filesystem causing these tests to still fail. - Changes since v1: - Drop patch for allocating scratch page - Drop patch for disabling READ_PLUS behind a Kconfig option Thanks, Anna Anna Schumaker (2): SUNRPC: Keep buf->len in sync with xdr->nwords when expanding holes SUNRPC: Check if the buffer has fewer bytes than requested net/sunrpc/xdr.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) -- 2.29.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] SUNRPC: Keep buf->len in sync with xdr->nwords when expanding holes 2020-12-08 20:29 [PATCH v2 0/2] Fixes for READ_PLUS schumaker.anna @ 2020-12-08 20:29 ` schumaker.anna 2020-12-08 20:56 ` Chuck Lever 2020-12-08 20:29 ` [PATCH v2 2/2] SUNRPC: Check if the buffer has fewer bytes than requested schumaker.anna 1 sibling, 1 reply; 6+ messages in thread From: schumaker.anna @ 2020-12-08 20:29 UTC (permalink / raw) To: linux-nfs; +Cc: Anna.Schumaker From: Anna Schumaker <Anna.Schumaker@Netapp.com> Otherwise we could end up placing data a few bytes off from where we actually want to put it. Fixes: 84ce182ab85b "SUNRPC: Add the ability to expand holes in data pages" Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com> --- net/sunrpc/xdr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c index 71e03b930b70..5b848fe65c81 100644 --- a/net/sunrpc/xdr.c +++ b/net/sunrpc/xdr.c @@ -1314,6 +1314,7 @@ uint64_t xdr_expand_hole(struct xdr_stream *xdr, uint64_t offset, uint64_t lengt unsigned int res = _shift_data_right_tail(buf, from + bytes - shift, shift); truncated = shift - res; xdr->nwords -= XDR_QUADLEN(truncated); + buf->len -= 4 * XDR_QUADLEN(truncated); bytes -= shift; } @@ -1325,7 +1326,7 @@ uint64_t xdr_expand_hole(struct xdr_stream *xdr, uint64_t offset, uint64_t lengt bytes); _zero_pages(buf->pages, buf->page_base + offset, length); - buf->len += length - (from - offset) - truncated; + buf->len += length - (from - offset); xdr_set_page(xdr, offset + length, PAGE_SIZE); return length; } -- 2.29.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] SUNRPC: Keep buf->len in sync with xdr->nwords when expanding holes 2020-12-08 20:29 ` [PATCH v2 1/2] SUNRPC: Keep buf->len in sync with xdr->nwords when expanding holes schumaker.anna @ 2020-12-08 20:56 ` Chuck Lever 2020-12-08 21:11 ` Anna Schumaker 0 siblings, 1 reply; 6+ messages in thread From: Chuck Lever @ 2020-12-08 20:56 UTC (permalink / raw) To: Anna Schumaker; +Cc: Linux NFS Mailing List, Anna Schumaker > On Dec 8, 2020, at 3:29 PM, schumaker.anna@gmail.com wrote: > > From: Anna Schumaker <Anna.Schumaker@Netapp.com> > > Otherwise we could end up placing data a few bytes off from where we > actually want to put it. > > Fixes: 84ce182ab85b "SUNRPC: Add the ability to expand holes in data pages" > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com> > --- > net/sunrpc/xdr.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c > index 71e03b930b70..5b848fe65c81 100644 > --- a/net/sunrpc/xdr.c > +++ b/net/sunrpc/xdr.c > @@ -1314,6 +1314,7 @@ uint64_t xdr_expand_hole(struct xdr_stream *xdr, uint64_t offset, uint64_t lengt > unsigned int res = _shift_data_right_tail(buf, from + bytes - shift, shift); > truncated = shift - res; > xdr->nwords -= XDR_QUADLEN(truncated); > + buf->len -= 4 * XDR_QUADLEN(truncated); If I understand what you're doing here correctly, the usual idiom is "XDR_QUADLEN(truncated) << 2" . > bytes -= shift; > } > > @@ -1325,7 +1326,7 @@ uint64_t xdr_expand_hole(struct xdr_stream *xdr, uint64_t offset, uint64_t lengt > bytes); > _zero_pages(buf->pages, buf->page_base + offset, length); > > - buf->len += length - (from - offset) - truncated; > + buf->len += length - (from - offset); > xdr_set_page(xdr, offset + length, PAGE_SIZE); > return length; > } > -- > 2.29.2 > -- Chuck Lever ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] SUNRPC: Keep buf->len in sync with xdr->nwords when expanding holes 2020-12-08 20:56 ` Chuck Lever @ 2020-12-08 21:11 ` Anna Schumaker 2020-12-09 16:05 ` Chuck Lever 0 siblings, 1 reply; 6+ messages in thread From: Anna Schumaker @ 2020-12-08 21:11 UTC (permalink / raw) To: Chuck Lever; +Cc: Linux NFS Mailing List On Tue, Dec 8, 2020 at 3:56 PM Chuck Lever <chuck.lever@oracle.com> wrote: > > > > > On Dec 8, 2020, at 3:29 PM, schumaker.anna@gmail.com wrote: > > > > From: Anna Schumaker <Anna.Schumaker@Netapp.com> > > > > Otherwise we could end up placing data a few bytes off from where we > > actually want to put it. > > > > Fixes: 84ce182ab85b "SUNRPC: Add the ability to expand holes in data pages" > > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com> > > --- > > net/sunrpc/xdr.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c > > index 71e03b930b70..5b848fe65c81 100644 > > --- a/net/sunrpc/xdr.c > > +++ b/net/sunrpc/xdr.c > > @@ -1314,6 +1314,7 @@ uint64_t xdr_expand_hole(struct xdr_stream *xdr, uint64_t offset, uint64_t lengt > > unsigned int res = _shift_data_right_tail(buf, from + bytes - shift, shift); > > truncated = shift - res; > > xdr->nwords -= XDR_QUADLEN(truncated); > > + buf->len -= 4 * XDR_QUADLEN(truncated); > > If I understand what you're doing here correctly, the usual idiom > is "XDR_QUADLEN(truncated) << 2" . Oh, that works too. I'll adjust the patch. Thanks for letting me know! Anna > > > > bytes -= shift; > > } > > > > @@ -1325,7 +1326,7 @@ uint64_t xdr_expand_hole(struct xdr_stream *xdr, uint64_t offset, uint64_t lengt > > bytes); > > _zero_pages(buf->pages, buf->page_base + offset, length); > > > > - buf->len += length - (from - offset) - truncated; > > + buf->len += length - (from - offset); > > xdr_set_page(xdr, offset + length, PAGE_SIZE); > > return length; > > } > > -- > > 2.29.2 > > > > -- > Chuck Lever > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] SUNRPC: Keep buf->len in sync with xdr->nwords when expanding holes 2020-12-08 21:11 ` Anna Schumaker @ 2020-12-09 16:05 ` Chuck Lever 0 siblings, 0 replies; 6+ messages in thread From: Chuck Lever @ 2020-12-09 16:05 UTC (permalink / raw) To: Anna Schumaker; +Cc: Linux NFS Mailing List > On Dec 8, 2020, at 4:11 PM, Anna Schumaker <schumaker.anna@gmail.com> wrote: > > On Tue, Dec 8, 2020 at 3:56 PM Chuck Lever <chuck.lever@oracle.com> wrote: >> >> >> >>> On Dec 8, 2020, at 3:29 PM, schumaker.anna@gmail.com wrote: >>> >>> From: Anna Schumaker <Anna.Schumaker@Netapp.com> >>> >>> Otherwise we could end up placing data a few bytes off from where we >>> actually want to put it. >>> >>> Fixes: 84ce182ab85b "SUNRPC: Add the ability to expand holes in data pages" >>> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com> >>> --- >>> net/sunrpc/xdr.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c >>> index 71e03b930b70..5b848fe65c81 100644 >>> --- a/net/sunrpc/xdr.c >>> +++ b/net/sunrpc/xdr.c >>> @@ -1314,6 +1314,7 @@ uint64_t xdr_expand_hole(struct xdr_stream *xdr, uint64_t offset, uint64_t lengt >>> unsigned int res = _shift_data_right_tail(buf, from + bytes - shift, shift); >>> truncated = shift - res; >>> xdr->nwords -= XDR_QUADLEN(truncated); >>> + buf->len -= 4 * XDR_QUADLEN(truncated); >> >> If I understand what you're doing here correctly, the usual idiom >> is "XDR_QUADLEN(truncated) << 2" . > > Oh, that works too. I'll adjust the patch. Thanks for letting me know! > Urp, sorry. These days, the preferred mechanism is xdr_align_size(). Old habits die hard, I guess. > Anna > >> >> >>> bytes -= shift; >>> } >>> >>> @@ -1325,7 +1326,7 @@ uint64_t xdr_expand_hole(struct xdr_stream *xdr, uint64_t offset, uint64_t lengt >>> bytes); >>> _zero_pages(buf->pages, buf->page_base + offset, length); >>> >>> - buf->len += length - (from - offset) - truncated; >>> + buf->len += length - (from - offset); >>> xdr_set_page(xdr, offset + length, PAGE_SIZE); >>> return length; >>> } >>> -- >>> 2.29.2 >>> >> >> -- >> Chuck Lever -- Chuck Lever ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] SUNRPC: Check if the buffer has fewer bytes than requested 2020-12-08 20:29 [PATCH v2 0/2] Fixes for READ_PLUS schumaker.anna 2020-12-08 20:29 ` [PATCH v2 1/2] SUNRPC: Keep buf->len in sync with xdr->nwords when expanding holes schumaker.anna @ 2020-12-08 20:29 ` schumaker.anna 1 sibling, 0 replies; 6+ messages in thread From: schumaker.anna @ 2020-12-08 20:29 UTC (permalink / raw) To: linux-nfs; +Cc: Anna.Schumaker From: Anna Schumaker <Anna.Schumaker@Netapp.com> xdr_expand_hole() might truncate data off the end of the buffer. If that happens, we need to return a short read to the NFS code to indicate that some data has been lost. Fixes: e6ac0accb27c "SUNRPC: Add an xdr_align_data() function" Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com> --- net/sunrpc/xdr.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c index 5b848fe65c81..68f470e33427 100644 --- a/net/sunrpc/xdr.c +++ b/net/sunrpc/xdr.c @@ -1273,6 +1273,8 @@ uint64_t xdr_align_data(struct xdr_stream *xdr, uint64_t offset, uint32_t length bytes = xdr->nwords << 2; if (length < bytes) bytes = length; + if (bytes < length) + length = bytes; /* Move page data to the left */ if (from > offset) { -- 2.29.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-09 16:08 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-12-08 20:29 [PATCH v2 0/2] Fixes for READ_PLUS schumaker.anna 2020-12-08 20:29 ` [PATCH v2 1/2] SUNRPC: Keep buf->len in sync with xdr->nwords when expanding holes schumaker.anna 2020-12-08 20:56 ` Chuck Lever 2020-12-08 21:11 ` Anna Schumaker 2020-12-09 16:05 ` Chuck Lever 2020-12-08 20:29 ` [PATCH v2 2/2] SUNRPC: Check if the buffer has fewer bytes than requested schumaker.anna
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox