From: Anna Schumaker <[email protected]>
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
From: Anna Schumaker <[email protected]>
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 <[email protected]>
---
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
From: Anna Schumaker <[email protected]>
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 <[email protected]>
---
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
> On Dec 8, 2020, at 3:29 PM, [email protected] wrote:
>
> From: Anna Schumaker <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
On Tue, Dec 8, 2020 at 3:56 PM Chuck Lever <[email protected]> wrote:
>
>
>
> > On Dec 8, 2020, at 3:29 PM, [email protected] wrote:
> >
> > From: Anna Schumaker <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
>
>
>
> On Dec 8, 2020, at 4:11 PM, Anna Schumaker <[email protected]> wrote:
>
> On Tue, Dec 8, 2020 at 3:56 PM Chuck Lever <[email protected]> wrote:
>>
>>
>>
>>> On Dec 8, 2020, at 3:29 PM, [email protected] wrote:
>>>
>>> From: Anna Schumaker <[email protected]>
>>>
>>> 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 <[email protected]>
>>> ---
>>> 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