* [PATCH] bonding: get netdev_rx_handler_unregister out of locks
@ 2013-04-02 15:15 Veaceslav Falico
2013-04-02 15:26 ` Eric Dumazet
0 siblings, 1 reply; 4+ messages in thread
From: Veaceslav Falico @ 2013-04-02 15:15 UTC (permalink / raw)
To: netdev; +Cc: vfalico, fubar, andy, edumazet
Now that netdev_rx_handler_unregister contains synchronize_net(), we need
to call it outside of bond->lock, cause it might sleep. Also, remove the
already unneded synchronize_net().
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 11a8cb3..78c9e2d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1975,12 +1975,11 @@ static int __bond_release_one(struct net_device *bond_dev,
return -EINVAL;
}
+ write_unlock_bh(&bond->lock);
/* unregister rx_handler early so bond_handle_frame wouldn't be called
* for this slave anymore.
*/
netdev_rx_handler_unregister(slave_dev);
- write_unlock_bh(&bond->lock);
- synchronize_net();
write_lock_bh(&bond->lock);
if (!all && !bond->params.fail_over_mac) {
--
1.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] bonding: get netdev_rx_handler_unregister out of locks
2013-04-02 15:15 [PATCH] bonding: get netdev_rx_handler_unregister out of locks Veaceslav Falico
@ 2013-04-02 15:26 ` Eric Dumazet
2013-04-02 15:45 ` Veaceslav Falico
2013-04-02 16:07 ` David Miller
0 siblings, 2 replies; 4+ messages in thread
From: Eric Dumazet @ 2013-04-02 15:26 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: netdev, fubar, andy, edumazet
On Tue, 2013-04-02 at 17:15 +0200, Veaceslav Falico wrote:
> Now that netdev_rx_handler_unregister contains synchronize_net(), we need
> to call it outside of bond->lock, cause it might sleep. Also, remove the
> already unneded synchronize_net().
>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---
> drivers/net/bonding/bond_main.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 11a8cb3..78c9e2d 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1975,12 +1975,11 @@ static int __bond_release_one(struct net_device *bond_dev,
> return -EINVAL;
> }
>
> + write_unlock_bh(&bond->lock);
> /* unregister rx_handler early so bond_handle_frame wouldn't be called
> * for this slave anymore.
> */
> netdev_rx_handler_unregister(slave_dev);
> - write_unlock_bh(&bond->lock);
> - synchronize_net();
> write_lock_bh(&bond->lock);
>
> if (!all && !bond->params.fail_over_mac) {
Good catch, thanks !
Acked-by: Eric Dumazet <edumazet@google.com>
I'll send a net-next patch to remove the synchronize_net() from bridge
code as well.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bonding: get netdev_rx_handler_unregister out of locks
2013-04-02 15:26 ` Eric Dumazet
@ 2013-04-02 15:45 ` Veaceslav Falico
2013-04-02 16:07 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: Veaceslav Falico @ 2013-04-02 15:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, fubar, andy, edumazet
On Tue, Apr 02, 2013 at 08:26:02AM -0700, Eric Dumazet wrote:
>On Tue, 2013-04-02 at 17:15 +0200, Veaceslav Falico wrote:
>> Now that netdev_rx_handler_unregister contains synchronize_net(), we need
>> to call it outside of bond->lock, cause it might sleep. Also, remove the
>> already unneded synchronize_net().
>>
>> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>> ---
>> drivers/net/bonding/bond_main.c | 3 +--
>> 1 files changed, 1 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 11a8cb3..78c9e2d 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -1975,12 +1975,11 @@ static int __bond_release_one(struct net_device *bond_dev,
>> return -EINVAL;
>> }
>>
>> + write_unlock_bh(&bond->lock);
>> /* unregister rx_handler early so bond_handle_frame wouldn't be called
>> * for this slave anymore.
>> */
>> netdev_rx_handler_unregister(slave_dev);
>> - write_unlock_bh(&bond->lock);
>> - synchronize_net();
>> write_lock_bh(&bond->lock);
>>
>> if (!all && !bond->params.fail_over_mac) {
>
>Good catch, thanks !
>
>Acked-by: Eric Dumazet <edumazet@google.com>
>
>I'll send a net-next patch to remove the synchronize_net() from bridge
>code as well.
Nice, when I've checked it not to be under other locks I've somehow missed
the part where the bridge calls synchronize_net() right after it.
Thank you!
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bonding: get netdev_rx_handler_unregister out of locks
2013-04-02 15:26 ` Eric Dumazet
2013-04-02 15:45 ` Veaceslav Falico
@ 2013-04-02 16:07 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2013-04-02 16:07 UTC (permalink / raw)
To: eric.dumazet; +Cc: vfalico, netdev, fubar, andy, edumazet
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 02 Apr 2013 08:26:02 -0700
> On Tue, 2013-04-02 at 17:15 +0200, Veaceslav Falico wrote:
>> Now that netdev_rx_handler_unregister contains synchronize_net(), we need
>> to call it outside of bond->lock, cause it might sleep. Also, remove the
>> already unneded synchronize_net().
>>
>> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
...
> Acked-by: Eric Dumazet <edumazet@google.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-04-02 16:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-02 15:15 [PATCH] bonding: get netdev_rx_handler_unregister out of locks Veaceslav Falico
2013-04-02 15:26 ` Eric Dumazet
2013-04-02 15:45 ` Veaceslav Falico
2013-04-02 16:07 ` 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).