Re: [PATCH] nfsd: no need to unhash_stid before free

[Benny Halevy <bhalevy@primarydata.com>]

On 2013-10-28 20:32, J. Bruce Fields wrote:
> On Mon, Oct 14, 2013 at 09:01:55AM +0300, Benny Halevy wrote:
>> idr_remove is about to be called before kmem_cache_free so unhashing it
>> is redundant
> 
> This leaves only two unhash_stid callers, in release_lock_stateid and
> unhash_stid, both called just before destroying the stateid, so perhaps
> we should remove those and unhash_stid?

In my state lock elimination patchset I actually keep using unhash_stid
after refactoring non-blocking unhashing from the blocking release
so I don't think it's worth it to remove the call.
That said, we can still open code it but encapsulating the assignment
in unhash_stid() which the compiler can inline anyway seems cleaner to me.

Benny

> 
> --b.
> 
>>
>> Signed-off-by: Benny Halevy <bhalevy@primarydata.com>
>> ---
>>  fs/nfsd/nfs4state.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 0874998..06984e3 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -668,7 +668,6 @@ static void unhash_open_stateid(struct nfs4_ol_stateid *stp)
>>  static void release_open_stateid(struct nfs4_ol_stateid *stp)
>>  {
>>  	unhash_open_stateid(stp);
>> -	unhash_stid(&stp->st_stid);
>>  	free_generic_stateid(stp);
>>  }
>>  
>> @@ -690,7 +689,6 @@ static void release_last_closed_stateid(struct nfs4_openowner *oo)
>>  	struct nfs4_ol_stateid *s = oo->oo_last_closed_stid;
>>  
>>  	if (s) {
>> -		unhash_stid(&s->st_stid);
>>  		free_generic_stateid(s);
>>  		oo->oo_last_closed_stid = NULL;
>>  	}
>> @@ -3998,10 +3996,9 @@ static void nfsd4_close_open_stateid(struct nfs4_ol_stateid *s)
>>  
>>  	nfsd4_close_open_stateid(stp);
>>  
>> -	if (cstate->minorversion) {
>> -		unhash_stid(&stp->st_stid);
>> +	if (cstate->minorversion)
>>  		free_generic_stateid(stp);
>> -	} else
>> +	else
>>  		oo->oo_last_closed_stid = stp;
>>  
>>  	if (list_empty(&oo->oo_owner.so_stateids)) {
>> -- 
>> 1.8.3.1
>>
> --
> 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
>