netdev
[Top] [All Lists]

Re: atomic_dec_and_test for child dst needed in dst_destroy?

To: Christoph Lameter <christoph@xxxxxxxxxxx>
Subject: Re: atomic_dec_and_test for child dst needed in dst_destroy?
From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Date: Fri, 8 Apr 2005 07:25:04 +1000
Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>, netdev@xxxxxxxxxxx, pravins@xxxxxxxxxxxxxx, shai@xxxxxxxxxxxxx
In-reply-to: <Pine.LNX.4.58.0504070842420.29731@xxxxxxxxxx>
References: <Pine.LNX.4.58.0504051925250.21486@xxxxxxxxxx> <E1DJ5y2-0003rF-00@xxxxxxxxxxxxxxxxxxxxxxxx> <20050406111721.3ac67605.davem@xxxxxxxxxxxxx> <20050407110724.GA8760@xxxxxxxxxxxxxxxxxxx> <Pine.LNX.4.58.0504070842420.29731@xxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.6+20040907i
On Thu, Apr 07, 2005 at 09:00:51AM -0700, Christoph Lameter wrote:
> 
> In an earlier email you said that there was a slight chance that child
> entries (typically NOHASH from what I can see in the code) could be on the
> gc list.

Only child entries with NOHASH unset can be on the GC list.

> If its not on the hash table and not on the gc list then how could the
> refcnt be > 1 ? Doesnt the refcnt > 1 imply that multiple concurrent
> accesses are possible?

For NOHASH entries refcnt can be greater than one due to dst_pop.
Of course concurrent access is possible.  However, the important
thing is that only the "owner" of an entry can call dst_destroy.

In the case of NOHASH entries, we are the owner.

> Also if this is as you say then both types of entries better be
> treated in a distinct way for clarity in the code. This also avoids
> having a nohash variable:

I never said that it is better to treat them differently.  I wrote
it like this earlier because I still had atomic_dec_and_test.

> if ((dst->flags & DST_NOHASH)) {
>       /* dst not on hash and also not on gc list */
>       atomic_dec(&dst->__refcnt);
>       if (!atomic_read(&dst->refcnt))
>               /* We were the only reference kill child */
>               goto again;
>       /* return child to put it on the gc list */
>       return dst;
> }

BTW this patch is missing a barrier.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

<Prev in Thread] Current Thread [Next in Thread>