Re: [PATCH v1 1/2] NFSD: Clean up write argument XDR decoders

[Bruce Fields <bfields@fieldses.org>]

On Fri, Mar 23, 2018 at 06:37:51PM -0400, Chuck Lever wrote:
> 
> 
> > On Mar 23, 2018, at 5:55 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
> > 
> > 
> > 
> >> On Mar 23, 2018, at 5:32 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> >> 
> >> If I understand correctly, in the v4 case you change the way the page
> >> data is passed by using rq_arg.pages instead of a list of pages for each
> >> write.
> >> 
> >> I don't think that handles the case of compounds with multiple writes.
> >> 
> >> If all the writes are contained in a page, then you may be OK, since the
> >> wr_head iovec is all the information you need to pass the write data
> >> from the xdr code to the proc code.
> >> 
> >> But if there are multiple larger writes, you also need to know where the
> >> page data is.  You're depending on rq_arg.pages for that, but there's
> >> only one of those per compound.
> > 
> > I thought we were going to handle that case by chaining xdr_bufs--
> > one per NFSv4 WRITE operation. There has to be some structured way
> > to pass distinct write payloads up to the NFS server, otherwise
> > direct placement is impossible.
> 
> Thinking on this a little more, I think you are saying the new shared
> decoder is adequate for NFSv2 and NFSv3, but not for NFSv4.

Yes, it would be a regression in the v4 case, but not in the v2 or v3
cases.

> Would you
> accept a patch that kept the NFSv2 and NFSv3 parts of this patch, but
> dropped the NFSv4-related hunks?
> 
> Likewise for the symlink decoder patch?

I'll have to take another look, but, could be.

> And we can still use a transport call-out in the XDR decoders, just as
> we have discussed. (I think I've misremembered our discussion about
> chaining xdr_bufs).

Maybe chaining xdr bufs would make sense, I haven't thought about it.

But we currently handle multiple IOs per compound without chaining xdr
bufs.

--b.