Re: [PATCH v2] nfsd: Implement large extent array support in pNFS

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

On 6/13/25 8:01 AM, Sergey Bashirov wrote:
> On Tue, Jun 10, 2025 at 02:10:46PM -0400, Chuck Lever wrote:
>> On 6/9/25 9:18 PM, Sergey Bashirov wrote:
>>> +	xdr_init_decode(&xdr, buf, buf->head[0].iov_base, NULL);
>>> +	xdr_set_scratch_buffer(&xdr, scratch, sizeof(scratch));
>>
>> Consider using svcxdr_init_decode() instead.
> 
> I see that svcxdr_init_decode() does the same two steps. What I
> concerned about is that it takes the top-level svc_rqst struct
> and modifies it. Of course, we can pass rqstp from nfsd4_layoutcommit()
> to the layout driver callback. But then we would need to make a backup
> of the original xdr buffer and stream position, set up and initialize
> the xdr sub-buffer, and at the end restore back the original xdr stream.
> All these actions seem somewhat unnecessary and not so elegant to me.
> 
> Is it acceptable to keep the current solution in the patch or am I
> missing something?

At the very least, you will need to allocate a proper scratch buffer
and free it when the layout decode is complete if you do not use
svcxdr_init_decode().

IIUC nfsd4_layoutcommit() is called after the COMPOUND arguments have
been XDR decoded, and before the COMPOUND result is encoded. (I'm
guessing) it should be safe to use svcxdr_init_decode() in
nfsd4_layoutcommit(), but of course you should test first.

If it turns out not to be safe, you might just use the idea of
invoking alloc_page() and put_page() for the scratch buffer.


-- 
Chuck Lever