[PATCH 1/2] igb: fix rtnl race in PM resume path

[PATCH 1/2] igb: fix rtnl race in PM resume path

[Benjamin Poirier]

Since the caller (PM resume code) is not the one holding rtnl, when taking the
'else' branch rtnl may be released at any moment, thereby defeating the whole
purpose of this code block.

Signed-off-by: Benjamin Poirier <bpoirier@suse.de>
---
 drivers/net/ethernet/intel/igb/igb_main.c |   20 ++++++--------------
 1 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index f022ff7..4854ab6 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1083,9 +1083,12 @@ msi_only:
 		adapter->flags |= IGB_FLAG_HAS_MSI;
 out:
 	/* Notify the stack of the (possibly) reduced queue counts. */
+	rtnl_lock();
 	netif_set_real_num_tx_queues(adapter->netdev, adapter->num_tx_queues);
-	return netif_set_real_num_rx_queues(adapter->netdev,
-					    adapter->num_rx_queues);
+	err = netif_set_real_num_rx_queues(adapter->netdev,
+		adapter->num_rx_queues);
+	rtnl_unlock();
+	return err;
 }
 
 /**
@@ -6646,18 +6649,7 @@ static int igb_resume(struct device *dev)
 	pci_enable_wake(pdev, PCI_D3hot, 0);
 	pci_enable_wake(pdev, PCI_D3cold, 0);
 
-	if (!rtnl_is_locked()) {
-		/*
-		 * shut up ASSERT_RTNL() warning in
-		 * netif_set_real_num_tx/rx_queues.
-		 */
-		rtnl_lock();
-		err = igb_init_interrupt_scheme(adapter);
-		rtnl_unlock();
-	} else {
-		err = igb_init_interrupt_scheme(adapter);
-	}
-	if (err) {
+	if (igb_init_interrupt_scheme(adapter)) {
 		dev_err(&pdev->dev, "Unable to allocate memory for queues\n");
 		return -ENOMEM;
 	}
-- 
1.7.7

[PATCH 2/2] ixgbe: add missing rtnl_lock in PM resume path

[Benjamin Poirier]

Upon resume from standby, ixgbe may trigger the ASSERT_RTNL() in
netif_set_real_num_tx_queues(). The call stack is:
	netif_set_real_num_tx_queues
	ixgbe_set_num_queues
	ixgbe_init_interrupt_scheme
	ixgbe_resume

Signed-off-by: Benjamin Poirier <bpoirier@suse.de>

---
Unlike patch 1 for igb, we cannot lock rtnl closer to the point where
netif_set_real_num_tx_queues() is called because ixgbe_init_interrupt_scheme()
is also called by the dcb code, already under rtnl.
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 3e26b1f..ff6edab 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4836,7 +4836,9 @@ static int ixgbe_resume(struct pci_dev *pdev)
 
 	pci_wake_from_d3(pdev, false);
 
+	rtnl_lock();
 	err = ixgbe_init_interrupt_scheme(adapter);
+	rtnl_unlock();
 	if (err) {
 		e_dev_err("Cannot initialize interrupts for device\n");
 		return err;
-- 
1.7.7

Re: [PATCH 2/2] ixgbe: add missing rtnl_lock in PM resume path

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 789 bytes --]

On Thu, 2012-04-05 at 11:11 -0400, Benjamin Poirier wrote:
> Upon resume from standby, ixgbe may trigger the ASSERT_RTNL() in
> netif_set_real_num_tx_queues(). The call stack is:
>         netif_set_real_num_tx_queues
>         ixgbe_set_num_queues
>         ixgbe_init_interrupt_scheme
>         ixgbe_resume
> 
> Signed-off-by: Benjamin Poirier <bpoirier@suse.de>
> 
> ---
> Unlike patch 1 for igb, we cannot lock rtnl closer to the point where
> netif_set_real_num_tx_queues() is called because
> ixgbe_init_interrupt_scheme()
> is also called by the dcb code, already under rtnl.
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-) 

Thanks Benjamin, I will add the patch to my igb queue of patches.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] igb: fix rtnl race in PM resume path

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

On Thu, 2012-04-05 at 11:11 -0400, Benjamin Poirier wrote:
> Since the caller (PM resume code) is not the one holding rtnl, when
> taking the
> 'else' branch rtnl may be released at any moment, thereby defeating
> the whole
> purpose of this code block.
> 
> Signed-off-by: Benjamin Poirier <bpoirier@suse.de>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c |   20 ++++++--------------
>  1 files changed, 6 insertions(+), 14 deletions(-) 

Thanks Benjamin, I will add the patch to my igb queue of patches.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]