On Fri, 3 Sep 2004 23:36:23 +1000
Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:
> On Thu, Sep 02, 2004 at 11:06:05PM +1000, herbert wrote:
> >
> > > Can you work on the next bit you mentioned, making
> > > sure the corresponding idev is still alive when we add
> > > a neighbour with its neigh_parms to the hash table?
> >
> > Sure. Actually I prefer to do it by ref counting neigh_parms directly.
> > I'll send you a patch soon.
>
> Here is the patch.
>
> I've added a refcnt on neigh_parms as well as a dead flag. The latter
> is checked under the tbl_lock before adding a neigh entry to the hash
> table.
>
> The non-trivial bit of the patch is the first chunk of net/core/neighbour.c.
> I removed that line because not doing so would mean that I have to drop
> the reference to the parms right there. That would've lead to race
> conditions since many places dereference neigh->parms without holding
> locks. It's also unnecessary to reset n->parms since we're no longer
> in a hurry to see it go due to the new ref counting.
>
> You'll also notice that I've put all dereferences of dev->*_ptr under
> the rcu_read_lock(). Without this we may get a neigh_parms that's
> already been released.
I haven't looked at the exact code in detail, but don't you need
use rcu_dereference() as well to make sure and get the smp_read_barrier_depends
on Alpha.
|