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
