3-word attributes encoding

3-word attributes encoding

[Benny Halevy]

Bruce,

As I mentioned in a reply to Trond,
"nfsd41: support for 3-word long attribute bitmask"
changes the server fattr encoding logic so it may send
back a bitmap of length 1, even if the client sent a
bitmap of length 2, if the second word of the bitmap
is zero.  Although I think this a valid implementation
and other servers may do the same, it seems sub-optimal
for the client's decoding of acl of fs_locations.

It's pretty easy to revert to the old behavior on the server
by always returning at least two bitmap words, or, if we
keep the bitmap length, not just the val, in nfsd4_decode_bitmap
we can return a bitmap of the same length in the reply.

Let me know what you think...

Other than that,
git://linux-nfs.org/~bhalevy/linux-pnfs.git nfsd41-for-2.6.30
holds version 3 of the nfsd41 patchset, rebased onto your
update for-2.6.30 branch, and with these cleanup patches squashed-in:

[PATCH 1/3] SQUASHME: get callback minorversion from fore channel's
[PATCH 2/3] SQUASHME: nfsd41: use nfsd4_cb_sequence for callback minorversion
[PATCH 3/3] SQUASHME: Revert "sunrpc: add cl_private field to struct rpc_clnt"

Benny

Re: [pnfs] 3-word attributes encoding

[J. Bruce Fields]

On Mon, Mar 30, 2009 at 05:37:37PM +0300, Benny Halevy wrote:
> Bruce,
> 
> As I mentioned in a reply to Trond,
> "nfsd41: support for 3-word long attribute bitmask"
> changes the server fattr encoding logic so it may send
> back a bitmap of length 1, even if the client sent a
> bitmap of length 2, if the second word of the bitmap
> is zero.  Although I think this a valid implementation
> and other servers may do the same, it seems sub-optimal
> for the client's decoding of acl of fs_locations.

"suboptimal" means it does some extra memcpy'ing?

> It's pretty easy to revert to the old behavior on the server
> by always returning at least two bitmap words, or, if we
> keep the bitmap length, not just the val, in nfsd4_decode_bitmap
> we can return a bitmap of the same length in the reply.

Odd thing to have to do, but OK. --b.  

> 
> Let me know what you think...
> 
> Other than that,
> git://linux-nfs.org/~bhalevy/linux-pnfs.git nfsd41-for-2.6.30
> holds version 3 of the nfsd41 patchset, rebased onto your
> update for-2.6.30 branch, and with these cleanup patches squashed-in:
> 
> [PATCH 1/3] SQUASHME: get callback minorversion from fore channel's
> [PATCH 2/3] SQUASHME: nfsd41: use nfsd4_cb_sequence for callback minorversion
> [PATCH 3/3] SQUASHME: Revert "sunrpc: add cl_private field to struct rpc_clnt"
> 
> Benny
> _______________________________________________
> pNFS mailing list
> pNFS@linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs

Re: [pnfs] 3-word attributes encoding

[Benny Halevy]

On Mar. 31, 2009, 4:44 +0300, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> On Mon, Mar 30, 2009 at 05:37:37PM +0300, Benny Halevy wrote:
>> Bruce,
>>
>> As I mentioned in a reply to Trond,
>> "nfsd41: support for 3-word long attribute bitmask"
>> changes the server fattr encoding logic so it may send
>> back a bitmap of length 1, even if the client sent a
>> bitmap of length 2, if the second word of the bitmap
>> is zero.  Although I think this a valid implementation
>> and other servers may do the same, it seems sub-optimal
>> for the client's decoding of acl of fs_locations.
> 
> "suboptimal" means it does some extra memcpy'ing?

Yes, I believe so.

> 
>> It's pretty easy to revert to the old behavior on the server
>> by always returning at least two bitmap words, or, if we
>> keep the bitmap length, not just the val, in nfsd4_decode_bitmap
>> we can return a bitmap of the same length in the reply.
> 
> Odd thing to have to do, but OK. --b.  

What would you prefer to do?
Revert to old behavior by returning at least two bitmap words
or use the args bitmap length for encoding the res?

Benny

> 
>> Let me know what you think...
>>
>> Other than that,
>> git://linux-nfs.org/~bhalevy/linux-pnfs.git nfsd41-for-2.6.30
>> holds version 3 of the nfsd41 patchset, rebased onto your
>> update for-2.6.30 branch, and with these cleanup patches squashed-in:
>>
>> [PATCH 1/3] SQUASHME: get callback minorversion from fore channel's
>> [PATCH 2/3] SQUASHME: nfsd41: use nfsd4_cb_sequence for callback minorversion
>> [PATCH 3/3] SQUASHME: Revert "sunrpc: add cl_private field to struct rpc_clnt"
>>
>> Benny
>> _______________________________________________
>> pNFS mailing list
>> pNFS@linux-nfs.org
>> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs

Re: [pnfs] 3-word attributes encoding

[J. Bruce Fields]

