[pnfs] [PATCH] initialize nops to 0

Benny Halevy bhalevy at panasas.com
Mon Jun 4 17:40:08 EDT 2007


Marc Eshel wrote:
> What I was getting with nop = 1 is the return code 
> NFS4ERR_OP_NOT_IN_SESSION and it will never get to check for 
> OP_CB_LAYOUTRECALL.

Did it happen with my latest patch?
without it cb_layoutrecall is sent without CB_SEQUENCE
and getting NFS4ERR_OP_NOT_IN_SESSION is correct client behavior.
Can you please try to reproduce with head of line?

Benny

> Marc.
> 
> Benny Halevy <bhalevy at panasas.com> wrote on 06/04/2007 10:58:56 AM:
> 
>> Why is that?
>>
>> There are at least two places in the current code that
>> assume nops is initialized to 1:
>>
>> in process_op:
>>    if (minorversion == 1) {
>>       if (op_nr == OP_CB_SEQUENCE) {
>>          if (nop != 1) {
>>             status = htonl(NFS4ERR_SEQUENCE_POS);
>>             goto out;
>>          }
>>       } else if (nop == 1) {
>>          status = htonl(NFS4ERR_OP_NOT_IN_SESSION);
>>          goto out;
>>       }
>>
>> in nfs4_callback_compound for loop:
>>       if (nops == hdr_arg.nops)
>>          break;
>>
>> the reason nops is not pre-incremented in the current code is
>> right after the for loop we do:
>>    *hdr_res.status = status;
>>    *hdr_res.nops = htonl(nops);
>>
>> In summary, if the reason you think nops should be initialized
>> to zero is purely aesthetic I suggest we drop it or at least
>> postpone after the bakeathon and do it properly.
>>
>> Benny
>>
>> Marc Eshel wrote:
>>> From: Marc Eshel <eshel at almaden.ibm.com>
>>>
>>>
>>> ---
>>>
>>>  fs/nfs/callback_xdr.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
>>> index 4fdcbc9..718110d 100644
>>> --- a/fs/nfs/callback_xdr.c
>>> +++ b/fs/nfs/callback_xdr.c
>>> @@ -618,7 +618,7 @@ static int nfs4_callback_compound(struct
>>>     struct xdr_stream xdr_in, xdr_out;
>>>     uint32_t *p;
>>>     unsigned int status;
>>> -   unsigned int nops = 1;
>>> +   unsigned int nops = 0;
>>>
>>>     dprintk("%s: start\n", __FUNCTION__);
>>>
>>> _______________________________________________
>>> pNFS mailing list
>>> pNFS at linux-nfs.org
>>> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> 



More information about the pNFS mailing list