Re: rpcauth_lookup_credcache() lock contentions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Fengguang Wu [fengguang.wu@xxxxxxxxx] wrote:
> > diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
> > index 727e506..645769e 100644
> > --- a/net/sunrpc/auth.c
> > +++ b/net/sunrpc/auth.c
> > @@ -364,13 +364,24 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
> >  	hlist_for_each_entry_rcu(entry, pos, &cache->hashtable[nr], cr_hash) {
> >  		if (!entry->cr_ops->crmatch(acred, entry, flags))
> >  			continue;
> > -		spin_lock(&cache->lock);
> >  		if (test_bit(RPCAUTH_CRED_HASHED, &entry->cr_flags) == 0) {
> > -			spin_unlock(&cache->lock);
> >  			continue;
> >  		}
> > -		cred = get_rpccred(entry);
> > -		spin_unlock(&cache->lock);
> > +		cred = entry;

I know very little about this code, so take this with grain of salt!
The cred symbol is set above but there is no guaranty that you will use
it.  If you don't win, you should set it to NULL.

> > +		if (atomic_add_return(1, &cred->cr_count) == 1) {
> > +			/*
> > +			 * When the count was 0 we could race with the GC.
> > +			 * Take the lock and recheck.
> > +			 */
> > +			spin_lock(&cache->lock);
> > +			if (!test_bit(RPCAUTH_CRED_HASHED, &entry->cr_flags)) {
> > +				/* Lost the race. */
> > +				atomic_dec(&cred->cr_count);
> > +				spin_unlock(&cache->lock);
> > +				continue;

If you execute the above "continue" statement, you will have stale "cred"
where the original code may end up with NULL cred. I don't know if that
can happen really though.

> > +			}
> > +			spin_unlock(&cache->lock);
> > +		}
> >  		break;

Setting cred to entry just before the above break and using entry in the
above code should be fine, I believe.

> >  	}
> >  	rcu_read_unlock();
> > -- 
> > 1.7.7
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux