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

[Chuck Lever <chuck.lever@oracle.com>]

> On Nov 23, 2020, at 12:59 PM, Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote:
> 
> On Mon, Nov 23, 2020 at 12:37 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>> 
>> 
>> 
>>> On Nov 23, 2020, at 11:42 AM, Olga Kornievskaia <olga.kornievskaia@gmail.com> 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?
>> 
>> Isn't the logic in rpcrdma_convert_iovs() allocating those pages?
> 
> No, rpcrdm_convert_iovs is only called for when you have reply chunks,
> lists etc but not for the inline messages. What am I missing?

So, then, rpcrdma_marshal_req() is deciding that the LISTXATTRS
reply is supposed to fit inline. That means rqst->rq_rcv_buf.buflen
is small.

But if rpcrdma_inline_fixup() is trying to fill pages,
rqst->rq_rcv_buf.page_len must not be zero? That sounds like the
LISTXATTRS encoder is not setting up the receive buffer correctly.

The receive buffer's buflen field is supposed to be set to a value
that is at least as large as page_len, I would think.


>>> Or, should LISTXATTR be re-written
>>> to be like READDIR which allocates pages before calling the code.
>> 
>> AIUI READDIR reads into the directory inode's page cache. I recall
>> that Frank couldn't do that for LISTXATTR because there's no
>> similar page cache associated with the xattr listing.
>> 
>> That said, I would prefer that the *XATTR procedures directly
>> allocate pages instead of relying on SPARSE_PAGES, which is a hack
>> IMO. I think it would have to use alloc_page() for that, and then
>> ensure those pages are released when the call has completed.
>> 
>> I'm not convinced this is the cause of the problem you're seeing,
>> though.
>> 
>> --
>> Chuck Lever

--
Chuck Lever