Re: [PATCH 2/2] We should clear NFS_DELEGATED_STATE after return delegation

[Li Yewang <lyw@cn.fujitsu.com>]

hi Trond

    Do you have plan to fix this bug.
    Because our custom want to solve it.

    If you have any comment or advice please let us know.

    thank you.


At 2010-9-9 4:37, Trond Myklebust wrote:
> On Wed, 2010-09-08 at 11:11 +0800, Bian Naimeng wrote:
>> If there are not any delegation at this inode, we should clear stateid's
>> NFS_DELEGATED_STATE when update it.
>>
>> Signed-off-by: Bian Naimeng<biannm@cn.fujitsu.com>
>>
>> ---
>>   fs/nfs/delegation.c |    1 +
>>   fs/nfs/nfs4proc.c   |   12 +++++++++++-
>>   2 files changed, 12 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
>> index b9c3c43..62f296e 100644
>> --- a/fs/nfs/delegation.c
>> +++ b/fs/nfs/delegation.c
>> @@ -106,6 +106,7 @@ again:
>>   			continue;
>>   		if (memcmp(state->stateid.data, stateid->data, sizeof(state->stateid.data)) != 0)
>>   			continue;
>> +		clear_bit(NFS_DELEGATED_STATE,&state->flags);
>>   		get_nfs_open_context(ctx);
>>   		spin_unlock(&inode->i_lock);
>>   		err = nfs4_open_delegation_recall(ctx, state, stateid);
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 089da5b..f7e45b4 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -919,8 +919,18 @@ static int update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stat
>>
>>   	rcu_read_lock();
>>   	deleg_cur = rcu_dereference(nfsi->delegation);
>> -	if (deleg_cur == NULL)
>> +	if (deleg_cur == NULL) {
>> +		if (delegation == NULL&&  open_stateid != NULL) {
>
> Well... What I really meant was that we should make sure that we don't
> get into this situation.
>
> I think the clear_bit() should be unconditional if delegation == NULL,
> but if the (delegation == NULL&&  open_stateid == NULL) _can_ occur,
> then we should probably mark the nfs4_state for recovery using
> nfs4_state_mark_reclaim_nograce(), and then fire of a recovery thread.
>
>> +			/*
>> +			 * FIXME: If the state has NFS_DELEGATED_STATE bit
>> +			 * we catch a race. Maybe should recover its open
>> +			 * stateid, now we just clear the NFS_DELEGATED_STATE
>> +			 * bit.
>> +			 */
>> +			clear_bit(NFS_DELEGATED_STATE,&state->flags);
>> +		}
>>   		goto no_delegation;
>> +	}
>>
>>   	spin_lock(&deleg_cur->lock);
>>   	if (nfsi->delegation != deleg_cur ||
>> --
>> 1.7.0
>>
>>
>>>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>>>>> index 36400d3..c0c0320 100644
>>>>>> --- a/fs/nfs/nfs4proc.c
>>>>>> +++ b/fs/nfs/nfs4proc.c
>>>>>> @@ -900,8 +900,18 @@ static int update_open_stateid(struct nfs4_state *state, nfs4_stateid *open_stat
>>>>>>
>>>>>>   	rcu_read_lock();
>>>>>>   	deleg_cur = rcu_dereference(nfsi->delegation);
>>>>>> -	if (deleg_cur == NULL)
>>>>>> +	if (deleg_cur == NULL) {
>>>>>> +		if (delegation == NULL&&
>>>>>> +		    test_bit(NFS_DELEGATED_STATE,&state->flags)) {
>>>>> Any reason why we can't ditch the 'test_bit'?
>>>>>
>>>>    Because i am not sure it's ok that we just clear_bit at here.
>>>>    If you think it's ok, i'd be fine with removing this test_bit.
>>>
>>> How is it different from just clearing the bit?
>>>
>>>>>> +			/*FIXME: If the state has NFS_DELEGATED_STATE bit,
>>>>>> +			 * we catch a race. Maybe should recover its open
>>>>>> +			 * stateid, here we just clear the NFS_DELEGATED_STATE
>>>>>> +			 * bit.
>>>>> Can this ever be called with both deleg_cur==NULL and
>>>>> open_stateid==NULL? If so, then we still have a bug.
>>>>>
>>>>    Yes, i will add a checking. Thanks very much.
>>>>
>>>>>> +			 */
>>>>>> +			clear_bit(NFS_DELEGATED_STATE,&state->flags);
>>>>>> +		}
>>>>>>   		goto no_delegation;
>>>>>> +	}
>>>>>>
>>>>>>   	spin_lock(&deleg_cur->lock);
>>>>>>   	if (nfsi->delegation != deleg_cur ||
>>>
>>> Cheers
>>>    Trond
>>>
>>>
>>>
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>

-- 
Regards
Li Yewang
--------------------------------------------------
Li Yewang
Development Dept.I
Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)
No. 6 Wenzhu Road, Nanjing, 210012, China
PHONE: +86+25-86630566-8507
COINS: 7998-8507
FAX: +86+25-83317685
MAIL: lyw@cn.fujitsu.com
--------------------------------------------------