On Fri, Jun 6, 2014 at 10:56 AM, Eric Dumazet [off-list ref] wrote:

On Fri, 2014-06-06 at 10:44 -0700, Alexei Starovoitov wrote:

quoted

On Fri, Jun 6, 2014 at 9:16 AM, Eric Dumazet [off-list ref] wrote:

quoted

quoted

We should either :

1) use xchg() and no lock at all to change sk_dst_cache, as we did for
sk_rx_dst ( cf udp_sk_rx_dst_set() )

2) No longer use sk_dst_lock, and always use the socket lock
(sk->sk_lock.slock) instead.

Probably needs some combination of both.

I do not think so.

If we use xchg(), then we do not need anything else.

If we use a spinlock, then xchg() seems useless.

Any combination is buggy.

In the past, UDP transmit was holding socket lock,
but we added a lockless path, and I suppose more people
use a UDP socket from different threads.

Then, Steffen added the ip4_datagram_release_cb() thing,
increasing the chance of mixing both 'protections'.

Setting sk_dst_cache should hardly be a hot path.

yeah. if we use sk_lock.slock then xchg is obviously not needed.
By 'both' I meant to do xchg() and get rid of sk_dst_lock to speed it up.
Though not worth going too fancy if speed is not needed.