Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\))
Subject: Re: [PATCH v1] NFS: Fix handling of reply page vector
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <bede7559be11f283e9f37db2da0b47ddf869a468.camel@hammerspace.com>
Date:   Mon, 8 Apr 2019 16:58:13 -0400
Cc:     Olga Kornievskaia <kolga@netapp.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <B28ACEA4-BB35-4137-A81F-50A1D2974F6A@oracle.com>
References: <20190408200030.16366.22026.stgit@manet.1015granger.net>
 <bede7559be11f283e9f37db2da0b47ddf869a468.camel@hammerspace.com>
To:     Trond Myklebust <trondmy@hammerspace.com>
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk


> On Apr 8, 2019, at 4:38 PM, Trond Myklebust <trondmy@hammerspace.com> =
wrote:
>=20
> On Mon, 2019-04-08 at 16:00 -0400, Chuck Lever wrote:
>> NFSv4 GETACL and FS_LOCATIONS requests stopped working in v5.1-rc.
>>=20
>> These two need the extra padding to be added directly to the reply
>> length.
>>=20
>> Reported-by: Olga Kornievskaia <aglo@umich.edu>
>> Fixes: 02ef04e432ba ("NFS: Account for XDR pad of buf->pages")
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> fs/nfs/nfs4xdr.c |    4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>=20
>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> index cfcabc3..6024461 100644
>> --- a/fs/nfs/nfs4xdr.c
>> +++ b/fs/nfs/nfs4xdr.c
>> @@ -2589,7 +2589,7 @@ static void nfs4_xdr_enc_getacl(struct rpc_rqst
>> *req, struct xdr_stream *xdr,
>> 			ARRAY_SIZE(nfs4_acl_bitmap), &hdr);
>>=20
>> 	rpc_prepare_reply_pages(req, args->acl_pages, 0,
>> -				args->acl_len, replen);
>> +				args->acl_len, replen + 1);
>> 	encode_nops(&hdr);
>> }
>>=20
>> @@ -2811,7 +2811,7 @@ static void nfs4_xdr_enc_fs_locations(struct
>> rpc_rqst *req,
>> 	}
>>=20
>> 	rpc_prepare_reply_pages(req, (struct page **)&args->page, 0,
>> -				PAGE_SIZE, replen);
>> +				PAGE_SIZE, replen + 1);
>> 	encode_nops(&hdr);
>> }
>>=20
>=20
> I'm having trouble with the math here. Why are we pre-emptively
> subtracting a word from the tail? The header constants are always =
4-bit=20
> aligned because they are calculated as a word count, so I'm not
> understanding why we need commit 02ef04e432ba at all.
>=20
> Can you please explain, Chuck?

The goal is to allocate a reply buffer that is just large enough
to fit the expected reply, and ensure that the variable-length
payload will start exactly where the xdr_buf's pages begin.

In cases where the payload length is not aligned to four bytes,
an extra quad has to be allocated in the tail. So, the total
reply length is increased by one quad so there is enough space
for the XDR pad bytes in the tail.


--
Chuck Lever