* [PATCH 0/3] bonding: 3 fixes for 2.6.24
@ 2008-01-08 1:56 Jay Vosburgh
2008-01-08 1:56 ` [PATCH 1/3] bonding: fix locking in sysfs primary/active selection Jay Vosburgh
2008-01-08 18:50 ` [PATCH 0/3] bonding: 3 fixes for 2.6.24 Krzysztof Oledzki
0 siblings, 2 replies; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-08 1:56 UTC (permalink / raw)
To: netdev; +Cc: Jeff Garzik, David Miller, Andy Gospodarek, Krzysztof Oledzki
Following are three fixes to fix locking problems and
silence locking-related warnings in the current 2.6.24-rc.
patch 1: fix locking in sysfs primary/active selection
Call core network functions with expected locks to
eliminate potential deadlock and silence warnings.
patch 2: fix ASSERT_RTNL that produces spurious warnings
Relocate ASSERT_RTNL to remove a false warning; after patch,
ASSERT is located in code that holds only RTNL (additional locks were
causing the ASSERT to trip)
patch 3: fix locking during alb failover and slave removal
Fix all call paths into alb_fasten_mac_swap to hold only RTNL.
Eliminates deadlock and silences warnings.
Patches are against the current netdev-2.6#upstream branch.
Please apply for 2.6.24.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/3] bonding: fix locking in sysfs primary/active selection
2008-01-08 1:56 [PATCH 0/3] bonding: 3 fixes for 2.6.24 Jay Vosburgh
@ 2008-01-08 1:56 ` Jay Vosburgh
2008-01-08 1:56 ` [PATCH 2/3] bonding: fix ASSERT_RTNL that produces spurious warnings Jay Vosburgh
2008-01-08 18:50 ` [PATCH 0/3] bonding: 3 fixes for 2.6.24 Krzysztof Oledzki
1 sibling, 1 reply; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-08 1:56 UTC (permalink / raw)
To: netdev
Cc: Jeff Garzik, David Miller, Andy Gospodarek, Krzysztof Oledzki,
Jay Vosburgh
Fix the functions that store the primary and active slave
options via sysfs to hold the correct locks in the correct order.
The bond_change_active_slave and bond_select_active_slave
functions both require rtnl, bond->lock for read and curr_slave_lock for
write_bh, and no other locks. This is so that the lower level
mode-specific functions (notably for balance-alb mode) can release locks
down to just rtnl in order to call, e.g., dev_set_mac_address with the
locks it expects (rtnl only).
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
---
drivers/net/bonding/bond_sysfs.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 11b76b3..28a2d80 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1075,7 +1075,10 @@ static ssize_t bonding_store_primary(struct device *d,
struct slave *slave;
struct bonding *bond = to_bond(d);
- write_lock_bh(&bond->lock);
+ rtnl_lock();
+ read_lock(&bond->lock);
+ write_lock_bh(&bond->curr_slave_lock);
+
if (!USES_PRIMARY(bond->params.mode)) {
printk(KERN_INFO DRV_NAME
": %s: Unable to set primary slave; %s is in mode %d\n",
@@ -1109,8 +1112,8 @@ static ssize_t bonding_store_primary(struct device *d,
}
}
out:
- write_unlock_bh(&bond->lock);
-
+ write_unlock_bh(&bond->curr_slave_lock);
+ read_unlock(&bond->lock);
rtnl_unlock();
return count;
@@ -1190,7 +1193,8 @@ static ssize_t bonding_store_active_slave(struct device *d,
struct bonding *bond = to_bond(d);
rtnl_lock();
- write_lock_bh(&bond->lock);
+ read_lock(&bond->lock);
+ write_lock_bh(&bond->curr_slave_lock);
if (!USES_PRIMARY(bond->params.mode)) {
printk(KERN_INFO DRV_NAME
@@ -1247,7 +1251,8 @@ static ssize_t bonding_store_active_slave(struct device *d,
}
}
out:
- write_unlock_bh(&bond->lock);
+ write_unlock_bh(&bond->curr_slave_lock);
+ read_unlock(&bond->lock);
rtnl_unlock();
return count;
--
1.5.3.4.206.g58ba4-dirty
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 2/3] bonding: fix ASSERT_RTNL that produces spurious warnings
2008-01-08 1:56 ` [PATCH 1/3] bonding: fix locking in sysfs primary/active selection Jay Vosburgh
@ 2008-01-08 1:56 ` Jay Vosburgh
2008-01-08 1:57 ` [PATCH 3/3] bonding: fix locking during alb failover and slave removal Jay Vosburgh
0 siblings, 1 reply; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-08 1:56 UTC (permalink / raw)
To: netdev
Cc: Jeff Garzik, David Miller, Andy Gospodarek, Krzysztof Oledzki,
Jay Vosburgh
Move an ASSERT_RTNL down to where we should hold only RTNL;
the existing check produces spurious warnings because we hold additional
locks at _bh, tripping a debug warning in spin_lock_mutex().
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
drivers/net/bonding/bond_alb.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 25b8dbf..9b55a12 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1601,9 +1601,6 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
struct slave *swap_slave;
int i;
- if (new_slave)
- ASSERT_RTNL();
-
if (bond->curr_active_slave == new_slave) {
return;
}
@@ -1649,6 +1646,8 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
write_unlock_bh(&bond->curr_slave_lock);
read_unlock(&bond->lock);
+ ASSERT_RTNL();
+
/* curr_active_slave must be set before calling alb_swap_mac_addr */
if (swap_slave) {
/* swap mac address */
--
1.5.3.4.206.g58ba4-dirty
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 3/3] bonding: fix locking during alb failover and slave removal
2008-01-08 1:56 ` [PATCH 2/3] bonding: fix ASSERT_RTNL that produces spurious warnings Jay Vosburgh
@ 2008-01-08 1:57 ` Jay Vosburgh
0 siblings, 0 replies; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-08 1:57 UTC (permalink / raw)
To: netdev
Cc: Jeff Garzik, David Miller, Andy Gospodarek, Krzysztof Oledzki,
Jay Vosburgh
alb_fasten_mac_swap (actually rlb_teach_disabled_mac_on_primary)
requries RTNL and no other locks. This could cause dev_set_promiscuity
and/or dev_set_mac_address to be called with improper locking.
Changed callers to hold only RTNL during calls to alb_fasten_mac_swap
or functions calling it. Updated header comments in affected functions to
reflect proper reality of locking requirements.
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
drivers/net/bonding/bond_alb.c | 18 ++++++++++++------
drivers/net/bonding/bond_main.c | 14 ++++++++------
2 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 9b55a12..b57bc94 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -979,7 +979,7 @@ static void alb_swap_mac_addr(struct bonding *bond, struct slave *slave1, struct
/*
* Send learning packets after MAC address swap.
*
- * Called with RTNL and bond->lock held for read.
+ * Called with RTNL and no other locks
*/
static void alb_fasten_mac_swap(struct bonding *bond, struct slave *slave1,
struct slave *slave2)
@@ -987,6 +987,8 @@ static void alb_fasten_mac_swap(struct bonding *bond, struct slave *slave1,
int slaves_state_differ = (SLAVE_IS_OK(slave1) != SLAVE_IS_OK(slave2));
struct slave *disabled_slave = NULL;
+ ASSERT_RTNL();
+
/* fasten the change in the switch */
if (SLAVE_IS_OK(slave1)) {
alb_send_learning_packets(slave1, slave1->dev->dev_addr);
@@ -1031,7 +1033,7 @@ static void alb_fasten_mac_swap(struct bonding *bond, struct slave *slave1,
* a slave that has @slave's permanet address as its current address.
* We'll make sure that that slave no longer uses @slave's permanent address.
*
- * Caller must hold bond lock
+ * Caller must hold RTNL and no other locks
*/
static void alb_change_hw_addr_on_detach(struct bonding *bond, struct slave *slave)
{
@@ -1542,7 +1544,12 @@ int bond_alb_init_slave(struct bonding *bond, struct slave *slave)
return 0;
}
-/* Caller must hold bond lock for write */
+/*
+ * Remove slave from tlb and rlb hash tables, and fix up MAC addresses
+ * if necessary.
+ *
+ * Caller must hold RTNL and no other locks
+ */
void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave)
{
if (bond->slave_cnt > 1) {
@@ -1658,12 +1665,11 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
bond->alb_info.rlb_enabled);
}
- read_lock(&bond->lock);
-
if (swap_slave) {
alb_fasten_mac_swap(bond, swap_slave, new_slave);
+ read_lock(&bond->lock);
} else {
- /* fasten bond mac on new current slave */
+ read_lock(&bond->lock);
alb_send_learning_packets(new_slave, bond->dev->dev_addr);
}
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b0b2603..77d004d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1746,7 +1746,9 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
* has been cleared (if our_slave == old_current),
* but before a new active slave is selected.
*/
+ write_unlock_bh(&bond->lock);
bond_alb_deinit_slave(bond, slave);
+ write_lock_bh(&bond->lock);
}
if (oldcurrent == slave) {
@@ -1905,6 +1907,12 @@ static int bond_release_all(struct net_device *bond_dev)
slave_dev = slave->dev;
bond_detach_slave(bond, slave);
+ /* now that the slave is detached, unlock and perform
+ * all the undo steps that should not be called from
+ * within a lock.
+ */
+ write_unlock_bh(&bond->lock);
+
if ((bond->params.mode == BOND_MODE_TLB) ||
(bond->params.mode == BOND_MODE_ALB)) {
/* must be called only after the slave
@@ -1915,12 +1923,6 @@ static int bond_release_all(struct net_device *bond_dev)
bond_compute_features(bond);
- /* now that the slave is detached, unlock and perform
- * all the undo steps that should not be called from
- * within a lock.
- */
- write_unlock_bh(&bond->lock);
-
bond_destroy_slave_symlinks(bond_dev, slave_dev);
bond_del_vlans_from_slave(bond, slave_dev);
--
1.5.3.4.206.g58ba4-dirty
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-08 1:56 [PATCH 0/3] bonding: 3 fixes for 2.6.24 Jay Vosburgh
2008-01-08 1:56 ` [PATCH 1/3] bonding: fix locking in sysfs primary/active selection Jay Vosburgh
@ 2008-01-08 18:50 ` Krzysztof Oledzki
2008-01-08 19:17 ` Andy Gospodarek
2008-01-08 19:30 ` Jay Vosburgh
1 sibling, 2 replies; 30+ messages in thread
From: Krzysztof Oledzki @ 2008-01-08 18:50 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, Jeff Garzik, David Miller, Andy Gospodarek
[-- Attachment #1: Type: TEXT/PLAIN, Size: 56733 bytes --]
On Mon, 7 Jan 2008, Jay Vosburgh wrote:
> Following are three fixes to fix locking problems and
> silence locking-related warnings in the current 2.6.24-rc.
>
> patch 1: fix locking in sysfs primary/active selection
>
> Call core network functions with expected locks to
> eliminate potential deadlock and silence warnings.
>
> patch 2: fix ASSERT_RTNL that produces spurious warnings
>
> Relocate ASSERT_RTNL to remove a false warning; after patch,
> ASSERT is located in code that holds only RTNL (additional locks were
> causing the ASSERT to trip)
>
> patch 3: fix locking during alb failover and slave removal
>
> Fix all call paths into alb_fasten_mac_swap to hold only RTNL.
> Eliminates deadlock and silences warnings.
>
> Patches are against the current netdev-2.6#upstream branch.
>
> Please apply for 2.6.24.
2.6.24-rc7 + patches #1, #2, #3:
bonding: bond0: setting mode to active-backup (1).
bonding: bond0: Setting MII monitoring interval to 100.
ADDRCONF(NETDEV_UP): bond0: link is not ready
bonding: bond0: Adding slave eth0.
e1000: eth0: e1000_watchdog: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
bonding: bond0: making interface eth0 the new active one.
bonding: bond0: first active interface up!
bonding: bond0: enslaving eth0 as an active interface with an up link.
bonding: bond0: Adding slave eth1.
ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
=========================================================
[ INFO: possible irq lock inversion dependency detected ]
2.6.24-rc7 #1
---------------------------------------------------------
events/0/9 just changed the state of lock:
(&mc->mca_lock){-+..}, at: [<c041258e>] mld_ifc_timer_expire+0x130/0x1fb
but this lock took another, soft-read-irq-unsafe lock in the past:
(&bond->lock){-.--}
and interrupts could create inverse lock ordering between them.
other info that might help us debug this:
4 locks held by events/0/9:
#0: (events){--..}, at: [<c0133d33>] run_workqueue+0x87/0x1b6
#1: ((linkwatch_work).work){--..}, at: [<c0133d33>] run_workqueue+0x87/0x1b6
#2: (rtnl_mutex){--..}, at: [<c03ac678>] linkwatch_event+0x5/0x22
#3: (&ndev->lock){-.-+}, at: [<c0412475>] mld_ifc_timer_expire+0x17/0x1fb
the first lock's dependencies:
-> (&mc->mca_lock){-+..} ops: 10 {
initial-use at:
[<c0104ee2>] dump_trace+0x83/0x8d
[<c0142890>] __lock_acquire+0x4ba/0xc07
[<c0109ef2>] save_stack_trace+0x20/0x3a
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0412d66>] ipv6_dev_mc_inc+0x24d/0x31c
[<c0143056>] lock_acquire+0x79/0x93
[<c04129ea>] igmp6_group_added+0x18/0x11d
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c04129ea>] igmp6_group_added+0x18/0x11d
[<c04129ea>] igmp6_group_added+0x18/0x11d
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0412d66>] ipv6_dev_mc_inc+0x24d/0x31c
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c05c5ae9>] addrconf_init+0x13/0x193
[<c019a04b>] proc_net_fops_create+0x10/0x21
[<c041a44c>] ip6_flowlabel_init+0x1e/0x20
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
in-softirq-W at:
[<c014197a>] mark_lock+0x64/0x451
[<c0142816>] __lock_acquire+0x440/0xc07
[<c0103f7b>] restore_nocheck+0x12/0x15
[<c0143056>] lock_acquire+0x79/0x93
[<c041258e>] mld_ifc_timer_expire+0x130/0x1fb
[<c041245e>] mld_ifc_timer_expire+0x0/0x1fb
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c041258e>] mld_ifc_timer_expire+0x130/0x1fb
[<c041258e>] mld_ifc_timer_expire+0x130/0x1fb
[<c041245e>] mld_ifc_timer_expire+0x0/0x1fb
[<c0141f7d>] trace_hardirqs_on+0x10c/0x14c
[<c041245e>] mld_ifc_timer_expire+0x0/0x1fb
[<c012e02e>] run_timer_softirq+0xfa/0x15d
[<c012a982>] __do_softirq+0x56/0xdb
[<c0141f7d>] trace_hardirqs_on+0x10c/0x14c
[<c012a994>] __do_softirq+0x68/0xdb
[<c012aa3d>] do_softirq+0x36/0x51
[<c012af26>] local_bh_enable_ip+0xad/0xed
[<c03bfa37>] rt_run_flush+0x64/0x8b
[<c03e9bbe>] fib_netdev_event+0x61/0x65
[<c013ac34>] notifier_call_chain+0x2a/0x52
[<c013ac7e>] raw_notifier_call_chain+0x17/0x1a
[<c03a340d>] netdev_state_change+0x18/0x29
[<c03ac645>] __linkwatch_run_queue+0x150/0x17e
[<c03ac690>] linkwatch_event+0x1d/0x22
[<c0133d87>] run_workqueue+0xdb/0x1b6
[<c0133d33>] run_workqueue+0x87/0x1b6
[<c03ac673>] linkwatch_event+0x0/0x22
[<c01347a7>] worker_thread+0x0/0x85
[<c0134820>] worker_thread+0x79/0x85
[<c0137255>] autoremove_wake_function+0x0/0x35
[<c013719e>] kthread+0x38/0x5e
[<c0137166>] kthread+0x0/0x5e
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
hardirq-on-W at:
[<c01417e2>] find_usage_backwards+0xbb/0xe2
[<c0104ee2>] dump_trace+0x83/0x8d
[<c014285e>] __lock_acquire+0x488/0xc07
[<c0109ef2>] save_stack_trace+0x20/0x3a
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0412d66>] ipv6_dev_mc_inc+0x24d/0x31c
[<c0143056>] lock_acquire+0x79/0x93
[<c04129ea>] igmp6_group_added+0x18/0x11d
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c04129ea>] igmp6_group_added+0x18/0x11d
[<c04129ea>] igmp6_group_added+0x18/0x11d
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0412d66>] ipv6_dev_mc_inc+0x24d/0x31c
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c05c5ae9>] addrconf_init+0x13/0x193
[<c019a04b>] proc_net_fops_create+0x10/0x21
[<c041a44c>] ip6_flowlabel_init+0x1e/0x20
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
}
... key at: [<c087e2d8>] __key.30803+0x0/0x8
-> (_xmit_ETHER){-...} ops: 8 {
initial-use at:
[<c0142890>] __lock_acquire+0x4ba/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0412a28>] igmp6_group_added+0x56/0x11d
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0410100>] icmpv6_rcv+0x6a4/0x828
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c040274b>] addrconf_notify+0x60/0x7b7
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043997e>] mutex_lock_nested+0x286/0x2ac
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a48b0>] register_netdevice_notifier+0x49/0x126
[<c05c5b83>] addrconf_init+0xad/0x193
[<c05c5af1>] addrconf_init+0x1b/0x193
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
hardirq-on-W at:
[<c014197a>] mark_lock+0x64/0x451
[<c014285e>] __lock_acquire+0x488/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0412a28>] igmp6_group_added+0x56/0x11d
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0410100>] icmpv6_rcv+0x6a4/0x828
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c040274b>] addrconf_notify+0x60/0x7b7
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043997e>] mutex_lock_nested+0x286/0x2ac
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a48b0>] register_netdevice_notifier+0x49/0x126
[<c05c5b83>] addrconf_init+0xad/0x193
[<c05c5af1>] addrconf_init+0x1b/0x193
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
}
... key at: [<c087adc8>] netdev_xmit_lock_key+0x8/0x1c0
... acquired at:
[<c0142df3>] __lock_acquire+0xa1d/0xc07
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0412a28>] igmp6_group_added+0x56/0x11d
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0410100>] icmpv6_rcv+0x6a4/0x828
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c040274b>] addrconf_notify+0x60/0x7b7
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043997e>] mutex_lock_nested+0x286/0x2ac
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a48b0>] register_netdevice_notifier+0x49/0x126
[<c05c5b83>] addrconf_init+0xad/0x193
[<c05c5af1>] addrconf_init+0x1b/0x193
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
-> (&bonding_netdev_xmit_lock_key){-...} ops: 6 {
initial-use at:
[<c0142890>] __lock_acquire+0x4ba/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0412a28>] igmp6_group_added+0x56/0x11d
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0410100>] icmpv6_rcv+0x6a4/0x828
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c040274b>] addrconf_notify+0x60/0x7b7
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043997e>] mutex_lock_nested+0x286/0x2ac
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a48b0>] register_netdevice_notifier+0x49/0x126
[<c05c5b83>] addrconf_init+0xad/0x193
[<c05c5af1>] addrconf_init+0x1b/0x193
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
hardirq-on-W at:
[<c014197a>] mark_lock+0x64/0x451
[<c014285e>] __lock_acquire+0x488/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0412a28>] igmp6_group_added+0x56/0x11d
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0410100>] icmpv6_rcv+0x6a4/0x828
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c040274b>] addrconf_notify+0x60/0x7b7
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043997e>] mutex_lock_nested+0x286/0x2ac
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a48b0>] register_netdevice_notifier+0x49/0x126
[<c05c5b83>] addrconf_init+0xad/0x193
[<c05c5af1>] addrconf_init+0x1b/0x193
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
}
... key at: [<c0877804>] bonding_netdev_xmit_lock_key+0x0/0x8
-> (&bond->lock){-.--} ops: 99 {
initial-use at:
[<c013fe29>] put_lock_stats+0xa/0x1e
[<c0142890>] __lock_acquire+0x4ba/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043af0d>] _spin_unlock_irqrestore+0x40/0x58
[<c0143056>] lock_acquire+0x79/0x93
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c043aa36>] _read_lock_bh+0x3b/0x64
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c03aad0f>] rtnl_fill_ifinfo+0x2bf/0x563
[<c03ab28d>] rtmsg_ifinfo+0x5d/0xdf
[<c03ab34e>] rtnetlink_event+0x3f/0x42
[<c013ac34>] notifier_call_chain+0x2a/0x52
[<c013ac7e>] raw_notifier_call_chain+0x17/0x1a
[<c03a3b06>] register_netdevice+0x2a7/0x2e7
[<c02ed842>] bond_create+0x1f2/0x26a
[<c05bed76>] bonding_init+0x761/0x7ea
[<c05be5de>] e1000_init_module+0x45/0x7c
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
hardirq-on-W at:
[<c014285e>] __lock_acquire+0x488/0xc07
[<c01208c9>] try_to_wake_up+0x2ce/0x2d8
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c02eda55>] bond_set_multicast_list+0x1d/0x241
[<c043a96d>] _write_lock_bh+0x3b/0x64
[<c02eda55>] bond_set_multicast_list+0x1d/0x241
[<c02eda55>] bond_set_multicast_list+0x1d/0x241
[<c013fe29>] put_lock_stats+0xa/0x1e
[<c03a1e03>] __dev_set_rx_mode+0x7b/0x7d
[<c03a1f9d>] dev_set_rx_mode+0x23/0x36
[<c03a4678>] dev_open+0x5e/0x77
[<c03a3347>] dev_change_flags+0x9d/0x14b
[<c03a214b>] __dev_get_by_name+0x68/0x73
[<c03e4176>] devinet_ioctl+0x22b/0x536
[<c03a446d>] dev_ioctl+0x46f/0x5b7
[<c039a598>] sock_ioctl+0x167/0x18b
[<c039a431>] sock_ioctl+0x0/0x18b
[<c0172677>] do_ioctl+0x1f/0x62
[<c01728e7>] vfs_ioctl+0x22d/0x23f
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c017292c>] sys_ioctl+0x33/0x4b
[<c0103e92>] sysenter_past_esp+0x5f/0xa5
[<ffffffff>] 0xffffffff
softirq-on-R at:
[<c014197a>] mark_lock+0x64/0x451
[<c013583a>] __kernel_text_address+0x5/0xe
[<c0104ee2>] dump_trace+0x83/0x8d
[<c014287d>] __lock_acquire+0x4a7/0xc07
[<c013fc6a>] save_trace+0x37/0x89
[<c0133d33>] run_workqueue+0x87/0x1b6
[<c0143056>] lock_acquire+0x79/0x93
[<c02eee43>] bond_mii_monitor+0x19/0x85
[<c043aa95>] _read_lock+0x36/0x5f
[<c02eee43>] bond_mii_monitor+0x19/0x85
[<c02eee43>] bond_mii_monitor+0x19/0x85
[<c0133d87>] run_workqueue+0xdb/0x1b6
[<c0133d33>] run_workqueue+0x87/0x1b6
[<c02eee2a>] bond_mii_monitor+0x0/0x85
[<c01347a7>] worker_thread+0x0/0x85
[<c0134820>] worker_thread+0x79/0x85
[<c0137255>] autoremove_wake_function+0x0/0x35
[<c013719e>] kthread+0x38/0x5e
[<c0137166>] kthread+0x0/0x5e
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
hardirq-on-R at:
[<c013fdfe>] get_lock_stats+0xd/0x2e
[<c013fe29>] put_lock_stats+0xa/0x1e
[<c0142838>] __lock_acquire+0x462/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043af0d>] _spin_unlock_irqrestore+0x40/0x58
[<c0143056>] lock_acquire+0x79/0x93
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c043aa36>] _read_lock_bh+0x3b/0x64
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c03aad0f>] rtnl_fill_ifinfo+0x2bf/0x563
[<c03ab28d>] rtmsg_ifinfo+0x5d/0xdf
[<c03ab34e>] rtnetlink_event+0x3f/0x42
[<c013ac34>] notifier_call_chain+0x2a/0x52
[<c013ac7e>] raw_notifier_call_chain+0x17/0x1a
[<c03a3b06>] register_netdevice+0x2a7/0x2e7
[<c02ed842>] bond_create+0x1f2/0x26a
[<c05bed76>] bonding_init+0x761/0x7ea
[<c05be5de>] e1000_init_module+0x45/0x7c
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
}
... key at: [<c08777d0>] __key.32976+0x0/0x8
-> (_xmit_ETHER){-...} ops: 8 {
initial-use at:
[<c0142890>] __lock_acquire+0x4ba/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0412a28>] igmp6_group_added+0x56/0x11d
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0410100>] icmpv6_rcv+0x6a4/0x828
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c040274b>] addrconf_notify+0x60/0x7b7
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043997e>] mutex_lock_nested+0x286/0x2ac
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a48b0>] register_netdevice_notifier+0x49/0x126
[<c05c5b83>] addrconf_init+0xad/0x193
[<c05c5af1>] addrconf_init+0x1b/0x193
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
hardirq-on-W at:
[<c014197a>] mark_lock+0x64/0x451
[<c014285e>] __lock_acquire+0x488/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0412a28>] igmp6_group_added+0x56/0x11d
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0410100>] icmpv6_rcv+0x6a4/0x828
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c040274b>] addrconf_notify+0x60/0x7b7
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043997e>] mutex_lock_nested+0x286/0x2ac
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a48b0>] register_netdevice_notifier+0x49/0x126
[<c05c5b83>] addrconf_init+0xad/0x193
[<c05c5af1>] addrconf_init+0x1b/0x193
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
}
... key at: [<c087adc8>] netdev_xmit_lock_key+0x8/0x1c0
... acquired at:
[<c0142df3>] __lock_acquire+0xa1d/0xc07
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043af0d>] _spin_unlock_irqrestore+0x40/0x58
[<c0109ef2>] save_stack_trace+0x20/0x3a
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c02ee472>] bond_change_active_slave+0x1a9/0x3bf
[<c02ec7a3>] bond_update_speed_duplex+0x26/0x65
[<c02ee991>] bond_select_active_slave+0x97/0xd3
[<c02ed20b>] bond_compute_features+0x45/0x84
[<c02ef9b6>] bond_enslave+0x6a7/0x884
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c02f61af>] bonding_store_slaves+0x1ae/0x2fb
[<c02f6001>] bonding_store_slaves+0x0/0x2fb
[<c02ce8b7>] dev_attr_store+0x27/0x2c
[<c019bda1>] sysfs_write_file+0xad/0xe0
[<c019bcf4>] sysfs_write_file+0x0/0xe0
[<c0168e5c>] vfs_write+0x8a/0x10c
[<c0118562>] do_page_fault+0x0/0x54a
[<c01693e1>] sys_write+0x41/0x67
[<c0103e92>] sysenter_past_esp+0x5f/0xa5
[<ffffffff>] 0xffffffff
-> (lweventlist_lock){.+..} ops: 10 {
initial-use at:
[<c014197a>] mark_lock+0x64/0x451
[<c0142890>] __lock_acquire+0x4ba/0xc07
[<c02e363c>] e1000_read_phy_reg+0x1c7/0x1d3
[<c02e346b>] e1000_write_phy_reg+0xb9/0xc3
[<c024a82e>] delay_tsc+0x25/0x3b
[<c0143056>] lock_acquire+0x79/0x93
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c043abc7>] _spin_lock_irqsave+0x3f/0x6c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac4e3>] linkwatch_fire_event+0x25/0x37
[<c02e1c23>] e1000_probe+0xad1/0xbe8
[<c0257f1b>] pci_device_probe+0x36/0x57
[<c02d0e3f>] driver_probe_device+0xe1/0x15f
[<c043ae41>] _spin_unlock+0x25/0x3b
[<c0437ec6>] klist_next+0x58/0x6d
[<c02d0f4f>] __driver_attach+0x0/0x7f
[<c02d0f98>] __driver_attach+0x49/0x7f
[<c02d03e3>] bus_for_each_dev+0x36/0x58
[<c02d0c97>] driver_attach+0x16/0x18
[<c02d0f4f>] __driver_attach+0x0/0x7f
[<c02d06da>] bus_add_driver+0x6d/0x18d
[<c0258065>] __pci_register_driver+0x53/0x7f
[<c05be5de>] e1000_init_module+0x45/0x7c
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
in-softirq-W at:
[<c011d1c0>] __wake_up_common+0x32/0x5c
[<c0142816>] __lock_acquire+0x440/0xc07
[<c043af0d>] _spin_unlock_irqrestore+0x40/0x58
[<c0143056>] lock_acquire+0x79/0x93
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c02dfee1>] e1000_watchdog+0x0/0x5c9
[<c043abc7>] _spin_lock_irqsave+0x3f/0x6c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac4e3>] linkwatch_fire_event+0x25/0x37
[<c03af46a>] netif_carrier_on+0x16/0x27
[<c02e0136>] e1000_watchdog+0x255/0x5c9
[<c02dfee1>] e1000_watchdog+0x0/0x5c9
[<c012e02e>] run_timer_softirq+0xfa/0x15d
[<c012a982>] __do_softirq+0x56/0xdb
[<c0141f7d>] trace_hardirqs_on+0x10c/0x14c
[<c012a994>] __do_softirq+0x68/0xdb
[<c012aa3d>] do_softirq+0x36/0x51
[<c012abe3>] irq_exit+0x43/0x4e
[<c0114122>] smp_apic_timer_interrupt+0x74/0x80
[<c0104a01>] apic_timer_interrupt+0x29/0x38
[<c0104a0b>] apic_timer_interrupt+0x33/0x38
[<c01600d8>] sys_swapon+0x254/0x9aa
[<c01021a6>] mwait_idle_with_hints+0x3b/0x3f
[<c0102447>] mwait_idle+0x0/0xf
[<c0102581>] cpu_idle+0x99/0xc6
[<c05a98c7>] start_kernel+0x2c7/0x2cf
[<c05a90e0>] unknown_bootoption+0x0/0x195
[<ffffffff>] 0xffffffff
}
... key at: [<c058a194>] lweventlist_lock+0x14/0x40
... acquired at:
[<c0142df3>] __lock_acquire+0xa1d/0xc07
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c043abc7>] _spin_lock_irqsave+0x3f/0x6c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac4e3>] linkwatch_fire_event+0x25/0x37
[<c03af46a>] netif_carrier_on+0x16/0x27
[<c02ede0c>] bond_set_carrier+0x31/0x55
[<c02ee998>] bond_select_active_slave+0x9e/0xd3
[<c02ed20b>] bond_compute_features+0x45/0x84
[<c02ef9b6>] bond_enslave+0x6a7/0x884
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c02f61af>] bonding_store_slaves+0x1ae/0x2fb
[<c02f6001>] bonding_store_slaves+0x0/0x2fb
[<c02ce8b7>] dev_attr_store+0x27/0x2c
[<c019bda1>] sysfs_write_file+0xad/0xe0
[<c019bcf4>] sysfs_write_file+0x0/0xe0
[<c0168e5c>] vfs_write+0x8a/0x10c
[<c0118562>] do_page_fault+0x0/0x54a
[<c01693e1>] sys_write+0x41/0x67
[<c0103e92>] sysenter_past_esp+0x5f/0xa5
[<ffffffff>] 0xffffffff
... acquired at:
[<c0142df3>] __lock_acquire+0xa1d/0xc07
[<c02eda55>] bond_set_multicast_list+0x1d/0x241
[<c0143056>] lock_acquire+0x79/0x93
[<c02eda55>] bond_set_multicast_list+0x1d/0x241
[<c043a96d>] _write_lock_bh+0x3b/0x64
[<c02eda55>] bond_set_multicast_list+0x1d/0x241
[<c02eda55>] bond_set_multicast_list+0x1d/0x241
[<c013fe29>] put_lock_stats+0xa/0x1e
[<c03a1e03>] __dev_set_rx_mode+0x7b/0x7d
[<c03a1f9d>] dev_set_rx_mode+0x23/0x36
[<c03a4678>] dev_open+0x5e/0x77
[<c03a3347>] dev_change_flags+0x9d/0x14b
[<c03a214b>] __dev_get_by_name+0x68/0x73
[<c03e4176>] devinet_ioctl+0x22b/0x536
[<c03a446d>] dev_ioctl+0x46f/0x5b7
[<c039a598>] sock_ioctl+0x167/0x18b
[<c039a431>] sock_ioctl+0x0/0x18b
[<c0172677>] do_ioctl+0x1f/0x62
[<c01728e7>] vfs_ioctl+0x22d/0x23f
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c017292c>] sys_ioctl+0x33/0x4b
[<c0103e92>] sysenter_past_esp+0x5f/0xa5
[<ffffffff>] 0xffffffff
... acquired at:
[<c0142df3>] __lock_acquire+0xa1d/0xc07
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0412a28>] igmp6_group_added+0x56/0x11d
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0410100>] icmpv6_rcv+0x6a4/0x828
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c040274b>] addrconf_notify+0x60/0x7b7
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043997e>] mutex_lock_nested+0x286/0x2ac
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a48b0>] register_netdevice_notifier+0x49/0x126
[<c05c5b83>] addrconf_init+0xad/0x193
[<c05c5af1>] addrconf_init+0x1b/0x193
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
the second lock's dependencies:
-> (&bond->lock){-.--} ops: 99 {
initial-use at:
[<c013fe29>] put_lock_stats+0xa/0x1e
[<c0142890>] __lock_acquire+0x4ba/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043af0d>] _spin_unlock_irqrestore+0x40/0x58
[<c0143056>] lock_acquire+0x79/0x93
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c043aa36>] _read_lock_bh+0x3b/0x64
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c03aad0f>] rtnl_fill_ifinfo+0x2bf/0x563
[<c03ab28d>] rtmsg_ifinfo+0x5d/0xdf
[<c03ab34e>] rtnetlink_event+0x3f/0x42
[<c013ac34>] notifier_call_chain+0x2a/0x52
[<c013ac7e>] raw_notifier_call_chain+0x17/0x1a
[<c03a3b06>] register_netdevice+0x2a7/0x2e7
[<c02ed842>] bond_create+0x1f2/0x26a
[<c05bed76>] bonding_init+0x761/0x7ea
[<c05be5de>] e1000_init_module+0x45/0x7c
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
hardirq-on-W at:
[<c014285e>] __lock_acquire+0x488/0xc07
[<c01208c9>] try_to_wake_up+0x2ce/0x2d8
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c02eda55>] bond_set_multicast_list+0x1d/0x241
[<c043a96d>] _write_lock_bh+0x3b/0x64
[<c02eda55>] bond_set_multicast_list+0x1d/0x241
[<c02eda55>] bond_set_multicast_list+0x1d/0x241
[<c013fe29>] put_lock_stats+0xa/0x1e
[<c03a1e03>] __dev_set_rx_mode+0x7b/0x7d
[<c03a1f9d>] dev_set_rx_mode+0x23/0x36
[<c03a4678>] dev_open+0x5e/0x77
[<c03a3347>] dev_change_flags+0x9d/0x14b
[<c03a214b>] __dev_get_by_name+0x68/0x73
[<c03e4176>] devinet_ioctl+0x22b/0x536
[<c03a446d>] dev_ioctl+0x46f/0x5b7
[<c039a598>] sock_ioctl+0x167/0x18b
[<c039a431>] sock_ioctl+0x0/0x18b
[<c0172677>] do_ioctl+0x1f/0x62
[<c01728e7>] vfs_ioctl+0x22d/0x23f
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c017292c>] sys_ioctl+0x33/0x4b
[<c0103e92>] sysenter_past_esp+0x5f/0xa5
[<ffffffff>] 0xffffffff
softirq-on-R at:
[<c014197a>] mark_lock+0x64/0x451
[<c013583a>] __kernel_text_address+0x5/0xe
[<c0104ee2>] dump_trace+0x83/0x8d
[<c014287d>] __lock_acquire+0x4a7/0xc07
[<c013fc6a>] save_trace+0x37/0x89
[<c0133d33>] run_workqueue+0x87/0x1b6
[<c0143056>] lock_acquire+0x79/0x93
[<c02eee43>] bond_mii_monitor+0x19/0x85
[<c043aa95>] _read_lock+0x36/0x5f
[<c02eee43>] bond_mii_monitor+0x19/0x85
[<c02eee43>] bond_mii_monitor+0x19/0x85
[<c0133d87>] run_workqueue+0xdb/0x1b6
[<c0133d33>] run_workqueue+0x87/0x1b6
[<c02eee2a>] bond_mii_monitor+0x0/0x85
[<c01347a7>] worker_thread+0x0/0x85
[<c0134820>] worker_thread+0x79/0x85
[<c0137255>] autoremove_wake_function+0x0/0x35
[<c013719e>] kthread+0x38/0x5e
[<c0137166>] kthread+0x0/0x5e
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
hardirq-on-R at:
[<c013fdfe>] get_lock_stats+0xd/0x2e
[<c013fe29>] put_lock_stats+0xa/0x1e
[<c0142838>] __lock_acquire+0x462/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043af0d>] _spin_unlock_irqrestore+0x40/0x58
[<c0143056>] lock_acquire+0x79/0x93
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c043aa36>] _read_lock_bh+0x3b/0x64
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c02edca1>] bond_get_stats+0x28/0xd0
[<c03aad0f>] rtnl_fill_ifinfo+0x2bf/0x563
[<c03ab28d>] rtmsg_ifinfo+0x5d/0xdf
[<c03ab34e>] rtnetlink_event+0x3f/0x42
[<c013ac34>] notifier_call_chain+0x2a/0x52
[<c013ac7e>] raw_notifier_call_chain+0x17/0x1a
[<c03a3b06>] register_netdevice+0x2a7/0x2e7
[<c02ed842>] bond_create+0x1f2/0x26a
[<c05bed76>] bonding_init+0x761/0x7ea
[<c05be5de>] e1000_init_module+0x45/0x7c
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
}
... key at: [<c08777d0>] __key.32976+0x0/0x8
-> (_xmit_ETHER){-...} ops: 8 {
initial-use at:
[<c0142890>] __lock_acquire+0x4ba/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0412a28>] igmp6_group_added+0x56/0x11d
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0410100>] icmpv6_rcv+0x6a4/0x828
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c040274b>] addrconf_notify+0x60/0x7b7
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043997e>] mutex_lock_nested+0x286/0x2ac
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a48b0>] register_netdevice_notifier+0x49/0x126
[<c05c5b83>] addrconf_init+0xad/0x193
[<c05c5af1>] addrconf_init+0x1b/0x193
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
hardirq-on-W at:
[<c014197a>] mark_lock+0x64/0x451
[<c014285e>] __lock_acquire+0x488/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c0412a28>] igmp6_group_added+0x56/0x11d
[<c0412dbc>] ipv6_dev_mc_inc+0x2a3/0x31c
[<c0410100>] icmpv6_rcv+0x6a4/0x828
[<c0412df1>] ipv6_dev_mc_inc+0x2d8/0x31c
[<c0412b19>] ipv6_dev_mc_inc+0x0/0x31c
[<c0402168>] ipv6_add_dev+0x21c/0x24b
[<c040b991>] ndisc_ifinfo_sysctl_change+0x0/0x1ef
[<c040274b>] addrconf_notify+0x60/0x7b7
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0141da0>] mark_held_locks+0x39/0x53
[<c043997e>] mutex_lock_nested+0x286/0x2ac
[<c0141f93>] trace_hardirqs_on+0x122/0x14c
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a4875>] register_netdevice_notifier+0xe/0x126
[<c03a48b0>] register_netdevice_notifier+0x49/0x126
[<c05c5b83>] addrconf_init+0xad/0x193
[<c05c5af1>] addrconf_init+0x1b/0x193
[<c05c59c9>] inet6_init+0x1f0/0x2ad
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
}
... key at: [<c087adc8>] netdev_xmit_lock_key+0x8/0x1c0
... acquired at:
[<c0142df3>] __lock_acquire+0xa1d/0xc07
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043af0d>] _spin_unlock_irqrestore+0x40/0x58
[<c0109ef2>] save_stack_trace+0x20/0x3a
[<c0143056>] lock_acquire+0x79/0x93
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c03a63c9>] dev_mc_add+0x1a/0x6a
[<c02ee472>] bond_change_active_slave+0x1a9/0x3bf
[<c02ec7a3>] bond_update_speed_duplex+0x26/0x65
[<c02ee991>] bond_select_active_slave+0x97/0xd3
[<c02ed20b>] bond_compute_features+0x45/0x84
[<c02ef9b6>] bond_enslave+0x6a7/0x884
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c02f61af>] bonding_store_slaves+0x1ae/0x2fb
[<c02f6001>] bonding_store_slaves+0x0/0x2fb
[<c02ce8b7>] dev_attr_store+0x27/0x2c
[<c019bda1>] sysfs_write_file+0xad/0xe0
[<c019bcf4>] sysfs_write_file+0x0/0xe0
[<c0168e5c>] vfs_write+0x8a/0x10c
[<c0118562>] do_page_fault+0x0/0x54a
[<c01693e1>] sys_write+0x41/0x67
[<c0103e92>] sysenter_past_esp+0x5f/0xa5
[<ffffffff>] 0xffffffff
-> (lweventlist_lock){.+..} ops: 10 {
initial-use at:
[<c014197a>] mark_lock+0x64/0x451
[<c0142890>] __lock_acquire+0x4ba/0xc07
[<c02e363c>] e1000_read_phy_reg+0x1c7/0x1d3
[<c02e346b>] e1000_write_phy_reg+0xb9/0xc3
[<c024a82e>] delay_tsc+0x25/0x3b
[<c0143056>] lock_acquire+0x79/0x93
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c043abc7>] _spin_lock_irqsave+0x3f/0x6c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac4e3>] linkwatch_fire_event+0x25/0x37
[<c02e1c23>] e1000_probe+0xad1/0xbe8
[<c0257f1b>] pci_device_probe+0x36/0x57
[<c02d0e3f>] driver_probe_device+0xe1/0x15f
[<c043ae41>] _spin_unlock+0x25/0x3b
[<c0437ec6>] klist_next+0x58/0x6d
[<c02d0f4f>] __driver_attach+0x0/0x7f
[<c02d0f98>] __driver_attach+0x49/0x7f
[<c02d03e3>] bus_for_each_dev+0x36/0x58
[<c02d0c97>] driver_attach+0x16/0x18
[<c02d0f4f>] __driver_attach+0x0/0x7f
[<c02d06da>] bus_add_driver+0x6d/0x18d
[<c0258065>] __pci_register_driver+0x53/0x7f
[<c05be5de>] e1000_init_module+0x45/0x7c
[<c05a9499>] kernel_init+0x150/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c05a9349>] kernel_init+0x0/0x2b7
[<c0104baf>] kernel_thread_helper+0x7/0x10
[<ffffffff>] 0xffffffff
in-softirq-W at:
[<c011d1c0>] __wake_up_common+0x32/0x5c
[<c0142816>] __lock_acquire+0x440/0xc07
[<c043af0d>] _spin_unlock_irqrestore+0x40/0x58
[<c0143056>] lock_acquire+0x79/0x93
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c02dfee1>] e1000_watchdog+0x0/0x5c9
[<c043abc7>] _spin_lock_irqsave+0x3f/0x6c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac4e3>] linkwatch_fire_event+0x25/0x37
[<c03af46a>] netif_carrier_on+0x16/0x27
[<c02e0136>] e1000_watchdog+0x255/0x5c9
[<c02dfee1>] e1000_watchdog+0x0/0x5c9
[<c012e02e>] run_timer_softirq+0xfa/0x15d
[<c012a982>] __do_softirq+0x56/0xdb
[<c0141f7d>] trace_hardirqs_on+0x10c/0x14c
[<c012a994>] __do_softirq+0x68/0xdb
[<c012aa3d>] do_softirq+0x36/0x51
[<c012abe3>] irq_exit+0x43/0x4e
[<c0114122>] smp_apic_timer_interrupt+0x74/0x80
[<c0104a01>] apic_timer_interrupt+0x29/0x38
[<c0104a0b>] apic_timer_interrupt+0x33/0x38
[<c01600d8>] sys_swapon+0x254/0x9aa
[<c01021a6>] mwait_idle_with_hints+0x3b/0x3f
[<c0102447>] mwait_idle+0x0/0xf
[<c0102581>] cpu_idle+0x99/0xc6
[<c05a98c7>] start_kernel+0x2c7/0x2cf
[<c05a90e0>] unknown_bootoption+0x0/0x195
[<ffffffff>] 0xffffffff
}
... key at: [<c058a194>] lweventlist_lock+0x14/0x40
... acquired at:
[<c0142df3>] __lock_acquire+0xa1d/0xc07
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c0142f95>] __lock_acquire+0xbbf/0xc07
[<c0143056>] lock_acquire+0x79/0x93
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c043abc7>] _spin_lock_irqsave+0x3f/0x6c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac41f>] linkwatch_add_event+0xd/0x2c
[<c03ac4e3>] linkwatch_fire_event+0x25/0x37
[<c03af46a>] netif_carrier_on+0x16/0x27
[<c02ede0c>] bond_set_carrier+0x31/0x55
[<c02ee998>] bond_select_active_slave+0x9e/0xd3
[<c02ed20b>] bond_compute_features+0x45/0x84
[<c02ef9b6>] bond_enslave+0x6a7/0x884
[<c043999c>] mutex_lock_nested+0x2a4/0x2ac
[<c02f61af>] bonding_store_slaves+0x1ae/0x2fb
[<c02f6001>] bonding_store_slaves+0x0/0x2fb
[<c02ce8b7>] dev_attr_store+0x27/0x2c
[<c019bda1>] sysfs_write_file+0xad/0xe0
[<c019bcf4>] sysfs_write_file+0x0/0xe0
[<c0168e5c>] vfs_write+0x8a/0x10c
[<c0118562>] do_page_fault+0x0/0x54a
[<c01693e1>] sys_write+0x41/0x67
[<c0103e92>] sysenter_past_esp+0x5f/0xa5
[<ffffffff>] 0xffffffff
stack backtrace:
Pid: 9, comm: events/0 Not tainted 2.6.24-rc7 #1
[<c0140b2c>] print_irq_inversion_bug+0x108/0x112
[<c0141911>] check_usage_forwards+0x3c/0x41
[<c0141afd>] mark_lock+0x1e7/0x451
[<c0142816>] __lock_acquire+0x440/0xc07
[<c0103f7b>] restore_nocheck+0x12/0x15
[<c0143056>] lock_acquire+0x79/0x93
[<c041258e>] mld_ifc_timer_expire+0x130/0x1fb
[<c041245e>] mld_ifc_timer_expire+0x0/0x1fb
[<c043a8aa>] _spin_lock_bh+0x3b/0x64
[<c041258e>] mld_ifc_timer_expire+0x130/0x1fb
[<c041258e>] mld_ifc_timer_expire+0x130/0x1fb
[<c041245e>] mld_ifc_timer_expire+0x0/0x1fb
[<c0141f7d>] trace_hardirqs_on+0x10c/0x14c
[<c041245e>] mld_ifc_timer_expire+0x0/0x1fb
[<c012e02e>] run_timer_softirq+0xfa/0x15d
[<c012a982>] __do_softirq+0x56/0xdb
[<c0141f7d>] trace_hardirqs_on+0x10c/0x14c
[<c012a994>] __do_softirq+0x68/0xdb
[<c012aa3d>] do_softirq+0x36/0x51
[<c012af26>] local_bh_enable_ip+0xad/0xed
[<c03bfa37>] rt_run_flush+0x64/0x8b
[<c03e9bbe>] fib_netdev_event+0x61/0x65
[<c013ac34>] notifier_call_chain+0x2a/0x52
[<c013ac7e>] raw_notifier_call_chain+0x17/0x1a
[<c03a340d>] netdev_state_change+0x18/0x29
[<c03ac645>] __linkwatch_run_queue+0x150/0x17e
[<c03ac690>] linkwatch_event+0x1d/0x22
[<c0133d87>] run_workqueue+0xdb/0x1b6
[<c0133d33>] run_workqueue+0x87/0x1b6
[<c03ac673>] linkwatch_event+0x0/0x22
[<c01347a7>] worker_thread+0x0/0x85
[<c0134820>] worker_thread+0x79/0x85
[<c0137255>] autoremove_wake_function+0x0/0x35
[<c013719e>] kthread+0x38/0x5e
[<c0137166>] kthread+0x0/0x5e
[<c0104baf>] kernel_thread_helper+0x7/0x10
=======================
bonding: bond0: enslaving eth1 as a backup interface with a down link.
bond0: no IPv6 routers present
Best regards,
Krzysztof Olędzki
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-08 18:50 ` [PATCH 0/3] bonding: 3 fixes for 2.6.24 Krzysztof Oledzki
@ 2008-01-08 19:17 ` Andy Gospodarek
2008-01-08 20:28 ` Jay Vosburgh
2008-01-09 6:08 ` Herbert Xu
2008-01-08 19:30 ` Jay Vosburgh
1 sibling, 2 replies; 30+ messages in thread
From: Andy Gospodarek @ 2008-01-08 19:17 UTC (permalink / raw)
To: Krzysztof Oledzki
Cc: Jay Vosburgh, netdev, Jeff Garzik, David Miller, Andy Gospodarek
On Tue, Jan 08, 2008 at 07:50:22PM +0100, Krzysztof Oledzki wrote:
>
>
> On Mon, 7 Jan 2008, Jay Vosburgh wrote:
>
> > Following are three fixes to fix locking problems and
> >silence locking-related warnings in the current 2.6.24-rc.
> >
> > patch 1: fix locking in sysfs primary/active selection
> >
> > Call core network functions with expected locks to
> >eliminate potential deadlock and silence warnings.
> >
> > patch 2: fix ASSERT_RTNL that produces spurious warnings
> >
> > Relocate ASSERT_RTNL to remove a false warning; after patch,
> >ASSERT is located in code that holds only RTNL (additional locks were
> >causing the ASSERT to trip)
> >
> > patch 3: fix locking during alb failover and slave removal
> >
> > Fix all call paths into alb_fasten_mac_swap to hold only RTNL.
> >Eliminates deadlock and silences warnings.
> >
> > Patches are against the current netdev-2.6#upstream branch.
> >
> > Please apply for 2.6.24.
>
> 2.6.24-rc7 + patches #1, #2, #3:
>
> bonding: bond0: setting mode to active-backup (1).
> bonding: bond0: Setting MII monitoring interval to 100.
> ADDRCONF(NETDEV_UP): bond0: link is not ready
> bonding: bond0: Adding slave eth0.
> e1000: eth0: e1000_watchdog: NIC Link is Up 1000 Mbps Full Duplex, Flow
> Control: RX/TX
> bonding: bond0: making interface eth0 the new active one.
> bonding: bond0: first active interface up!
> bonding: bond0: enslaving eth0 as an active interface with an up link.
> bonding: bond0: Adding slave eth1.
> ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
>
> =========================================================
> [ INFO: possible irq lock inversion dependency detected ]
> 2.6.24-rc7 #1
> ---------------------------------------------------------
> events/0/9 just changed the state of lock:
> (&mc->mca_lock){-+..}, at: [<c041258e>] mld_ifc_timer_expire+0x130/0x1fb
> but this lock took another, soft-read-irq-unsafe lock in the past:
> (&bond->lock){-.--}
>
> and interrupts could create inverse lock ordering between them.
>
>
Jay's patches will not fix this issue. I think something like this did
it for me, but as I mentioned to Jay in the last thread, I'm not
convinced it doesn't violate some of the locking expectations we have.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 423298c..3c6619a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3915,7 +3915,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
struct bonding *bond = bond_dev->priv;
struct dev_mc_list *dmi;
- write_lock_bh(&bond->lock);
+ read_lock(&bond->lock);
/*
* Do promisc before checking multicast_mode
@@ -3957,7 +3957,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
bond_mc_list_destroy(bond);
bond_mc_list_copy(bond_dev->mc_list, bond, GFP_ATOMIC);
- write_unlock_bh(&bond->lock);
+ read_unlock(&bond->lock);
}
/*
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-08 18:50 ` [PATCH 0/3] bonding: 3 fixes for 2.6.24 Krzysztof Oledzki
2008-01-08 19:17 ` Andy Gospodarek
@ 2008-01-08 19:30 ` Jay Vosburgh
2008-01-09 6:35 ` Krzysztof Oledzki
1 sibling, 1 reply; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-08 19:30 UTC (permalink / raw)
To: Krzysztof Oledzki; +Cc: netdev, Jeff Garzik, David Miller, Andy Gospodarek
Krzysztof Oledzki <olel@ans.pl> wrote:
>On Mon, 7 Jan 2008, Jay Vosburgh wrote:
>
>> Following are three fixes to fix locking problems and
>> silence locking-related warnings in the current 2.6.24-rc.
>>
>> patch 1: fix locking in sysfs primary/active selection
>>
>> Call core network functions with expected locks to
>> eliminate potential deadlock and silence warnings.
>>
>> patch 2: fix ASSERT_RTNL that produces spurious warnings
>>
>> Relocate ASSERT_RTNL to remove a false warning; after patch,
>> ASSERT is located in code that holds only RTNL (additional locks were
>> causing the ASSERT to trip)
>>
>> patch 3: fix locking during alb failover and slave removal
>>
>> Fix all call paths into alb_fasten_mac_swap to hold only RTNL.
>> Eliminates deadlock and silences warnings.
>>
>> Patches are against the current netdev-2.6#upstream branch.
>>
>> Please apply for 2.6.24.
>
>2.6.24-rc7 + patches #1, #2, #3:
>
>bonding: bond0: setting mode to active-backup (1).
>bonding: bond0: Setting MII monitoring interval to 100.
>ADDRCONF(NETDEV_UP): bond0: link is not ready
>bonding: bond0: Adding slave eth0.
>e1000: eth0: e1000_watchdog: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
>bonding: bond0: making interface eth0 the new active one.
>bonding: bond0: first active interface up!
>bonding: bond0: enslaving eth0 as an active interface with an up link.
>bonding: bond0: Adding slave eth1.
>ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
>
>=========================================================
>[ INFO: possible irq lock inversion dependency detected ]
>2.6.24-rc7 #1
>---------------------------------------------------------
>events/0/9 just changed the state of lock:
> (&mc->mca_lock){-+..}, at: [<c041258e>] mld_ifc_timer_expire+0x130/0x1fb
>but this lock took another, soft-read-irq-unsafe lock in the past:
> (&bond->lock){-.--}
>
>and interrupts could create inverse lock ordering between them.
Just to be clear: the patch set I posted yesterday was not
intended to resolve the lockdep problem; I haven't studied that one yet.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-08 19:17 ` Andy Gospodarek
@ 2008-01-08 20:28 ` Jay Vosburgh
2008-01-09 6:08 ` Herbert Xu
1 sibling, 0 replies; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-08 20:28 UTC (permalink / raw)
To: Andy Gospodarek; +Cc: Krzysztof Oledzki, netdev, Jeff Garzik, David Miller
Andy Gospodarek <andy@greyhouse.net> wrote:
[...]
>Jay's patches will not fix this issue. I think something like this did
>it for me, but as I mentioned to Jay in the last thread, I'm not
>convinced it doesn't violate some of the locking expectations we have.
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 423298c..3c6619a 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -3915,7 +3915,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> struct bonding *bond = bond_dev->priv;
> struct dev_mc_list *dmi;
>
>- write_lock_bh(&bond->lock);
>+ read_lock(&bond->lock);
>
> /*
> * Do promisc before checking multicast_mode
>@@ -3957,7 +3957,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> bond_mc_list_destroy(bond);
> bond_mc_list_copy(bond_dev->mc_list, bond, GFP_ATOMIC);
>
>- write_unlock_bh(&bond->lock);
>+ read_unlock(&bond->lock);
> }
>
> /*
Actually, I think we might be good here with no locks at all, as
it appears that all of the accesses to and manipulations of the
bond->mc_list are protected under RTNL. I haven't checked this 100%,
but it looks that way to me after 20 minutes of poking around. I'm
pretty sure that bonding doesn't internally mess with the mc_lists
without RTNL, it's the outside callers that I'm not entirely sure of.
I delve into "no locks" because bond_set_multicast_list should
do a bunch of things with no extra locks beyond RTNL (all of the calls
to bond_set_promisc, and _allmulti), so simply removing the acquisition
of bond->lock would help there, too. I don't think we'll go down the
promisc or allmulti paths when called from ipv6 (which holds extra locks
in addition to RTNL) because those (apparently) won't alter the
IFF_PROMISC or IFF_ALLMULTI flags.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-08 19:17 ` Andy Gospodarek
2008-01-08 20:28 ` Jay Vosburgh
@ 2008-01-09 6:08 ` Herbert Xu
1 sibling, 0 replies; 30+ messages in thread
From: Herbert Xu @ 2008-01-09 6:08 UTC (permalink / raw)
To: Andy Gospodarek; +Cc: olel, fubar, netdev, jgarzik, davem, andy
Andy Gospodarek <andy@greyhouse.net> wrote:
>
> Jay's patches will not fix this issue. I think something like this did
> it for me, but as I mentioned to Jay in the last thread, I'm not
> convinced it doesn't violate some of the locking expectations we have.
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 423298c..3c6619a 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3915,7 +3915,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> struct bonding *bond = bond_dev->priv;
> struct dev_mc_list *dmi;
>
> - write_lock_bh(&bond->lock);
> + read_lock(&bond->lock);
This is wrong. As I said before, the correct fix (until set_multicast
no longer gets called from BH context) is to change all you process
context read_lock's to read_lock_bh.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-08 19:30 ` Jay Vosburgh
@ 2008-01-09 6:35 ` Krzysztof Oledzki
2008-01-09 7:58 ` Jay Vosburgh
0 siblings, 1 reply; 30+ messages in thread
From: Krzysztof Oledzki @ 2008-01-09 6:35 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, Jeff Garzik, David Miller, Andy Gospodarek
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2403 bytes --]
On Tue, 8 Jan 2008, Jay Vosburgh wrote:
> Krzysztof Oledzki <olel@ans.pl> wrote:
>
>> On Mon, 7 Jan 2008, Jay Vosburgh wrote:
>>
>>> Following are three fixes to fix locking problems and
>>> silence locking-related warnings in the current 2.6.24-rc.
>>>
>>> patch 1: fix locking in sysfs primary/active selection
>>>
>>> Call core network functions with expected locks to
>>> eliminate potential deadlock and silence warnings.
>>>
>>> patch 2: fix ASSERT_RTNL that produces spurious warnings
>>>
>>> Relocate ASSERT_RTNL to remove a false warning; after patch,
>>> ASSERT is located in code that holds only RTNL (additional locks were
>>> causing the ASSERT to trip)
>>>
>>> patch 3: fix locking during alb failover and slave removal
>>>
>>> Fix all call paths into alb_fasten_mac_swap to hold only RTNL.
>>> Eliminates deadlock and silences warnings.
>>>
>>> Patches are against the current netdev-2.6#upstream branch.
>>>
>>> Please apply for 2.6.24.
>>
>> 2.6.24-rc7 + patches #1, #2, #3:
>>
>> bonding: bond0: setting mode to active-backup (1).
>> bonding: bond0: Setting MII monitoring interval to 100.
>> ADDRCONF(NETDEV_UP): bond0: link is not ready
>> bonding: bond0: Adding slave eth0.
>> e1000: eth0: e1000_watchdog: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
>> bonding: bond0: making interface eth0 the new active one.
>> bonding: bond0: first active interface up!
>> bonding: bond0: enslaving eth0 as an active interface with an up link.
>> bonding: bond0: Adding slave eth1.
>> ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
>>
>> =========================================================
>> [ INFO: possible irq lock inversion dependency detected ]
>> 2.6.24-rc7 #1
>> ---------------------------------------------------------
>> events/0/9 just changed the state of lock:
>> (&mc->mca_lock){-+..}, at: [<c041258e>] mld_ifc_timer_expire+0x130/0x1fb
>> but this lock took another, soft-read-irq-unsafe lock in the past:
>> (&bond->lock){-.--}
>>
>> and interrupts could create inverse lock ordering between them.
>
> Just to be clear: the patch set I posted yesterday was not
> intended to resolve the lockdep problem; I haven't studied that one yet.
Fine. Just let you know that someone test your patches and everything
works, except mentioned problem.
Best regards,
Krzysztof Olędzki
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-09 6:35 ` Krzysztof Oledzki
@ 2008-01-09 7:58 ` Jay Vosburgh
2008-01-09 9:36 ` Krzysztof Oledzki
2008-01-09 15:27 ` Andy Gospodarek
0 siblings, 2 replies; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-09 7:58 UTC (permalink / raw)
To: Krzysztof Oledzki
Cc: netdev, Jeff Garzik, David Miller, Andy Gospodarek, Herbert Xu
Krzysztof Oledzki <olel@ans.pl> wrote:
>Fine. Just let you know that someone test your patches and everything
>works, except mentioned problem.
And I appreciate it; I just wanted to make sure our many fans
following along at home didn't misunderstand.
Could you let me know if the patch below make the lockdep
warning go away? This applies on top of the previous three, although it
should be trivial to do by hand.
I'm still checking to make sure this is safe with regard to
mutexing the bonding structures, but it would be good to know if it
eliminates the warning.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 77d004d..1baaadc 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3937,8 +3937,6 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
struct bonding *bond = bond_dev->priv;
struct dev_mc_list *dmi;
- write_lock_bh(&bond->lock);
-
/*
* Do promisc before checking multicast_mode
*/
@@ -3978,8 +3976,6 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
/* save master's multicast list */
bond_mc_list_destroy(bond);
bond_mc_list_copy(bond_dev->mc_list, bond, GFP_ATOMIC);
-
- write_unlock_bh(&bond->lock);
}
/*
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-09 7:58 ` Jay Vosburgh
@ 2008-01-09 9:36 ` Krzysztof Oledzki
2008-01-09 15:27 ` Andy Gospodarek
1 sibling, 0 replies; 30+ messages in thread
From: Krzysztof Oledzki @ 2008-01-09 9:36 UTC (permalink / raw)
To: Jay Vosburgh
Cc: netdev, Jeff Garzik, David Miller, Andy Gospodarek, Herbert Xu
[-- Attachment #1: Type: TEXT/PLAIN, Size: 800 bytes --]
On Tue, 8 Jan 2008, Jay Vosburgh wrote:
> Krzysztof Oledzki <olel@ans.pl> wrote:
>
>> Fine. Just let you know that someone test your patches and everything
>> works, except mentioned problem.
>
> And I appreciate it; I just wanted to make sure our many fans
> following along at home didn't misunderstand.
>
> Could you let me know if the patch below make the lockdep
> warning go away? This applies on top of the previous three, although it
> should be trivial to do by hand.
>
> I'm still checking to make sure this is safe with regard to
> mutexing the bonding structures, but it would be good to know if it
> eliminates the warning.
I can confirm that the warning went away.
Tested-by: Krzysztof Piotr Oledzki <ole@ans.pl>
Best regards,
Krzysztof Olędzki
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-09 7:58 ` Jay Vosburgh
2008-01-09 9:36 ` Krzysztof Oledzki
@ 2008-01-09 15:27 ` Andy Gospodarek
2008-01-09 17:54 ` Jay Vosburgh
1 sibling, 1 reply; 30+ messages in thread
From: Andy Gospodarek @ 2008-01-09 15:27 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Krzysztof Oledzki, netdev, Jeff Garzik, David Miller,
Andy Gospodarek, Herbert Xu
On Tue, Jan 08, 2008 at 11:58:34PM -0800, Jay Vosburgh wrote:
> Krzysztof Oledzki <olel@ans.pl> wrote:
>
> >Fine. Just let you know that someone test your patches and everything
> >works, except mentioned problem.
>
> And I appreciate it; I just wanted to make sure our many fans
> following along at home didn't misunderstand.
>
> Could you let me know if the patch below make the lockdep
> warning go away? This applies on top of the previous three, although it
> should be trivial to do by hand.
>
> I'm still checking to make sure this is safe with regard to
> mutexing the bonding structures, but it would be good to know if it
> eliminates the warning.
>
> -J
>
Jay,
My initial concern was that a slave device could disappear out from
under us, but it seems like this certainly isn't the case since all
calls to bond_release are protected by rtnl-locks, so I think you are
correct that we are safe. I'll test this on my setup here and let you
know if I see any problems.
-andy
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-09 15:27 ` Andy Gospodarek
@ 2008-01-09 17:54 ` Jay Vosburgh
2008-01-09 20:17 ` Andy Gospodarek
0 siblings, 1 reply; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-09 17:54 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Krzysztof Oledzki, netdev, Jeff Garzik, David Miller, Herbert Xu
Andy Gospodarek <andy@greyhouse.net> wrote:
[...]
>My initial concern was that a slave device could disappear out from
>under us, but it seems like this certainly isn't the case since all
>calls to bond_release are protected by rtnl-locks, so I think you are
>correct that we are safe. I'll test this on my setup here and let you
>know if I see any problems.
Yep, all entries into enslave or remove come in with RTNL, so if
we have RTNL there then slaves can't vanish.
On further inspection, I don't think it's safe to simply drop
the locks in bond_set_multicast_list, I'm seeing a couple of cases that
could be troublesome:
bond_set_promiscuity and bond_set_allmulti both reference
curr_active_slave, which isn't protected from change by RTNL, so that
could conflict with a change_active_slave calling bond_mc_swap (which is
also holding the wrong locks for dev_set_promisc/allmulti).
It also looks like there are paths (igmp6 for one) into
dev_mc_add that just hold a bunch of regular locks, and not RTNL, so
those wouldn't be safe from having slaves vanish due to concurrent
deslavement.
Looks like read_lock_bh for bond-lock and curr_slave_lock is
needed in bond_set_multicast_list, and some dropping of locks is needed
inside bond_set_promisc/allmulti. Methinks that without any locks,
bond_mc_add/delete could race with either a change of active slave or a
de-enslavement of the active slave.
I'm wondering if this is worth trying to make perfect for 2.6.24
(and maybe making things worse), and, instead, just do this:
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 77d004d..8b9e33a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3937,7 +3937,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
struct bonding *bond = bond_dev->priv;
struct dev_mc_list *dmi;
- write_lock_bh(&bond->lock);
+ read_lock_bh(&bond->lock);
/*
* Do promisc before checking multicast_mode
@@ -3979,7 +3979,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
bond_mc_list_destroy(bond);
bond_mc_list_copy(bond_dev->mc_list, bond, GFP_ATOMIC);
- write_unlock_bh(&bond->lock);
+ read_unlock_bh(&bond->lock);
}
/*
This should silence the lockdep (if I'm understanding what
everybody's saying), and keep the change set to a minimum. This might
not even be worth pushing for 2.6.24; I'm not exactly sure how difficult
the lockdep problem would be to trigger.
The other stuff I mention above can be dealt with later; they're
very low-probability races that would be pretty difficult to hit even on
purpose.
Thoughts?
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-09 17:54 ` Jay Vosburgh
@ 2008-01-09 20:17 ` Andy Gospodarek
2008-01-09 22:05 ` Herbert Xu
2008-01-12 10:53 ` Krzysztof Oledzki
0 siblings, 2 replies; 30+ messages in thread
From: Andy Gospodarek @ 2008-01-09 20:17 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Andy Gospodarek, Krzysztof Oledzki, netdev, Jeff Garzik,
David Miller, Herbert Xu
On Wed, Jan 09, 2008 at 09:54:56AM -0800, Jay Vosburgh wrote:
> Andy Gospodarek <andy@greyhouse.net> wrote:
> [...]
> >My initial concern was that a slave device could disappear out from
> >under us, but it seems like this certainly isn't the case since all
> >calls to bond_release are protected by rtnl-locks, so I think you are
> >correct that we are safe. I'll test this on my setup here and let you
> >know if I see any problems.
>
> Yep, all entries into enslave or remove come in with RTNL, so if
> we have RTNL there then slaves can't vanish.
>
> On further inspection, I don't think it's safe to simply drop
> the locks in bond_set_multicast_list, I'm seeing a couple of cases that
> could be troublesome:
>
> bond_set_promiscuity and bond_set_allmulti both reference
> curr_active_slave, which isn't protected from change by RTNL, so that
> could conflict with a change_active_slave calling bond_mc_swap (which is
> also holding the wrong locks for dev_set_promisc/allmulti).
>
> It also looks like there are paths (igmp6 for one) into
> dev_mc_add that just hold a bunch of regular locks, and not RTNL, so
> those wouldn't be safe from having slaves vanish due to concurrent
> deslavement.
Eeeek! I didn't realize that rtnl wasn't held for all those calls. If
that's the case we can't drop all the locks.
> Looks like read_lock_bh for bond-lock and curr_slave_lock is
> needed in bond_set_multicast_list, and some dropping of locks is needed
> inside bond_set_promisc/allmulti. Methinks that without any locks,
> bond_mc_add/delete could race with either a change of active slave or a
> de-enslavement of the active slave.
Agreed. And despite Herbert's opinion that this isn't the correct fix,
I think this will work fine. This is one of the cases where we can take
a write_lock(bond->lock) in softirq context, so we need to drop that (or
make sure all the read_lock's are read_lock_bh's). The latter isn't
really an option since having a majority of the bonding code run in
softirq context was what we are trying to avoid with the workqueue
conversion.
> I'm wondering if this is worth trying to make perfect for 2.6.24
> (and maybe making things worse), and, instead, just do this:
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 77d004d..8b9e33a 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3937,7 +3937,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> struct bonding *bond = bond_dev->priv;
> struct dev_mc_list *dmi;
>
> - write_lock_bh(&bond->lock);
> + read_lock_bh(&bond->lock);
>
> /*
> * Do promisc before checking multicast_mode
> @@ -3979,7 +3979,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> bond_mc_list_destroy(bond);
> bond_mc_list_copy(bond_dev->mc_list, bond, GFP_ATOMIC);
>
> - write_unlock_bh(&bond->lock);
> + read_unlock_bh(&bond->lock);
> }
>
> /*
>
>
> This should silence the lockdep (if I'm understanding what
> everybody's saying), and keep the change set to a minimum. This might
The lockdep problem is easy to trigger. The lockdep code does a good
job of noticing problems quickly regardless of how easy the deadlocks
are to create.
> not even be worth pushing for 2.6.24; I'm not exactly sure how difficult
> the lockdep problem would be to trigger.
>
I'd like to see it go in there (for correct-ness) and to avoid hearing
about these lockdep issues for the next few months until it makes it
into 2.4.25.
> The other stuff I mention above can be dealt with later; they're
> very low-probability races that would be pretty difficult to hit even on
> purpose.
>
> Thoughts?
>
> -J
>
> ---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
> --
> 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] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-09 20:17 ` Andy Gospodarek
@ 2008-01-09 22:05 ` Herbert Xu
2008-01-09 23:19 ` Jay Vosburgh
2008-01-12 10:53 ` Krzysztof Oledzki
1 sibling, 1 reply; 30+ messages in thread
From: Herbert Xu @ 2008-01-09 22:05 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jay Vosburgh, Krzysztof Oledzki, netdev, Jeff Garzik,
David Miller
On Wed, Jan 09, 2008 at 03:17:09PM -0500, Andy Gospodarek wrote:
>
> Agreed. And despite Herbert's opinion that this isn't the correct fix,
> I think this will work fine. This is one of the cases where we can take
> a write_lock(bond->lock) in softirq context, so we need to drop that (or
> make sure all the read_lock's are read_lock_bh's). The latter isn't
> really an option since having a majority of the bonding code run in
> softirq context was what we are trying to avoid with the workqueue
> conversion.
No that's not the point. The point is to move the majority of the code
into process context so that you can take the RTNL. Once you have taken
the RTNL you can disable BH all you want and I don't care one bit.
In any case, fixing a known dead-lock is important.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-09 22:05 ` Herbert Xu
@ 2008-01-09 23:19 ` Jay Vosburgh
2008-01-10 0:58 ` Herbert Xu
0 siblings, 1 reply; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-09 23:19 UTC (permalink / raw)
To: Herbert Xu
Cc: Andy Gospodarek, Krzysztof Oledzki, netdev, Jeff Garzik,
David Miller
Herbert Xu <herbert@gondor.apana.org.au> wrote:
>On Wed, Jan 09, 2008 at 03:17:09PM -0500, Andy Gospodarek wrote:
>>
>> Agreed. And despite Herbert's opinion that this isn't the correct fix,
>> I think this will work fine. This is one of the cases where we can take
>> a write_lock(bond->lock) in softirq context, so we need to drop that (or
>> make sure all the read_lock's are read_lock_bh's). The latter isn't
>> really an option since having a majority of the bonding code run in
>> softirq context was what we are trying to avoid with the workqueue
>> conversion.
>
>No that's not the point. The point is to move the majority of the code
>into process context so that you can take the RTNL. Once you have taken
>the RTNL you can disable BH all you want and I don't care one bit.
I'm not sure how we could move more code into a process context;
much of the bonding driver is at the mercy of its callers, as in this
case. The monitoring stuff and enslave / deslave is all in a process
context now (workqueue). The transmit processing functions, for
example, can't be assumed to be in any particular context as they're
called by dev_queue_xmit.
The function in question here is the dev->set_multicast_list
method function, which is sometimes called with RTNL, and sometimes with
other random locks, but seems to always be called with netif_tx_lock_bh.
Then, bonding's bond_set_multicast_list calls dev_set_promiscuity (on
slaves), which I believe is an "RTNL only" function (which is probably
another discussion).
>In any case, fixing a known dead-lock is important.
Agreed.
I'm now thinking for just the topic at hand (the previously
posted lockdep report), something like this will resolve it:
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 77d004d..b7ac10b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3937,8 +3937,6 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
struct bonding *bond = bond_dev->priv;
struct dev_mc_list *dmi;
- write_lock_bh(&bond->lock);
-
/*
* Do promisc before checking multicast_mode
*/
@@ -3959,6 +3957,8 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
bond_set_allmulti(bond, -1);
}
+ read_lock_bh(&bond->lock);
+
bond->flags = bond_dev->flags;
/* looking for addresses to add to slaves' mc list */
@@ -3979,7 +3979,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
bond_mc_list_destroy(bond);
bond_mc_list_copy(bond_dev->mc_list, bond, GFP_ATOMIC);
- write_unlock_bh(&bond->lock);
+ read_unlock_bh(&bond->lock);
}
/*
This (a) converts the write_lock_bh to a read_lock_bh, and moves
it down to not cover the promisc/allmulti code (which is protected by
RTNL). I'm not sure that this is really a signficant alteration, but
it's a bit closer to "correct."
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-09 23:19 ` Jay Vosburgh
@ 2008-01-10 0:58 ` Herbert Xu
2008-01-10 14:51 ` Andy Gospodarek
0 siblings, 1 reply; 30+ messages in thread
From: Herbert Xu @ 2008-01-10 0:58 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Andy Gospodarek, Krzysztof Oledzki, netdev, Jeff Garzik,
David Miller
On Wed, Jan 09, 2008 at 03:19:10PM -0800, Jay Vosburgh wrote:
>
> >No that's not the point. The point is to move the majority of the code
> >into process context so that you can take the RTNL. Once you have taken
> >the RTNL you can disable BH all you want and I don't care one bit.
>
> I'm not sure how we could move more code into a process context;
> much of the bonding driver is at the mercy of its callers, as in this
> case. The monitoring stuff and enslave / deslave is all in a process
> context now (workqueue). The transmit processing functions, for
> example, can't be assumed to be in any particular context as they're
> called by dev_queue_xmit.
No I'm not calling for you to move any more code into process context.
I was replying to the comment that changing the read_lock calls in
process context to read_lock_bh somehow undoes the benefit of moving
softirq code into process context. It does not since the point of the
move is to be able to take the RTNL, which you can still do as long as
you do it before you disable BH.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-10 0:58 ` Herbert Xu
@ 2008-01-10 14:51 ` Andy Gospodarek
2008-01-10 20:36 ` Herbert Xu
2008-01-10 20:45 ` Jay Vosburgh
0 siblings, 2 replies; 30+ messages in thread
From: Andy Gospodarek @ 2008-01-10 14:51 UTC (permalink / raw)
To: Herbert Xu
Cc: Jay Vosburgh, Andy Gospodarek, Krzysztof Oledzki, netdev,
Jeff Garzik, David Miller
On Thu, Jan 10, 2008 at 11:58:09AM +1100, Herbert Xu wrote:
> On Wed, Jan 09, 2008 at 03:19:10PM -0800, Jay Vosburgh wrote:
> >
> > >No that's not the point. The point is to move the majority of the code
> > >into process context so that you can take the RTNL. Once you have taken
> > >the RTNL you can disable BH all you want and I don't care one bit.
> >
> > I'm not sure how we could move more code into a process context;
> > much of the bonding driver is at the mercy of its callers, as in this
> > case. The monitoring stuff and enslave / deslave is all in a process
> > context now (workqueue). The transmit processing functions, for
> > example, can't be assumed to be in any particular context as they're
> > called by dev_queue_xmit.
>
> No I'm not calling for you to move any more code into process context.
> I was replying to the comment that changing the read_lock calls in
> process context to read_lock_bh somehow undoes the benefit of moving
> softirq code into process context. It does not since the point of the
> move is to be able to take the RTNL, which you can still do as long as
> you do it before you disable BH.
>
That wasn't the only purpose, Herbert. Making sure that calls to
dev_set_mac_address were called from process context was important at
the time of the coding as well since at least the tg3 driver took locks
that could not be taken reliably in soft-irq context. Michael Chan
fixed this here:
commit 986e0aeb9ae09127b401c3baa66f15b7a31f354c
Author: Michael Chan <mchan@broadcom.com>
Date: Sat May 5 12:10:20 2007 -0700
[TG3]: Remove reset during MAC address changes.
so if wasn't as much of an issue after that, but moving as much of the
code to process context was important for that as well (hence the move
to not continue to try to not use bh-locks everywhere).
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-10 14:51 ` Andy Gospodarek
@ 2008-01-10 20:36 ` Herbert Xu
2008-01-10 20:50 ` Jay Vosburgh
2008-01-10 20:45 ` Jay Vosburgh
1 sibling, 1 reply; 30+ messages in thread
From: Herbert Xu @ 2008-01-10 20:36 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jay Vosburgh, Krzysztof Oledzki, netdev, Jeff Garzik,
David Miller
On Thu, Jan 10, 2008 at 09:51:44AM -0500, Andy Gospodarek wrote:
>
> That wasn't the only purpose, Herbert. Making sure that calls to
> dev_set_mac_address were called from process context was important at
> the time of the coding as well since at least the tg3 driver took locks
> that could not be taken reliably in soft-irq context. Michael Chan
> fixed this here:
Sure, but where do you call that function while holding the bond lock?
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-10 14:51 ` Andy Gospodarek
2008-01-10 20:36 ` Herbert Xu
@ 2008-01-10 20:45 ` Jay Vosburgh
1 sibling, 0 replies; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-10 20:45 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Herbert Xu, Krzysztof Oledzki, netdev, Jeff Garzik, David Miller
Andy Gospodarek <andy@greyhouse.net> wrote:
[...]
>That wasn't the only purpose, Herbert. Making sure that calls to
>dev_set_mac_address were called from process context was important at
>the time of the coding as well since at least the tg3 driver took locks
>that could not be taken reliably in soft-irq context. Michael Chan
>fixed this here:
>
>commit 986e0aeb9ae09127b401c3baa66f15b7a31f354c
>Author: Michael Chan <mchan@broadcom.com>
>Date: Sat May 5 12:10:20 2007 -0700
>
> [TG3]: Remove reset during MAC address changes.
>
>so if wasn't as much of an issue after that, but moving as much of the
>code to process context was important for that as well (hence the move
>to not continue to try to not use bh-locks everywhere).
Well, not for tg3 perhaps, but other network device drivers do
the same thing (if memory serves, any USB ethernet adapter will have
issues there). Also, I believe the netlink notifier callback,
rtnetlink_event, which every dev_set_whatever calls, does a
possibly-sleeping memory allocation (rtmsg_ifinfo -> nlmsg_new ->
alloc_skb(GFP_KERNEL)); so we don't really want to hold extra locks for
that, either.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-10 20:36 ` Herbert Xu
@ 2008-01-10 20:50 ` Jay Vosburgh
2008-01-10 21:03 ` Andy Gospodarek
0 siblings, 1 reply; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-10 20:50 UTC (permalink / raw)
To: Herbert Xu
Cc: Andy Gospodarek, Krzysztof Oledzki, netdev, Jeff Garzik,
David Miller
Herbert Xu <herbert@gondor.apana.org.au> wrote:
>On Thu, Jan 10, 2008 at 09:51:44AM -0500, Andy Gospodarek wrote:
>>
>> That wasn't the only purpose, Herbert. Making sure that calls to
>> dev_set_mac_address were called from process context was important at
>> the time of the coding as well since at least the tg3 driver took locks
>> that could not be taken reliably in soft-irq context. Michael Chan
>> fixed this here:
>
>Sure, but where do you call that function while holding the bond lock?
If I recall correctly, the problem was that tg3, et al, did
things that might sleep, and bonding was calling from a timer context,
which couldn't sleep. It wasn't about the lock.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-10 20:50 ` Jay Vosburgh
@ 2008-01-10 21:03 ` Andy Gospodarek
2008-01-10 21:05 ` Herbert Xu
0 siblings, 1 reply; 30+ messages in thread
From: Andy Gospodarek @ 2008-01-10 21:03 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Herbert Xu, Andy Gospodarek, Krzysztof Oledzki, netdev,
Jeff Garzik, David Miller
On Thu, Jan 10, 2008 at 12:50:46PM -0800, Jay Vosburgh wrote:
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> >On Thu, Jan 10, 2008 at 09:51:44AM -0500, Andy Gospodarek wrote:
> >>
> >> That wasn't the only purpose, Herbert. Making sure that calls to
> >> dev_set_mac_address were called from process context was important at
> >> the time of the coding as well since at least the tg3 driver took locks
> >> that could not be taken reliably in soft-irq context. Michael Chan
> >> fixed this here:
> >
> >Sure, but where do you call that function while holding the bond lock?
>
> If I recall correctly, the problem was that tg3, et al, did
> things that might sleep, and bonding was calling from a timer context,
> which couldn't sleep. It wasn't about the lock.
>
Exactly, I was just about to post the same.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-10 21:03 ` Andy Gospodarek
@ 2008-01-10 21:05 ` Herbert Xu
2008-01-11 1:06 ` Jay Vosburgh
0 siblings, 1 reply; 30+ messages in thread
From: Herbert Xu @ 2008-01-10 21:05 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jay Vosburgh, Krzysztof Oledzki, netdev, Jeff Garzik,
David Miller
On Thu, Jan 10, 2008 at 04:03:53PM -0500, Andy Gospodarek wrote:
>
> > >Sure, but where do you call that function while holding the bond lock?
> >
> > If I recall correctly, the problem was that tg3, et al, did
> > things that might sleep, and bonding was calling from a timer context,
> > which couldn't sleep. It wasn't about the lock.
>
> Exactly, I was just about to post the same.
In other words, changing read_lock on bond->lock to read_lock_bh doesn't
affect this one bit.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-10 21:05 ` Herbert Xu
@ 2008-01-11 1:06 ` Jay Vosburgh
2008-01-11 4:55 ` Herbert Xu
0 siblings, 1 reply; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-11 1:06 UTC (permalink / raw)
To: Herbert Xu
Cc: Andy Gospodarek, Krzysztof Oledzki, netdev, Jeff Garzik,
David Miller
Herbert Xu <herbert@gondor.apana.org.au> wrote:
>On Thu, Jan 10, 2008 at 04:03:53PM -0500, Andy Gospodarek wrote:
>>
>> > >Sure, but where do you call that function while holding the bond lock?
>> >
>> > If I recall correctly, the problem was that tg3, et al, did
>> > things that might sleep, and bonding was calling from a timer context,
>> > which couldn't sleep. It wasn't about the lock.
>>
>> Exactly, I was just about to post the same.
>
>In other words, changing read_lock on bond->lock to read_lock_bh doesn't
>affect this one bit.
For the case of the bond_set_multicast_list function, changing
the existing write_lock to a read_lock_bh doesn't affect any calls to
dev_set_mac_address (since it isn't called from there) and, apparently,
also resolves the lockdep warning. I'm still trying to get lockdep to
generate the warning for me locally; I'm not sure which magic thing I'm
missing.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-11 1:06 ` Jay Vosburgh
@ 2008-01-11 4:55 ` Herbert Xu
0 siblings, 0 replies; 30+ messages in thread
From: Herbert Xu @ 2008-01-11 4:55 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: herbert, andy, olel, netdev, jgarzik, davem
Jay Vosburgh <fubar@us.ibm.com> wrote:
>
> For the case of the bond_set_multicast_list function, changing
> the existing write_lock to a read_lock_bh doesn't affect any calls to
Right that should also resolve the lockdep issue. So as long as you
guys are sure that this is safe with respect to the rest of the bonding
driver it's fine by me.
BTW you only need read_lock since set_multicast_list always gets called
with BH off.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-09 20:17 ` Andy Gospodarek
2008-01-09 22:05 ` Herbert Xu
@ 2008-01-12 10:53 ` Krzysztof Oledzki
2008-01-12 17:56 ` Jay Vosburgh
1 sibling, 1 reply; 30+ messages in thread
From: Krzysztof Oledzki @ 2008-01-12 10:53 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jay Vosburgh, netdev, Jeff Garzik, David Miller, Herbert Xu
[-- Attachment #1: Type: TEXT/PLAIN, Size: 947 bytes --]
On Wed, 9 Jan 2008, Andy Gospodarek wrote:
> On Wed, Jan 09, 2008 at 09:54:56AM -0800, Jay Vosburgh wrote:
<CUT>
>> This should silence the lockdep (if I'm understanding what
>> everybody's saying), and keep the change set to a minimum. This might
>
> The lockdep problem is easy to trigger. The lockdep code does a good
> job of noticing problems quickly regardless of how easy the deadlocks
> are to create.
Exactly. All I need to do is to reboot my server, I have 100% probability
to get the warning.
>> not even be worth pushing for 2.6.24; I'm not exactly sure how difficult
>> the lockdep problem would be to trigger.
>>
>
> I'd like to see it go in there (for correct-ness) and to avoid hearing
> about these lockdep issues for the next few months until it makes it
> into 2.4.25.
Right. So, what is the final patch? I would like to test it if that's
possible. ;)
Best regards,
Krzysztof Olędzki
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-12 10:53 ` Krzysztof Oledzki
@ 2008-01-12 17:56 ` Jay Vosburgh
2008-01-13 0:19 ` Herbert Xu
2008-01-14 22:15 ` Krzysztof Oledzki
0 siblings, 2 replies; 30+ messages in thread
From: Jay Vosburgh @ 2008-01-12 17:56 UTC (permalink / raw)
To: Krzysztof Oledzki
Cc: Andy Gospodarek, netdev, Jeff Garzik, David Miller, Herbert Xu
Krzysztof Oledzki <olel@ans.pl> wrote:
[...]
>Exactly. All I need to do is to reboot my server, I have 100% probability
>to get the warning.
I wish it were that easy for me; I'm not sure what magic thing
you've got on your server or network that I don't, but I haven't been
able to make this lockdep warning happen at all.
>Right. So, what is the final patch? I would like to test it if that's
>possible. ;)
Can you test the following and let me know if it triggers the
warning? I believe this is the minimum locking needed, and based on
input from Herbert, we shouldn't need to hold the lock at _bh. If this
one works, and nobody sees any other issues with it, then it's the final
patch for this lockdep problem. I'll add some deep, meaningful comments
to explain the locking a bit (i.e., we're called with rtnl for the
allmulti and promisc cases, so we're ok there without additional locks,
but the later code could be called from anywhere, so it needs locks to
prevent the slave list from changing, but the mc_lists themselves are
covered by the netif_tx_lock that all callers will hold), but this would
be the actual code change.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 77d004d..6906dbc 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3937,8 +3937,6 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
struct bonding *bond = bond_dev->priv;
struct dev_mc_list *dmi;
- write_lock_bh(&bond->lock);
-
/*
* Do promisc before checking multicast_mode
*/
@@ -3959,6 +3957,8 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
bond_set_allmulti(bond, -1);
}
+ read_lock(&bond->lock);
+
bond->flags = bond_dev->flags;
/* looking for addresses to add to slaves' mc list */
@@ -3979,7 +3979,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
bond_mc_list_destroy(bond);
bond_mc_list_copy(bond_dev->mc_list, bond, GFP_ATOMIC);
- write_unlock_bh(&bond->lock);
+ read_unlock(&bond->lock);
}
/*
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-12 17:56 ` Jay Vosburgh
@ 2008-01-13 0:19 ` Herbert Xu
2008-01-14 22:15 ` Krzysztof Oledzki
1 sibling, 0 replies; 30+ messages in thread
From: Herbert Xu @ 2008-01-13 0:19 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: olel, andy, netdev, jgarzik, davem, herbert
Jay Vosburgh <fubar@us.ibm.com> wrote:
>
> Can you test the following and let me know if it triggers the
> warning? I believe this is the minimum locking needed, and based on
> input from Herbert, we shouldn't need to hold the lock at _bh. If this
> one works, and nobody sees any other issues with it, then it's the final
> patch for this lockdep problem. I'll add some deep, meaningful comments
> to explain the locking a bit (i.e., we're called with rtnl for the
> allmulti and promisc cases, so we're ok there without additional locks,
> but the later code could be called from anywhere, so it needs locks to
> prevent the slave list from changing, but the mc_lists themselves are
> covered by the netif_tx_lock that all callers will hold), but this would
> be the actual code change.
I just had a look at the bonding code and while I didn't find anything
wrong with the change of the write lock to a read lock, the mc_list
itself does not seem to have adequete protection. In particular, there
doesn't seem to be anything protecting the walking of mc_list in
bond_enslave.
This could be a problem if we have an IGMP6 event triggering the change
in the master's mc_list.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24
2008-01-12 17:56 ` Jay Vosburgh
2008-01-13 0:19 ` Herbert Xu
@ 2008-01-14 22:15 ` Krzysztof Oledzki
1 sibling, 0 replies; 30+ messages in thread
From: Krzysztof Oledzki @ 2008-01-14 22:15 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Andy Gospodarek, netdev, Jeff Garzik, David Miller, Herbert Xu
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2394 bytes --]
On Sat, 12 Jan 2008, Jay Vosburgh wrote:
> Krzysztof Oledzki <olel@ans.pl> wrote:
> [...]
>> Exactly. All I need to do is to reboot my server, I have 100% probability
>> to get the warning.
>
> I wish it were that easy for me; I'm not sure what magic thing
> you've got on your server or network that I don't, but I haven't been
> able to make this lockdep warning happen at all.
>
>> Right. So, what is the final patch? I would like to test it if that's
>> possible. ;)
>
> Can you test the following and let me know if it triggers the
> warning? I believe this is the minimum locking needed, and based on
> input from Herbert, we shouldn't need to hold the lock at _bh. If this
> one works, and nobody sees any other issues with it, then it's the final
> patch for this lockdep problem. I'll add some deep, meaningful comments
> to explain the locking a bit (i.e., we're called with rtnl for the
> allmulti and promisc cases, so we're ok there without additional locks,
> but the later code could be called from anywhere, so it needs locks to
> prevent the slave list from changing, but the mc_lists themselves are
> covered by the netif_tx_lock that all callers will hold), but this would
> be the actual code change.
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 77d004d..6906dbc 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3937,8 +3937,6 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> struct bonding *bond = bond_dev->priv;
> struct dev_mc_list *dmi;
>
> - write_lock_bh(&bond->lock);
> -
> /*
> * Do promisc before checking multicast_mode
> */
> @@ -3959,6 +3957,8 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> bond_set_allmulti(bond, -1);
> }
>
> + read_lock(&bond->lock);
> +
> bond->flags = bond_dev->flags;
>
> /* looking for addresses to add to slaves' mc list */
> @@ -3979,7 +3979,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
> bond_mc_list_destroy(bond);
> bond_mc_list_copy(bond_dev->mc_list, bond, GFP_ATOMIC);
>
> - write_unlock_bh(&bond->lock);
> + read_unlock(&bond->lock);
> }
>
> /*
>
>
I can confirm that the warning went away.
Tested-by: Krzysztof Piotr Oledzki <ole@ans.pl>
Best regards,
Krzysztof Olędzki
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2008-01-14 22:22 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-08 1:56 [PATCH 0/3] bonding: 3 fixes for 2.6.24 Jay Vosburgh
2008-01-08 1:56 ` [PATCH 1/3] bonding: fix locking in sysfs primary/active selection Jay Vosburgh
2008-01-08 1:56 ` [PATCH 2/3] bonding: fix ASSERT_RTNL that produces spurious warnings Jay Vosburgh
2008-01-08 1:57 ` [PATCH 3/3] bonding: fix locking during alb failover and slave removal Jay Vosburgh
2008-01-08 18:50 ` [PATCH 0/3] bonding: 3 fixes for 2.6.24 Krzysztof Oledzki
2008-01-08 19:17 ` Andy Gospodarek
2008-01-08 20:28 ` Jay Vosburgh
2008-01-09 6:08 ` Herbert Xu
2008-01-08 19:30 ` Jay Vosburgh
2008-01-09 6:35 ` Krzysztof Oledzki
2008-01-09 7:58 ` Jay Vosburgh
2008-01-09 9:36 ` Krzysztof Oledzki
2008-01-09 15:27 ` Andy Gospodarek
2008-01-09 17:54 ` Jay Vosburgh
2008-01-09 20:17 ` Andy Gospodarek
2008-01-09 22:05 ` Herbert Xu
2008-01-09 23:19 ` Jay Vosburgh
2008-01-10 0:58 ` Herbert Xu
2008-01-10 14:51 ` Andy Gospodarek
2008-01-10 20:36 ` Herbert Xu
2008-01-10 20:50 ` Jay Vosburgh
2008-01-10 21:03 ` Andy Gospodarek
2008-01-10 21:05 ` Herbert Xu
2008-01-11 1:06 ` Jay Vosburgh
2008-01-11 4:55 ` Herbert Xu
2008-01-10 20:45 ` Jay Vosburgh
2008-01-12 10:53 ` Krzysztof Oledzki
2008-01-12 17:56 ` Jay Vosburgh
2008-01-13 0:19 ` Herbert Xu
2008-01-14 22:15 ` Krzysztof Oledzki
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).