[pnfs] sequence xdr fix
Benny Halevy
bhalevy at panasas.com
Wed Oct 3 18:30:29 EDT 2007
Dean, a couple minor nits: it looks like long lines are
wrapped in this patch and tabs were expanded to spaces.
This makes applying the patch much harder.
Please see more comments inline below...
Thanks,
Benny
dean hildebrand wrote:
> Here is a quick patch which removes duplicate sequence xdr allocations
> from all operations using sessions. With this patch, our read
> performance increases 10x since the data doesn't need to be copied on
> the client after it is received.
>
> This is something we can talk about at the bakeathon, but for now I
> removed the global variable nr_sequence_quads and just set the buffer
> sizes statically. We need to develop an approach that will support
> NFSv4.0 and NFSv4.1.
I think that the encoding function could return their respective reply
size in case of success or a negative status in case they fail.
Then the minorversion specific entry points can accumulate their
specific headers reply size and pass it on to the common encode function.
For example, nfs40_xdr_enc_read would sum only encode_compound_hdr while
nfs41_xdr_enc_read would sum the return value of encode_compound_hdr and
encode_sequence. Both will pass the sum to nfs4_xdr_enc_read which will add to it
the result of encode_putfh and encode_read. This should add up to the correct
replen;
>
>
>
> Description:
> Eliminate duplicate response xdr headers for NFSv4.1 operations
> with regards to sequence id space. For now, I just made every
> static and used the xdr constants from the top of nfs4xdr.c.
> Eliminated global variable nr_sequence_quads to allow a more
> flexible approach in the future that will support NFSv4.0 and
> NFSv4.1. We still need to separate out xdr header sizes between
> NFSv4.0 and NFSv4.1.
>
>
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 6e620ea..9dcd97b 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -60,7 +60,6 @@ #define NFSDBG_FACILITY NFSDBG_XDR
>
> /* Mapping from NFS error code to "errno" error code. */
> #define errno_NFSERR_IO EIO
> -static int nr_sequence_quads = 0;
>
> static int nfs_stat_to_errno(int);
>
> @@ -198,7 +197,7 @@ #define NFS4_enc_readlink_sz (compound_e
> #define NFS4_dec_readlink_sz (compound_decode_hdr_maxsz + \
> decode_sequence_maxsz + \
> decode_putfh_maxsz + \
> - op_decode_hdr_maxsz)
> + op_decode_hdr_maxsz + 1)
Yeah... Is this this for the link string length?
> #define NFS4_enc_readdir_sz (compound_encode_hdr_maxsz + \
> encode_sequence_maxsz +\
> encode_putfh_maxsz + \
> @@ -1300,10 +1299,9 @@ static int encode_readdir(struct xdr_str
> /* set up reply kvec
> * toplevel_status + taglen + rescount + OP_PUTFH + status
> * + OP_READDIR + status + verifer(2) = 9
> - * Add to this the size for the sequence op; 0 if no sessions,
> - * 11 otherwise
> + * TODO: Session sequence space is statically added.
> */
> - replen = (RPC_REPHDRSIZE + auth->au_rslack + 9 +
> nr_sequence_quads) << 2;
> + replen = (RPC_REPHDRSIZE + auth->au_rslack + NFS4_dec_readdir_sz) << 2;
> xdr_inline_pages(&req->rq_rcv_buf, replen, readdir->pages,
> readdir->pgbase, readdir->count);
> dprintk("%s: inlined page args = (%u, %p, %u, %u)\n",
> @@ -1325,9 +1323,9 @@ static int encode_readlink(struct xdr_st
> /* set up reply kvec
> * toplevel_status + taglen + rescount + OP_PUTFH + status
> * + OP_READLINK + status + string length = 8
> - * Similar to readdir, add space for the sequence op
> + * TODO: Session sequence space is statically added.
> */
> - replen = (RPC_REPHDRSIZE + auth->au_rslack + 8 +
> nr_sequence_quads) << 2;
> + replen = (RPC_REPHDRSIZE + auth->au_rslack + NFS4_dec_readlink_sz) << 2;
> xdr_inline_pages(&req->rq_rcv_buf, replen, readlink->pages,
> readlink->pgbase, readlink->pglen);
>
> @@ -2626,9 +2624,9 @@ static int nfs4_xdr_enc_read(struct rpc_
> /* set up reply kvec
> * toplevel status + taglen=0 + rescount + OP_PUTFH + status
> * + OP_READ + status + eof + datalen = 9
> - * Add space for sequence op too.
> + * TODO: Session sequence space is statically added.
> */
> - replen = (RPC_REPHDRSIZE + auth->au_rslack + NFS4_dec_read_sz +
> nr_sequence_quads) << 2;
> + replen = (RPC_REPHDRSIZE + auth->au_rslack + NFS4_dec_read_sz) << 2;
> xdr_inline_pages(&req->rq_rcv_buf, replen,
> args->pages, args->pgbase, args->count);
> out:
> @@ -2729,8 +2727,10 @@ nfs4_xdr_enc_getacl(struct rpc_rqst *req
> if (status)
> goto out;
> status = encode_getattr_two(xdr, FATTR4_WORD0_ACL, 0);
> - /* set up reply buffer: */
> - replen = (RPC_REPHDRSIZE + auth->au_rslack + NFS4_dec_getacl_sz +
> nr_sequence_quads) << 2;
> + /* set up reply buffer:
> + * TODO: Session sequence space is statically added.
> + */
> + replen = (RPC_REPHDRSIZE + auth->au_rslack + NFS4_dec_getacl_sz) << 2;
> xdr_inline_pages(&req->rq_rcv_buf, replen,
> args->acl_pages, args->acl_pgbase, args->acl_len);
> out:
> @@ -3246,8 +3246,9 @@ static int nfs4_xdr_enc_fs_locations(str
> /* set up reply
> * toplevel_status + OP_PUTFH + status
> * + OP_LOOKUP + status + OP_GETATTR + status = 7
> + * TODO: Session sequence space is statically added.
> */
> - replen = (RPC_REPHDRSIZE + auth->au_rslack + 7 +
> nr_sequence_quads) << 2;
> + replen = (RPC_REPHDRSIZE + auth->au_rslack +
> NFS4_dec_fs_locations_sz) << 2;
> xdr_inline_pages(&req->rq_rcv_buf, replen, &args->page,
> 0, PAGE_SIZE);
> out:
> @@ -6761,10 +6762,6 @@ static int nfs41_xdr_dec_create_session(
>
> status = decode_create_session(&xdr, res);
>
> - if (!status) {
> - /* Decode session succeeded; set nr_sequence_quads */
> - nr_sequence_quads = decode_sequence_maxsz;
> - }
> return status;
> }
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
More information about the pNFS
mailing list