| Message ID | 20201208202925.597663-2-Anna.Schumaker@Netapp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes for READ_PLUS | expand |
> 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
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 > > >
> 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
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; }