Re: [PATCH 1/1] NFSv4.2: fix LISTXATTR buffer receive size

From: Chuck Lever <chuck.lever@oracle.com>
To: Frank van der Linden <fllinden@amazon.com>
Cc: Olga Kornievskaia <olga.kornievskaia@gmail.com>,
	Trond Myklebust <trond.myklebust@hammerspace.com>,
	Anna Schumaker <anna.schumaker@netapp.com>,
	Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 1/1] NFSv4.2: fix LISTXATTR buffer receive size
Date: Mon, 23 Nov 2020 12:56:11 -0500	[thread overview]
Message-ID: <F8B0C391-9230-4B30-BCCD-D5E7EECF4910@oracle.com> (raw)
In-Reply-To: <20201123173802.GA26158@dev-dsk-fllinden-2c-c1893d73.us-west-2.amazon.com>

> On Nov 23, 2020, at 12:38 PM, Frank van der Linden <fllinden@amazon.com> wrote:
> 
> On Mon, Nov 23, 2020 at 11:42:46AM -0500, Olga Kornievskaia wrote:
>> Hi Frank, Chuck,
>> 
>> I would like your option on how LISTXATTR is supposed to work over
>> RDMA. Here's my current understanding of why the listxattr is not
>> working over the RDMA.
>> 
>> This happens when the listxattr is called with a very small buffer
>> size which RDMA wants to send an inline request. I really dont
>> understand why, Chuck, you are not seeing any problems with hardware
>> as far as I can tell it would have the same problem because the inline
>> threshold size would still make this size inline.
>> rcprdma_inline_fixup() is trying to write to pages that don't exist.
>> 
>> When LISTXATTR sets this flag XDRBUF_SPARSE_PAGES there is code that
>> will allocate pages in xs_alloc_sparse_pages() but this is ONLY for
>> TCP. RDMA doesn't have anything like that.
>> 
>> Question: Should there be code added to RDMA that will do something
>> similar when it sees that flag set? Or, should LISTXATTR be re-written
>> to be like READDIR which allocates pages before calling the code.
> 
> Hm.. so if the flag never worked for RDMA, was NFS_V3_ACL ever tested
> over RDMA? That's the only other user.
> 
> If the flag simply doesn't work, I agree that it should either be fixed
> or just removed.
> 
> It wouldn't be the end of the world to change LISTXATTRS (and GETXATTR)
> to use preallocated pages. But, I didn't do that because I didn't want to
> waste the max size (64k) every time, even though you usually just get
> a few hundred bytes at most. So it seems like fixing XDRBUF_SPARSE_PAGES
> is cleaner.

Also, because this is for a receive buffer, the transport always has
to allocate the maximum number of pages. Especially for RPC/RDMA, this
is not an "allocate on demand" kind of thing. The maximum buffer size
has to be allocated every time.

--
Chuck Lever