Re: [PATCH net-next-2.6] net: build_ehash_secret() and rt_bind_peer() cleanups

[Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>]

* Changli Gao (xiaosuo@gmail.com) wrote:
> On Thu, Aug 19, 2010 at 11:46 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Now cmpxchg() is available on all arches, we can use it in
> > build_ehash_secret() and rt_bind_peer() instead of using spinlocks.
> >
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > CC: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > ---
[...]
> >  void rt_bind_peer(struct rtable *rt, int create)
> >  {
> > -       static DEFINE_SPINLOCK(rt_peer_lock);
> >        struct inet_peer *peer;
> >
> >        peer = inet_getpeer(rt->rt_dst, create);
> >
> > -       spin_lock_bh(&rt_peer_lock);
> > -       if (rt->peer == NULL) {
> > -               rt->peer = peer;
> > -               peer = NULL;
> > -       }
> > -       spin_unlock_bh(&rt_peer_lock);
> > -       if (peer)
> > +       peer = cmpxchg(&rt->peer, NULL, peer);
> > +
> > +       if (unlikely(peer))
> 
> It isn't correct, and should be
>      if (unlikely(cmpxchg(&rt->peer, NULL, peer) != NULL))
> 
> >                inet_putpeer(peer);
> >  }

What do you mean by "It isn't correct" ? Eric code and yours perform the
exact same thing. So I assume this is just a code style issue here.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com