* br_forward.c - rcu dereference warning @ 2010-07-27 10:45 Johannes Berg 2010-07-27 12:38 ` Paul E. McKenney 0 siblings, 1 reply; 11+ messages in thread From: Johannes Berg @ 2010-07-27 10:45 UTC (permalink / raw) To: netdev I couldn't find this reported yet, apologies if I missed it. johannes [ 60.140433] =================================================== [ 60.140437] [ INFO: suspicious rcu_dereference_check() usage. ] [ 60.140440] --------------------------------------------------- [ 60.140444] /home/johannes/sys/wireless-testing/net/bridge/br_forward.c:215 invoked rcu_dereference_check() without protection! [ 60.140447] [ 60.140448] other info that might help us debug this: [ 60.140449] [ 60.140452] [ 60.140453] rcu_scheduler_active = 1, debug_locks = 1 [ 60.140457] 2 locks held by Xorg/3083: [ 60.140459] #0: (&im->timer){+.-...}, at: [<ffffffff81058180>] call_timer_fn+0x0/0x2f0 [ 60.140473] #1: (rcu_read_lock_bh){.+....}, at: [<ffffffff813b6c3a>] dev_queue_xmit+0x5a/0x690 [ 60.140484] [ 60.140484] stack backtrace: [ 60.140489] Pid: 3083, comm: Xorg Not tainted 2.6.35-rc6-wl-47665-gc2e2180-dirty #174 [ 60.140492] Call Trace: [ 60.140495] <IRQ> [<ffffffff8107e494>] lockdep_rcu_dereference+0xa4/0xc0 [ 60.140514] [<ffffffffa07617c3>] br_multicast_flood+0x293/0x310 [bridge] [ 60.140531] [<ffffffffa0761877>] br_multicast_deliver+0x17/0x20 [bridge] [ 60.140539] [<ffffffffa07607bc>] br_dev_xmit+0x10c/0x170 [bridge] [ 60.140550] [<ffffffff813b69ba>] dev_hard_start_xmit+0x21a/0x2e0 [ 60.140556] [<ffffffff813b708e>] dev_queue_xmit+0x4ae/0x690 [ 60.140576] [<ffffffff813c0d43>] neigh_resolve_output+0x113/0x250 [ 60.140582] [<ffffffff813eec46>] ip_finish_output+0x2a6/0x570 [ 60.140588] [<ffffffff813ef33c>] ip_mc_output+0x1dc/0x320 [ 60.140593] [<ffffffff813ed7ed>] ip_local_out+0x2d/0x80 [ 60.140600] [<ffffffff81422236>] igmp_send_report+0x1c6/0x200 [ 60.140610] [<ffffffff814238f0>] igmp_timer_expire+0x100/0x130 [ 60.140615] [<ffffffff81058219>] call_timer_fn+0x99/0x2f0 [ 60.140636] [<ffffffff810585e3>] run_timer_softirq+0x173/0x330 [ 60.140641] [<ffffffff8104f114>] __do_softirq+0x114/0x3d0 [ 60.140652] [<ffffffff8100360c>] call_softirq+0x1c/0x50 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: br_forward.c - rcu dereference warning 2010-07-27 10:45 br_forward.c - rcu dereference warning Johannes Berg @ 2010-07-27 12:38 ` Paul E. McKenney 2010-07-27 18:26 ` Stephen Hemminger 0 siblings, 1 reply; 11+ messages in thread From: Paul E. McKenney @ 2010-07-27 12:38 UTC (permalink / raw) To: Johannes Berg; +Cc: netdev On Tue, Jul 27, 2010 at 12:45:53PM +0200, Johannes Berg wrote: > I couldn't find this reported yet, apologies if I missed it. My first guess is that the call to br_multicast_deliver() in br_dev_xmit() needs to be enclosed in rcu_read_lock(), but I have to defer to someone who knows the code better. Another possible fix would be to change the rcu_dereference() in br_multicast_flood() to rcu_dereference_bh(), for example. Thanx, Paul > johannes > > [ 60.140433] =================================================== > [ 60.140437] [ INFO: suspicious rcu_dereference_check() usage. ] > [ 60.140440] --------------------------------------------------- > [ 60.140444] /home/johannes/sys/wireless-testing/net/bridge/br_forward.c:215 invoked rcu_dereference_check() without protection! > [ 60.140447] > [ 60.140448] other info that might help us debug this: > [ 60.140449] > [ 60.140452] > [ 60.140453] rcu_scheduler_active = 1, debug_locks = 1 > [ 60.140457] 2 locks held by Xorg/3083: > [ 60.140459] #0: (&im->timer){+.-...}, at: [<ffffffff81058180>] call_timer_fn+0x0/0x2f0 > [ 60.140473] #1: (rcu_read_lock_bh){.+....}, at: [<ffffffff813b6c3a>] dev_queue_xmit+0x5a/0x690 > [ 60.140484] > [ 60.140484] stack backtrace: > [ 60.140489] Pid: 3083, comm: Xorg Not tainted 2.6.35-rc6-wl-47665-gc2e2180-dirty #174 > [ 60.140492] Call Trace: > [ 60.140495] <IRQ> [<ffffffff8107e494>] lockdep_rcu_dereference+0xa4/0xc0 > [ 60.140514] [<ffffffffa07617c3>] br_multicast_flood+0x293/0x310 [bridge] > [ 60.140531] [<ffffffffa0761877>] br_multicast_deliver+0x17/0x20 [bridge] > [ 60.140539] [<ffffffffa07607bc>] br_dev_xmit+0x10c/0x170 [bridge] > [ 60.140550] [<ffffffff813b69ba>] dev_hard_start_xmit+0x21a/0x2e0 > [ 60.140556] [<ffffffff813b708e>] dev_queue_xmit+0x4ae/0x690 > [ 60.140576] [<ffffffff813c0d43>] neigh_resolve_output+0x113/0x250 > [ 60.140582] [<ffffffff813eec46>] ip_finish_output+0x2a6/0x570 > [ 60.140588] [<ffffffff813ef33c>] ip_mc_output+0x1dc/0x320 > [ 60.140593] [<ffffffff813ed7ed>] ip_local_out+0x2d/0x80 > [ 60.140600] [<ffffffff81422236>] igmp_send_report+0x1c6/0x200 > [ 60.140610] [<ffffffff814238f0>] igmp_timer_expire+0x100/0x130 > [ 60.140615] [<ffffffff81058219>] call_timer_fn+0x99/0x2f0 > [ 60.140636] [<ffffffff810585e3>] run_timer_softirq+0x173/0x330 > [ 60.140641] [<ffffffff8104f114>] __do_softirq+0x114/0x3d0 > [ 60.140652] [<ffffffff8100360c>] call_softirq+0x1c/0x50 > > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: br_forward.c - rcu dereference warning 2010-07-27 12:38 ` Paul E. McKenney @ 2010-07-27 18:26 ` Stephen Hemminger 2010-07-27 20:14 ` Johannes Berg 0 siblings, 1 reply; 11+ messages in thread From: Stephen Hemminger @ 2010-07-27 18:26 UTC (permalink / raw) To: paulmck; +Cc: Johannes Berg, netdev I think this is needed (untested)... ----- Subject: bridge: add rcu_read_lock on transmit Long ago, when bridge was converted to RCU, rcu lock was equivalent to having preempt disabled. RCU has changed a lot since then and bridge code was still assuming the since transmit was called with bottom half disabled, it was RCU safe. Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> --- net/bridge/br_device.c | 4 +++- net/bridge/br_fdb.c | 2 +- net/bridge/br_input.c | 7 +++---- net/bridge/br_stp_bpdu.c | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) --- a/net/bridge/br_device.c 2010-07-27 08:57:43.169399349 -0700 +++ b/net/bridge/br_device.c 2010-07-27 11:18:51.677053426 -0700 @@ -22,7 +22,7 @@ #include <asm/uaccess.h> #include "br_private.h" -/* net device transmit always called with no BH (preempt_disabled) */ +/* net device transmit always called with no BH disabled */ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev) { struct net_bridge *br = netdev_priv(dev); @@ -48,6 +48,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff * skb_reset_mac_header(skb); skb_pull(skb, ETH_HLEN); + rcu_read_lock(); if (is_multicast_ether_addr(dest)) { if (unlikely(netpoll_tx_running(dev))) { br_flood_deliver(br, skb); @@ -67,6 +68,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff * br_flood_deliver(br, skb); out: + rcu_read_unlock(); return NETDEV_TX_OK; } --- a/net/bridge/br_fdb.c 2010-07-27 11:18:30.815320981 -0700 +++ b/net/bridge/br_fdb.c 2010-07-27 11:18:59.597710975 -0700 @@ -214,7 +214,7 @@ void br_fdb_delete_by_port(struct net_br spin_unlock_bh(&br->hash_lock); } -/* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */ +/* No locking or refcounting, assumes caller has rcu_read_lock */ struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br, const unsigned char *addr) { --- a/net/bridge/br_input.c 2010-07-27 11:19:05.266181492 -0700 +++ b/net/bridge/br_input.c 2010-07-27 11:19:29.148163149 -0700 @@ -39,7 +39,7 @@ static int br_pass_frame_up(struct sk_bu netif_receive_skb); } -/* note: already called with rcu_read_lock (preempt_disabled) */ +/* note: already called with rcu_read_lock */ int br_handle_frame_finish(struct sk_buff *skb) { const unsigned char *dest = eth_hdr(skb)->h_dest; @@ -110,7 +110,7 @@ drop: goto out; } -/* note: already called with rcu_read_lock (preempt_disabled) */ +/* note: already called with rcu_read_lock */ static int br_handle_local_finish(struct sk_buff *skb) { struct net_bridge_port *p = br_port_get_rcu(skb->dev); @@ -133,8 +133,7 @@ static inline int is_link_local(const un /* * Return NULL if skb is handled - * note: already called with rcu_read_lock (preempt_disabled) from - * netif_receive_skb + * note: already called with rcu_read_lock from netif_receive_skb */ struct sk_buff *br_handle_frame(struct sk_buff *skb) { --- a/net/bridge/br_stp_bpdu.c 2010-07-27 11:19:34.092573294 -0700 +++ b/net/bridge/br_stp_bpdu.c 2010-07-27 11:19:40.725123403 -0700 @@ -131,7 +131,7 @@ void br_send_tcn_bpdu(struct net_bridge_ /* * Called from llc. * - * NO locks, but rcu_read_lock (preempt_disabled) + * NO locks, but rcu_read_lock */ void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb, struct net_device *dev) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: br_forward.c - rcu dereference warning 2010-07-27 18:26 ` Stephen Hemminger @ 2010-07-27 20:14 ` Johannes Berg 2010-07-27 20:42 ` Stephen Hemminger 0 siblings, 1 reply; 11+ messages in thread From: Johannes Berg @ 2010-07-27 20:14 UTC (permalink / raw) To: Stephen Hemminger; +Cc: paulmck, netdev On Tue, 2010-07-27 at 11:26 -0700, Stephen Hemminger wrote: > -/* net device transmit always called with no BH (preempt_disabled) */ > +/* net device transmit always called with no BH disabled */ "no BH disabled"? > @@ -48,6 +48,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff * > skb_reset_mac_header(skb); > skb_pull(skb, ETH_HLEN); > > + rcu_read_lock(); > if (is_multicast_ether_addr(dest)) { > if (unlikely(netpoll_tx_running(dev))) { > br_flood_deliver(br, skb); > @@ -67,6 +68,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff * > br_flood_deliver(br, skb); > > out: > + rcu_read_unlock(); > return NETDEV_TX_OK; > } > > --- a/net/bridge/br_fdb.c 2010-07-27 11:18:30.815320981 -0700 > +++ b/net/bridge/br_fdb.c 2010-07-27 11:18:59.597710975 -0700 > @@ -214,7 +214,7 @@ void br_fdb_delete_by_port(struct net_br > spin_unlock_bh(&br->hash_lock); > } > > -/* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */ > +/* No locking or refcounting, assumes caller has rcu_read_lock */ > struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br, > const unsigned char *addr) > { > --- a/net/bridge/br_input.c 2010-07-27 11:19:05.266181492 -0700 > +++ b/net/bridge/br_input.c 2010-07-27 11:19:29.148163149 -0700 > @@ -39,7 +39,7 @@ static int br_pass_frame_up(struct sk_bu > netif_receive_skb); > } > > -/* note: already called with rcu_read_lock (preempt_disabled) */ > +/* note: already called with rcu_read_lock */ > int br_handle_frame_finish(struct sk_buff *skb) > { > const unsigned char *dest = eth_hdr(skb)->h_dest; > @@ -110,7 +110,7 @@ drop: > goto out; > } > > -/* note: already called with rcu_read_lock (preempt_disabled) */ > +/* note: already called with rcu_read_lock */ > static int br_handle_local_finish(struct sk_buff *skb) > { > struct net_bridge_port *p = br_port_get_rcu(skb->dev); > @@ -133,8 +133,7 @@ static inline int is_link_local(const un > > /* > * Return NULL if skb is handled > - * note: already called with rcu_read_lock (preempt_disabled) from > - * netif_receive_skb > + * note: already called with rcu_read_lock from netif_receive_skb > */ > struct sk_buff *br_handle_frame(struct sk_buff *skb) > { > --- a/net/bridge/br_stp_bpdu.c 2010-07-27 11:19:34.092573294 -0700 > +++ b/net/bridge/br_stp_bpdu.c 2010-07-27 11:19:40.725123403 -0700 > @@ -131,7 +131,7 @@ void br_send_tcn_bpdu(struct net_bridge_ > /* > * Called from llc. > * > - * NO locks, but rcu_read_lock (preempt_disabled) > + * NO locks, but rcu_read_lock > */ > void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb, > struct net_device *dev) > Did you want me to test the patch? johannes ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: br_forward.c - rcu dereference warning 2010-07-27 20:14 ` Johannes Berg @ 2010-07-27 20:42 ` Stephen Hemminger 2010-07-28 6:48 ` Johannes Berg 2010-07-28 7:33 ` Johannes Berg 0 siblings, 2 replies; 11+ messages in thread From: Stephen Hemminger @ 2010-07-27 20:42 UTC (permalink / raw) To: Johannes Berg; +Cc: paulmck, netdev On Tue, 27 Jul 2010 22:14:39 +0200 Johannes Berg <johannes@sipsolutions.net> wrote: > On Tue, 2010-07-27 at 11:26 -0700, Stephen Hemminger wrote: > > > -/* net device transmit always called with no BH (preempt_disabled) */ > > +/* net device transmit always called with no BH disabled */ > > "no BH disabled"? > > > @@ -48,6 +48,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff * > > skb_reset_mac_header(skb); > > skb_pull(skb, ETH_HLEN); > > > > + rcu_read_lock(); > > if (is_multicast_ether_addr(dest)) { > > if (unlikely(netpoll_tx_running(dev))) { > > br_flood_deliver(br, skb); > > @@ -67,6 +68,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff * > > br_flood_deliver(br, skb); > > > > out: > > + rcu_read_unlock(); > > return NETDEV_TX_OK; > > } > > > > --- a/net/bridge/br_fdb.c 2010-07-27 11:18:30.815320981 -0700 > > +++ b/net/bridge/br_fdb.c 2010-07-27 11:18:59.597710975 -0700 > > @@ -214,7 +214,7 @@ void br_fdb_delete_by_port(struct net_br > > spin_unlock_bh(&br->hash_lock); > > } > > > > -/* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */ > > +/* No locking or refcounting, assumes caller has rcu_read_lock */ > > struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br, > > const unsigned char *addr) > > { > > --- a/net/bridge/br_input.c 2010-07-27 11:19:05.266181492 -0700 > > +++ b/net/bridge/br_input.c 2010-07-27 11:19:29.148163149 -0700 > > @@ -39,7 +39,7 @@ static int br_pass_frame_up(struct sk_bu > > netif_receive_skb); > > } > > > > -/* note: already called with rcu_read_lock (preempt_disabled) */ > > +/* note: already called with rcu_read_lock */ > > int br_handle_frame_finish(struct sk_buff *skb) > > { > > const unsigned char *dest = eth_hdr(skb)->h_dest; > > @@ -110,7 +110,7 @@ drop: > > goto out; > > } > > > > -/* note: already called with rcu_read_lock (preempt_disabled) */ > > +/* note: already called with rcu_read_lock */ > > static int br_handle_local_finish(struct sk_buff *skb) > > { > > struct net_bridge_port *p = br_port_get_rcu(skb->dev); > > @@ -133,8 +133,7 @@ static inline int is_link_local(const un > > > > /* > > * Return NULL if skb is handled > > - * note: already called with rcu_read_lock (preempt_disabled) from > > - * netif_receive_skb > > + * note: already called with rcu_read_lock from netif_receive_skb > > */ > > struct sk_buff *br_handle_frame(struct sk_buff *skb) > > { > > --- a/net/bridge/br_stp_bpdu.c 2010-07-27 11:19:34.092573294 -0700 > > +++ b/net/bridge/br_stp_bpdu.c 2010-07-27 11:19:40.725123403 -0700 > > @@ -131,7 +131,7 @@ void br_send_tcn_bpdu(struct net_bridge_ > > /* > > * Called from llc. > > * > > - * NO locks, but rcu_read_lock (preempt_disabled) > > + * NO locks, but rcu_read_lock > > */ > > void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb, > > struct net_device *dev) > > > > Did you want me to test the patch? Yes please, I can make sure it works, but not that it gets rid of your error -- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: br_forward.c - rcu dereference warning 2010-07-27 20:42 ` Stephen Hemminger @ 2010-07-28 6:48 ` Johannes Berg 2010-07-28 7:33 ` Johannes Berg 1 sibling, 0 replies; 11+ messages in thread From: Johannes Berg @ 2010-07-28 6:48 UTC (permalink / raw) To: Stephen Hemminger; +Cc: paulmck, netdev On Tue, 2010-07-27 at 13:42 -0700, Stephen Hemminger wrote: > > Did you want me to test the patch? > > Yes please, I can make sure it works, but not that it gets rid > of your error Testing now, but it didn't quite apply cleanly on 2.6.35-rc. johannes ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: br_forward.c - rcu dereference warning 2010-07-27 20:42 ` Stephen Hemminger 2010-07-28 6:48 ` Johannes Berg @ 2010-07-28 7:33 ` Johannes Berg 2010-07-28 16:57 ` [PATCH] bridge: add rcu_read_lock Stephen Hemminger 2010-07-28 17:39 ` br_forward.c - rcu dereference warning David Miller 1 sibling, 2 replies; 11+ messages in thread From: Johannes Berg @ 2010-07-28 7:33 UTC (permalink / raw) To: Stephen Hemminger; +Cc: paulmck, netdev On Tue, 2010-07-27 at 13:42 -0700, Stephen Hemminger wrote: > > > > Did you want me to test the patch? > > Yes please, I can make sure it works, but not that it gets rid > of your error Yes, it fixed it, thanks. johannes ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] bridge: add rcu_read_lock 2010-07-28 7:33 ` Johannes Berg @ 2010-07-28 16:57 ` Stephen Hemminger 2010-07-28 17:52 ` David Miller 2010-07-28 17:39 ` br_forward.c - rcu dereference warning David Miller 1 sibling, 1 reply; 11+ messages in thread From: Stephen Hemminger @ 2010-07-28 16:57 UTC (permalink / raw) To: Johannes Berg, David Miller; +Cc: paulmck, netdev Long ago, when bridge was converted to RCU, rcu lock was equivalent to having preempt disabled. RCU has changed a lot since then and bridge code was still assuming the since transmit was called with bottom half disabled, it was RCU safe. In addition to fixing the code, update the comments about locking to match current state as well. Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> --- net/bridge/br_device.c | 4 +++- net/bridge/br_fdb.c | 2 +- net/bridge/br_input.c | 7 +++---- net/bridge/br_stp_bpdu.c | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) --- a/net/bridge/br_device.c 2010-07-27 08:57:43.169399349 -0700 +++ b/net/bridge/br_device.c 2010-07-27 13:57:20.278665500 -0700 @@ -22,7 +22,7 @@ #include <asm/uaccess.h> #include "br_private.h" -/* net device transmit always called with no BH (preempt_disabled) */ +/* net device transmit always called with BH disabled */ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev) { struct net_bridge *br = netdev_priv(dev); @@ -48,6 +48,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff * skb_reset_mac_header(skb); skb_pull(skb, ETH_HLEN); + rcu_read_lock(); if (is_multicast_ether_addr(dest)) { if (unlikely(netpoll_tx_running(dev))) { br_flood_deliver(br, skb); @@ -67,6 +68,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff * br_flood_deliver(br, skb); out: + rcu_read_unlock(); return NETDEV_TX_OK; } --- a/net/bridge/br_fdb.c 2010-07-27 11:18:30.815320981 -0700 +++ b/net/bridge/br_fdb.c 2010-07-27 11:18:59.597710975 -0700 @@ -214,7 +214,7 @@ void br_fdb_delete_by_port(struct net_br spin_unlock_bh(&br->hash_lock); } -/* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */ +/* No locking or refcounting, assumes caller has rcu_read_lock */ struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br, const unsigned char *addr) { --- a/net/bridge/br_input.c 2010-07-27 11:19:05.266181492 -0700 +++ b/net/bridge/br_input.c 2010-07-27 11:19:29.148163149 -0700 @@ -39,7 +39,7 @@ static int br_pass_frame_up(struct sk_bu netif_receive_skb); } -/* note: already called with rcu_read_lock (preempt_disabled) */ +/* note: already called with rcu_read_lock */ int br_handle_frame_finish(struct sk_buff *skb) { const unsigned char *dest = eth_hdr(skb)->h_dest; @@ -110,7 +110,7 @@ drop: goto out; } -/* note: already called with rcu_read_lock (preempt_disabled) */ +/* note: already called with rcu_read_lock */ static int br_handle_local_finish(struct sk_buff *skb) { struct net_bridge_port *p = br_port_get_rcu(skb->dev); @@ -133,8 +133,7 @@ static inline int is_link_local(const un /* * Return NULL if skb is handled - * note: already called with rcu_read_lock (preempt_disabled) from - * netif_receive_skb + * note: already called with rcu_read_lock from netif_receive_skb */ struct sk_buff *br_handle_frame(struct sk_buff *skb) { --- a/net/bridge/br_stp_bpdu.c 2010-07-27 11:19:34.092573294 -0700 +++ b/net/bridge/br_stp_bpdu.c 2010-07-27 11:19:40.725123403 -0700 @@ -131,7 +131,7 @@ void br_send_tcn_bpdu(struct net_bridge_ /* * Called from llc. * - * NO locks, but rcu_read_lock (preempt_disabled) + * NO locks, but rcu_read_lock */ void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb, struct net_device *dev) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] bridge: add rcu_read_lock 2010-07-28 16:57 ` [PATCH] bridge: add rcu_read_lock Stephen Hemminger @ 2010-07-28 17:52 ` David Miller 2010-07-28 18:01 ` Stephen Hemminger 0 siblings, 1 reply; 11+ messages in thread From: David Miller @ 2010-07-28 17:52 UTC (permalink / raw) To: shemminger; +Cc: johannes, paulmck, netdev From: Stephen Hemminger <shemminger@vyatta.com> Date: Wed, 28 Jul 2010 09:57:30 -0700 > Long ago, when bridge was converted to RCU, rcu lock was equivalent > to having preempt disabled. RCU has changed a lot since then and > bridge code was still assuming the since transmit was called with > bottom half disabled, it was RCU safe. > > In addition to fixing the code, update the comments about > locking to match current state as well. > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> As I stated in another email I added the original commit. This version still didn't apply to net-2.6 and also you failed to add a tested-by tag for Johannes to the commit message, which I also did alongside backporting the original patch to net-2.6 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] bridge: add rcu_read_lock 2010-07-28 17:52 ` David Miller @ 2010-07-28 18:01 ` Stephen Hemminger 0 siblings, 0 replies; 11+ messages in thread From: Stephen Hemminger @ 2010-07-28 18:01 UTC (permalink / raw) To: David Miller; +Cc: johannes, paulmck, netdev On Wed, 28 Jul 2010 10:52:09 -0700 (PDT) David Miller <davem@davemloft.net> wrote: > From: Stephen Hemminger <shemminger@vyatta.com> > Date: Wed, 28 Jul 2010 09:57:30 -0700 > > > Long ago, when bridge was converted to RCU, rcu lock was equivalent > > to having preempt disabled. RCU has changed a lot since then and > > bridge code was still assuming the since transmit was called with > > bottom half disabled, it was RCU safe. > > > > In addition to fixing the code, update the comments about > > locking to match current state as well. > > > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> > > As I stated in another email I added the original commit. > > This version still didn't apply to net-2.6 and also you failed > to add a tested-by tag for Johannes to the commit message, which > I also did alongside backporting the original patch to net-2.6 I didn't see yours, if you look at the date it was ships in the night. -- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: br_forward.c - rcu dereference warning 2010-07-28 7:33 ` Johannes Berg 2010-07-28 16:57 ` [PATCH] bridge: add rcu_read_lock Stephen Hemminger @ 2010-07-28 17:39 ` David Miller 1 sibling, 0 replies; 11+ messages in thread From: David Miller @ 2010-07-28 17:39 UTC (permalink / raw) To: johannes; +Cc: shemminger, paulmck, netdev From: Johannes Berg <johannes@sipsolutions.net> Date: Wed, 28 Jul 2010 09:33:51 +0200 > On Tue, 2010-07-27 at 13:42 -0700, Stephen Hemminger wrote: > >> > >> > Did you want me to test the patch? >> >> Yes please, I can make sure it works, but not that it gets rid >> of your error > > Yes, it fixed it, thanks. I fixed up the comment thinko Johannes noticed, added a tested-by tag, and applied this to net-2.6 Thanks! ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-07-28 18:02 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-07-27 10:45 br_forward.c - rcu dereference warning Johannes Berg 2010-07-27 12:38 ` Paul E. McKenney 2010-07-27 18:26 ` Stephen Hemminger 2010-07-27 20:14 ` Johannes Berg 2010-07-27 20:42 ` Stephen Hemminger 2010-07-28 6:48 ` Johannes Berg 2010-07-28 7:33 ` Johannes Berg 2010-07-28 16:57 ` [PATCH] bridge: add rcu_read_lock Stephen Hemminger 2010-07-28 17:52 ` David Miller 2010-07-28 18:01 ` Stephen Hemminger 2010-07-28 17:39 ` br_forward.c - rcu dereference warning David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).