Re: [PATCH 5/7] nfsd4: rewrite xdr encoding of attributes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux