Skip to contentSkip to conversation

igb: Fix locking to silence ASSERT_RTNL() warning during suspend/resume

PATCH Srivatsa S. Bhat 1 participant 1 message Mar 29, 2012 2 messages not in the archive

Subsystems (3)
Networking Network drivers Stable branch
lore
Share & tools
lore

Srivatsa S. Bhat

author
#2 lore raw
In reply to a message not in the archive

[ CC'ing netdev mailing list and lkml. I have put relevant info by hand to
preserve the context, since I didn't find any archives for the stable mailing list, to give a link to. ]

Quoted 8 lines · Ben Hutchings
Quoted 7 lines · Srivatsa S. Bhat

From: Zheng Yan <[email protected]>

[ Upstream commit 749ab2cd127046df79084b6b9165b23491b1db5f ]

During suspend/resume (and possibly in other scenarios as well), the ASSERT_RTNL() warning is triggered in netif_set_real_num_tx/rx_queues() functions, something like:

[  374.073765] RTNL: assertion failed at net/core/dev.c (1718)
[  374.073767] Pid: 566, comm: kworker/u:4 Not tainted 3.2.13-0.0.0.2.0f8ab74-trace #1
[  374.073769] RTNL: assertion failed at net/core/dev.c (1718)
[  374.073771] Call Trace:
[  374.073772] Pid: 5271, comm: kworker/u:10 Not tainted 3.2.13-0.0.0.2.0f8ab74-trace #1
[  374.073775] Call Trace:
[  374.073780]  [<ffffffff81385e3e>] netif_set_real_num_tx_queues+0x1ae/0x1d0
[  374.073785]  [<ffffffff81385e3e>] netif_set_real_num_tx_queues+0x1ae/0x1d0
[  374.073792]  [<ffffffffa0323f92>] igb_set_interrupt_capability+0x152/0x1f0 [igb]
[  374.073797]  [<ffffffffa0323f92>] igb_set_interrupt_capability+0x152/0x1f0 [igb]
[  374.073814]  [<ffffffffa0328968>] igb_init_interrupt_scheme+0x28/0x330 [igb]
[  374.073819]  [<ffffffffa0328968>] igb_init_interrupt_scheme+0x28/0x330 [igb]
[  374.073823]  [<ffffffffa0329a9c>] igb_resume+0x9c/0x160 [igb]
[  374.073828]  [<ffffffffa0329a9c>] igb_resume+0x9c/0x160 [igb]
[  374.073831]  [<ffffffff812707b2>] pci_legacy_resume+0x42/0x60
[  374.073835]  [<ffffffff812707b2>] pci_legacy_resume+0x42/0x60
[  374.073837]  [<ffffffff812709c0>] pci_pm_resume+0x90/0xd0
[  374.073840]  [<ffffffff812709c0>] pci_pm_resume+0x90/0xd0
[  374.073843]  [<ffffffff8131b59c>] pm_op+0x10c/0x1e0
[  374.073847]  [<ffffffff8131b59c>] pm_op+0x10c/0x1e0
[  374.073849]  [<ffffffff8131c5ca>] device_resume+0x27a/0x2c0
[  374.073852]  [<ffffffff8131c5ca>] device_resume+0x27a/0x2c0
[  374.073854]  [<ffffffff8131c631>] async_resume+0x21/0x50
[  374.073857]  [<ffffffff8131c631>] async_resume+0x21/0x50
[  374.073860]  [<ffffffff8107e564>] async_run_entry_fn+0x84/0x180
[  374.073864]  [<ffffffff8107e564>] async_run_entry_fn+0x84/0x180
[  374.073867]  [<ffffffff8106ecb1>] process_one_work+0x171/0x350
[  374.073871]  [<ffffffff8106ecb1>] process_one_work+0x171/0x350
[  374.073874]  [<ffffffff8107e4e0>] ? async_schedule+0x20/0x20
[  374.073876]  [<ffffffff8107e4e0>] ? async_schedule+0x20/0x20
[  374.073879]  [<ffffffff81071eeb>] worker_thread+0x18b/0x430
[  374.073881]  [<ffffffff81071eeb>] worker_thread+0x18b/0x430
[  374.073884]  [<ffffffff81071d60>] ? manage_workers+0x120/0x120
[  374.073887]  [<ffffffff81071d60>] ? manage_workers+0x120/0x120
[  374.073890]  [<ffffffff8107683e>] kthread+0x9e/0xb0
[  374.073892]  [<ffffffff8107683e>] kthread+0x9e/0xb0
[  374.073896]  [<ffffffff81451e04>] kernel_thread_helper+0x4/0x10
[  374.073899]  [<ffffffff81451e04>] kernel_thread_helper+0x4/0x10
[  374.073902]  [<ffffffff81447c21>] ? retint_restore_args+0x13/0x13
[  374.073905]  [<ffffffff81447c21>] ? retint_restore_args+0x13/0x13
[  374.073907]  [<ffffffff810767a0>] ? kthread_worker_fn+0x1b0/0x1b0
[  374.073911]  [<ffffffff810767a0>] ? kthread_worker_fn+0x1b0/0x1b0
[  374.073913]  [<ffffffff81451e00>] ? gs_change+0x13/0x13
[  374.073915]  [<ffffffff81451e00>] ? gs_change+0x13/0x13
Quoted 14 lines

drivers/net/ethernet/intel/igb/igb_main.c static int igb_resume(struct pci_dev *pdev) · line 6700

Quoted 10 lines
drivers/net/ethernet/intel/igb/igb_main.c +1 -1
index 222954d..8775087 100644
@@ -6697,7 +6697,18 @@ static int igb_resume(struct pci_dev *pdev)
 	pci_enable_wake(pdev, PCI_D3hot, 0);
 	pci_enable_wake(pdev, PCI_D3cold, 0);
 
-	if (igb_init_interrupt_scheme(adapter)) {
+	if (!rtnl_is_locked()) {

This doesn't tell whether this process holds the lock. So it suppresses the warning but doesn't fix the real bug.

Quoted 4 lines

Ben.

You mean to say that this issue needs to be revisited in mainline too?

On 03/27/2012 07:57 PM, Ben Hutchings wrote:

| Oh, yes. Because you said this was a smaller patch for stable, I didn't | realise that mainline had this problem too.

Quoted 17 lines
Quoted 5 lines
drivers/net/ethernet/intel/igb/igb_main.c +4 -0
+		/*
+		 * shut up ASSERT_RTNL() warning in
+		 * netif_set_real_num_tx/rx_queues.
+		 */

drivers/net/ethernet/intel/igb/igb_main.c static int igb_resume(struct pci_dev *pdev) · line 6714

Quoted 10 lines
drivers/net/ethernet/intel/igb/igb_main.c +7 -0
+		rtnl_lock();
+		err = igb_init_interrupt_scheme(adapter);
+		rtnl_unlock();
+	} else {
+		err = igb_init_interrupt_scheme(adapter);
+	}
+	if (err) {
 		dev_err(&pdev->dev, "Unable to allocate memory for queues\n");
 		return -ENOMEM;
 	}

Regards,
Srivatsa S. Bhat