Hello!
> Race 1: w/o RCU
> Cpu 0: is in neigh_lookup
> gets read_lock()
> finds entry
> ++refcount to 2
>
On Wed, 30 Aug 2006 03:21:26 +0400
Alexey Kuznetsov <[EMAIL PROTECTED]> wrote:
> Hello!
>
> > This should not be any more racy than the existing code.
>
> Existing code is not racy.
Race 1: w/o RCU
Cpu 0: is in neigh_lookup
Hello!
Yes, I forgot to say I take back my suggestion about atomic_inc_test_zero().
It would not work.
Seems, it is possible to add some barriers around setting n->dead
and testing it in neigh_lookup_rcu(), but it would be scary and ugly.
To be honest, I just do not know how to do this. :-)
-
To
Hello!
> This should not be any more racy than the existing code.
Existing code is not racy.
Critical place is interpretation of refcnt==1. Current code assumes,
that when refcnt=1 and entry is in hash table, nobody can take this
entry (table is locked). So, it can be unlinked from the table.
S
Here is the full version of neigh_lookup_rcu().
The race of interest is what happens when neigh_lookup_rcu() returns a table
entry and it gets deleted at the same time. See Documentation/RCU/listRCU.txt
for more info.
There are a couple different scenario's.
1) update or reader looking at dead e
Hello!
> > Also, probably, it makes sense to add neigh_lookup_light(), which does
> > not take refcnt, but required to call
> > neigh_release_light() (which is just rcu_read_unlock_bh()).
>
> Which code paths would that make sense on?
> fib_detect_death (ok)
> infiniband (ok)
>
On Wed, 30 Aug 2006 01:17:22 +0400
Alexey Kuznetsov <[EMAIL PROTECTED]> wrote:
> Hello!
>
> > atomic_inc_and_test is true iff result is zero, so that won't work.
>
> I meant atomic_inc_not_zero(), as Martin noticed.
>
>
> > But the following should work:
> >
> > hlist_for_each_entry_rcu(n
Hello!
> atomic_inc_and_test is true iff result is zero, so that won't work.
I meant atomic_inc_not_zero(), as Martin noticed.
> But the following should work:
>
> hlist_for_each_entry_rcu(n, tmp, &tbl->hash_buckets[hash_val], hlist) {
> if (dev == n->dev && !memcmp(n->prim
On Tue, 29 Aug 2006 20:34:01 +0200
Martin Josefsson <[EMAIL PROTECTED]> wrote:
> On Tue, 2006-08-29 at 11:22 -0700, Stephen Hemminger wrote:
>
> > > Probably, you should do atomic_inc_and_test() here and restart lookup,
> > > if it fails.
> > >
> > > Alexey
> >
> > atomic_inc_and_test is true i
On Tue, 29 Aug 2006 19:28:16 +0400
Alexey Kuznetsov <[EMAIL PROTECTED]> wrote:
> Hello!
>
> > @@ -346,8 +354,8 @@ struct neighbour *neigh_lookup(struct ne
> >
> > NEIGH_CACHE_STAT_INC(tbl, lookups);
> >
> > - read_lock_bh(&tbl->lock);
> > - hlist_for_each_entry(n, tmp, &tbl->hash_b
On Tue, 2006-08-29 at 11:22 -0700, Stephen Hemminger wrote:
> > Probably, you should do atomic_inc_and_test() here and restart lookup,
> > if it fails.
> >
> > Alexey
>
> atomic_inc_and_test is true iff result is zero, so that won't work.
Wouldn't atomic_inc_not_zero() do what you want?
--
/M
Hello!
> @@ -346,8 +354,8 @@ struct neighbour *neigh_lookup(struct ne
>
> NEIGH_CACHE_STAT_INC(tbl, lookups);
>
> - read_lock_bh(&tbl->lock);
> - hlist_for_each_entry(n, tmp, &tbl->hash_buckets[hash_val], hlist) {
> + rcu_read_lock();
> + hlist_for_each_entry_rcu(n,
Use RCU to allow for lock less access to the neighbour table.
This should speedup the send path because no atomic operations
will be needed to lookup ARP entries, etc.
Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]>
---
include/net/neighbour.h |4 -
net/core/neighbour.c| 158 +
13 matches
Mail list logo
