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

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

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

  Thanks very much for your explainning.

> I think the clear_bit() should be unconditional if delegation == NULL,

  en..., i have a question.

  If the (deleg_cur == NULL && delegation == NULL) occured, that means
  there are not any delegation at this nfs_inode, i think this state
  do not need a NFS_DELEGATED_STATE bit anymore, is it right?

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

  It looks like that (delegation == NULL && open_stateid == NULL) can not
  occur at our kernel.

  And..., would you tell me  why we must start recovery with using
  nfs4_state_mark_reclaim_nograce,  are there any hint tell us that this
  state has expired ?

-- 
Regards
Bian Naimeng

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

-- 
Regards
Bian Naimeng