netdev
[Top] [All Lists]

Re: [RFC][PATCH] identify in_dev_get rcu read-side critical sections

To: Suzanne Wood <suzannew@xxxxxxxxxx>
Subject: Re: [RFC][PATCH] identify in_dev_get rcu read-side critical sections
From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Date: Sat, 1 Oct 2005 17:12:48 +1000
Cc: Robert.Olsson@xxxxxxxxxxx, davem@xxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, netdev@xxxxxxxxxxx, paulmck@xxxxxxxxxx, walpole@xxxxxxxxxx
In-reply-to: <200510010656.j916ufhT007410@rastaban.cs.pdx.edu>
References: <200510010656.j916ufhT007410@rastaban.cs.pdx.edu>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.9i
On Fri, Sep 30, 2005 at 11:56:41PM -0700, Suzanne Wood wrote:
> 
> But it is interesting to have discarded what was developed yesterday
> to minimize rcu_dereference impact:
>
> >> ----- Original message  -----
> >> From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> >> Date: Fri, 30 Sep 2005 11:19:07 +1000
> >> 
> >> On Thu, Sep 29, 2005 at 06:16:03PM -0700, Paul E. McKenney wrote:
> >>> 
> >>> OK, how about this instead?
> >>> 
> >>> rcu_read_lock();
> >>> in_dev = dev->ip_ptr;
> >>> if (in_dev) {
> >>> atomic_inc(&rcu_dereference(in_dev)->refcnt);
> >>> }
> >>> rcu_read_unlock();
> >>> return in_dev;
> >> 
> >> Looks great. 
> 
> while adding a function call level by wrapping  __in_dev_get_rcu
> with in_dev_get as suggested here.

It might look different, but it should compile to the same result.
GCC should be smart enough to combine the two branches and produce a
memory barrier only when in_dev is not NULL.
 
> The other thing I'd hoped to address in pktgen.c was 
> removing the __in_dev_put() which decrements refcnt 
> while __in_dev_get_rcu() does not increment.

Well spotted.

Here is a patch on top of the last one to fix this bogus decrement.

Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

Thanks,
-- 
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

Attachment: p
Description: Text document

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