Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\))
Subject: Re: [PATCH v2 1/2] SUNRPC: Keep buf->len in sync with xdr->nwords
 when expanding holes
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <CAFX2JfkNO9LL_iaRE_RGjP+BHKiZGk8Eedg88_MKkuZUAysThw@mail.gmail.com>
Date:   Wed, 9 Dec 2020 11:05:18 -0500
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <4AE085D3-7DA3-4CC3-B9F9-F9A5D6A718B2@oracle.com>
References: <20201208202925.597663-1-Anna.Schumaker@Netapp.com>
 <20201208202925.597663-2-Anna.Schumaker@Netapp.com>
 <2A6797DD-246D-4994-B38C-57AA0196D061@oracle.com>
 <CAFX2JfkNO9LL_iaRE_RGjP+BHKiZGk8Eedg88_MKkuZUAysThw@mail.gmail.com>
To:     Anna Schumaker <schumaker.anna@gmail.com>
Precedence: bulk


> On Dec 8, 2020, at 4:11 PM, Anna Schumaker <schumaker.anna@gmail.com> =
wrote:
>=20
> On Tue, Dec 8, 2020 at 3:56 PM Chuck Lever <chuck.lever@oracle.com> =
wrote:
>>=20
>>=20
>>=20
>>> On Dec 8, 2020, at 3:29 PM, schumaker.anna@gmail.com wrote:
>>>=20
>>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>>=20
>>> Otherwise we could end up placing data a few bytes off from where we
>>> actually want to put it.
>>>=20
>>> 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(-)
>>>=20
>>> 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 =3D _shift_data_right_tail(buf, from + =
bytes - shift, shift);
>>>              truncated =3D shift - res;
>>>              xdr->nwords -=3D XDR_QUADLEN(truncated);
>>> +             buf->len -=3D 4 * XDR_QUADLEN(truncated);
>>=20
>> If I understand what you're doing here correctly, the usual idiom
>> is "XDR_QUADLEN(truncated) << 2" .
>=20
> Oh, that works too. I'll adjust the patch. Thanks for letting me know!
>=20

Urp, sorry. These days, the preferred mechanism is xdr_align_size().
Old habits die hard, I guess.


> Anna
>=20
>>=20
>>=20
>>>              bytes -=3D shift;
>>>      }
>>>=20
>>> @@ -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);
>>>=20
>>> -     buf->len +=3D length - (from - offset) - truncated;
>>> +     buf->len +=3D length - (from - offset);
>>>      xdr_set_page(xdr, offset + length, PAGE_SIZE);
>>>      return length;
>>> }
>>> --
>>> 2.29.2
>>>=20
>>=20
>> --
>> Chuck Lever

--
Chuck Lever