On Tue, Mar 31, 2009 at 09:36:08PM +0300, Benny Halevy wrote:
> On Mar. 31, 2009, 4:44 +0300, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> > On Mon, Mar 30, 2009 at 05:37:37PM +0300, Benny Halevy wrote:
> >> Bruce,
> >>
> >> As I mentioned in a reply to Trond,
> >> "nfsd41: support for 3-word long attribute bitmask"
> >> changes the server fattr encoding logic so it may send
> >> back a bitmap of length 1, even if the client sent a
> >> bitmap of length 2, if the second word of the bitmap
> >> is zero.  Although I think this a valid implementation
> >> and other servers may do the same, it seems sub-optimal
> >> for the client's decoding of acl of fs_locations.
> > 
> > "suboptimal" means it does some extra memcpy'ing?
> 
> Yes, I believe so.
> 
> > 
> >> It's pretty easy to revert to the old behavior on the server
> >> by always returning at least two bitmap words, or, if we
> >> keep the bitmap length, not just the val, in nfsd4_decode_bitmap
> >> we can return a bitmap of the same length in the reply.
> > 
> > Odd thing to have to do, but OK. --b.  
> 
> What would you prefer to do?
> Revert to old behavior by returning at least two bitmap words
> or use the args bitmap length for encoding the res?

Whichever requires the least code?  I don't know.

If the client really needs this, then it needs to be agreed on by more
than just the linux server.  It's unfortunate if making this arbitrary
undocumented choice of result encoding makes a significant difference to
the client's performance.

--b.

Re: [pnfs] 3-word attributes encoding

[Benny Halevy]

On Mar. 31, 2009, 22:42 +0300, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> On Tue, Mar 31, 2009 at 09:36:08PM +0300, Benny Halevy wrote:
>> On Mar. 31, 2009, 4:44 +0300, "J. Bruce Fields" <bfields@fieldses.org> wrote:
>>> On Mon, Mar 30, 2009 at 05:37:37PM +0300, Benny Halevy wrote:
>>>> Bruce,
>>>>
>>>> As I mentioned in a reply to Trond,
>>>> "nfsd41: support for 3-word long attribute bitmask"
>>>> changes the server fattr encoding logic so it may send
>>>> back a bitmap of length 1, even if the client sent a
>>>> bitmap of length 2, if the second word of the bitmap
>>>> is zero.  Although I think this a valid implementation
>>>> and other servers may do the same, it seems sub-optimal
>>>> for the client's decoding of acl of fs_locations.
>>> "suboptimal" means it does some extra memcpy'ing?
>> Yes, I believe so.
>>
>>>> It's pretty easy to revert to the old behavior on the server
>>>> by always returning at least two bitmap words, or, if we
>>>> keep the bitmap length, not just the val, in nfsd4_decode_bitmap
>>>> we can return a bitmap of the same length in the reply.
>>> Odd thing to have to do, but OK. --b.  
>> What would you prefer to do?
>> Revert to old behavior by returning at least two bitmap words
>> or use the args bitmap length for encoding the res?
> 
> Whichever requires the least code?  I don't know.

The former seems simpler to code.

> 
> If the client really needs this, then it needs to be agreed on by more
> than just the linux server.  It's unfortunate if making this arbitrary
> undocumented choice of result encoding makes a significant difference to
> the client's performance.

I think that currently it affects only getacl and I have no measurements
of to what extent the effect of performance is.

Benny

> 
> --b.

Re: [pnfs] 3-word attributes encoding

[J. Bruce Fields]

On Tue, Mar 31, 2009 at 11:00:50PM +0300, Benny Halevy wrote:
> On Mar. 31, 2009, 22:42 +0300, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> > On Tue, Mar 31, 2009 at 09:36:08PM +0300, Benny Halevy wrote:
> >> On Mar. 31, 2009, 4:44 +0300, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> >>> On Mon, Mar 30, 2009 at 05:37:37PM +0300, Benny Halevy wrote:
> >>>> Bruce,
> >>>>
> >>>> As I mentioned in a reply to Trond,
> >>>> "nfsd41: support for 3-word long attribute bitmask"
> >>>> changes the server fattr encoding logic so it may send
> >>>> back a bitmap of length 1, even if the client sent a
> >>>> bitmap of length 2, if the second word of the bitmap
> >>>> is zero.  Although I think this a valid implementation
> >>>> and other servers may do the same, it seems sub-optimal
> >>>> for the client's decoding of acl of fs_locations.
> >>> "suboptimal" means it does some extra memcpy'ing?
> >> Yes, I believe so.
> >>
> >>>> It's pretty easy to revert to the old behavior on the server
> >>>> by always returning at least two bitmap words, or, if we
> >>>> keep the bitmap length, not just the val, in nfsd4_decode_bitmap
> >>>> we can return a bitmap of the same length in the reply.
> >>> Odd thing to have to do, but OK. --b.  
> >> What would you prefer to do?
> >> Revert to old behavior by returning at least two bitmap words
> >> or use the args bitmap length for encoding the res?
> > 
> > Whichever requires the least code?  I don't know.
> 
> The former seems simpler to code.
> 
> > 
> > If the client really needs this, then it needs to be agreed on by more
> > than just the linux server.  It's unfortunate if making this arbitrary
> > undocumented choice of result encoding makes a significant difference to
> > the client's performance.
> 
> I think that currently it affects only getacl and I have no measurements
> of to what extent the effect of performance is.

I'd rather just not worry about it.  If it turns out to be a problem
then we can try to figure out a less fragile solution.

--b.