Messages in this thread Patch in this message | | | Subject | Re: [PATCH 2/2] packet: convert socket list to RCU | From | Eric Dumazet <> | Date | Fri, 19 Feb 2010 16:16:24 +0100 |
| |
Le jeudi 18 février 2010 à 21:41 -0800, Stephen Hemminger a écrit :
> Convert AF_PACKET to use RCU, eliminating one more reader/writer lock. > > I needed to create some minor additional socket list RCU infrastructure > to make this work. Note: there is no need for a real sk_del_node_init_rcu(), > because sk_del_node_init is doing the equivalent thing to > hlst_del_init_rcu already; but added some comments to try and make that obvious. > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> >
Stephen, I am a bit worried by the interaction between packet_release() and packet_notifier()
With your version, packet_notifier() can run and let another cpu run packet_release() un-contented. Both cpus could manipulate same po (and particularly po->running)
Before your patch, the read_lock() done in packet_notifier() was preventing packet_release() runnning at the same time.
Maybe packet_release() should lock po->bind_lock before manipulating po->running, avoiding a refcount error.
Something like this preliminary patch :
[PATCH] packet: fix a race in packet_release
packet_release() has a potential race with packet_notifier(NETDEV_DOWN), leading to a double __sock_put(). (dev_remove_pack() is safe)
Fix is to always use po->bind_lock before accessing po->running
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 10f7295..b706031 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1271,15 +1271,15 @@ static int packet_release(struct socket *sock) * Unhook packet receive handler. */ + spin_lock(&po->bind_lock); if (po->running) { - /* - * Remove the protocol hook - */ - dev_remove_pack(&po->prot_hook); + __sock_put(sk); po->running = 0; po->num = 0; - __sock_put(sk); - } + spin_unlock(&po->bind_lock); + dev_remove_pack(&po->prot_hook); + } else + spin_unlock(&po->bind_lock); packet_flush_mclist(sk);
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |