On Thu, Jan 24, 2013 at 05:22:43AM +0000, Myklebust, Trond wrote: > On Wed, 2013-01-23 at 17:55 -0500, J. Bruce Fields wrote: > > From: "J. Bruce Fields" <bfields@xxxxxxxxxx> > > > > Write xdr encoding primitives that can handle crossing page boundaries, > > and use them in nfsd4_encode_fattr. > > > > The main practical advantage for now is that we can return arbitrarily > > large ACLs (well, up to our maximum rpc size). However, compounds with > > other operations following such a getattr may fail. > > > > Eventually we plan to use the same xdr code through v4 at least. > > > > Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx> > > --- > > fs/nfsd/nfs4proc.c | 6 +- > > fs/nfsd/nfs4xdr.c | 705 +++++++++++++++++++++++++++++++++++------------------ > > fs/nfsd/xdr4.h | 18 +- > > 3 files changed, 483 insertions(+), 246 deletions(-) > > > > Why do these belong in the NFS server layer, and not the SUNRPC layer? > > Also, why are you inventing an entirely new server-specific structure > instead of just reusing the existing struct xdr_stream? The xdr_stream > already has support for bounce buffers to deal with crossing page > boundaries (although I've only implemented the decode side of the > equation so far)... I thought a little about using a bounce buffer. What do you see as the advantage? A couple issues I ran into: I want to be able to encode fields out of order, and to measure the offset between two points in the buffer. For example, we encode attributes by encoding a dummy length field at the beginning, encoding the attributes, then calculating the length of the encoded data and going back to reencode the length field; roughly: nfsd4_encode_fattr: ... encode bitmap attrlenp = p; ... encode attributes attrlen = (char *)p - (char *)attrlenp - 4; *attrlenp = htonl(attrlen); Replacing p by a (__be32 *, page **) pair allows me to do basically the same thing: attrlenp = p; ... attrlen = svcxdr_byte_offset(&attrlenp, &xdr->ptr) - 4; write32(&attrlenp, attrlen); The "pointers" have all the information I need to calculate the length, and I don't have to worry about how many pages the attributes spanned or whether attrlenp itself spans a page boundary. (OK, actually it won't because it's 4 bytes. But we do the same thing elsewhere, e.g. when encoding readdir cookies.) Using a bounce buffer also sets an arbitrary limits on how much space I can reserve at once. Maybe that's not a problem in practice. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html