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

[Bian Naimeng <biannm@cn.fujitsu.com>]

>>> OK. I see agree that is a race, but AFAICS, your fix means that we end
>>> up with an nfs_state structure that has cleared NFS_DELEGATED_STATE, but
>>> that did not recover its open stateid.
>>>
>> Hi trond, what do you think about this one?
> 
> Hi Bian,
> 
> See comments/questions below.
> 

  Thanks for your answer.

>> 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.

>> +			/*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 ||


-- 
Regards
Bian Naimeng