All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-wired-lan] [next PATCH 0/5] Misc Intel driver cleanups
@ 2015-10-27 23:59 Alexander Duyck
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure Alexander Duyck
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Alexander Duyck @ 2015-10-27 23:59 UTC (permalink / raw)
  To: intel-wired-lan

These are some assorted cleanups that occurred to me over the last several
days.  They are compile tested only.

The first three are based on review of some patches I saw recently accepted
into fm10k.  We weren't doing consistent exception handling when we
probably should have been.  Also it looks like there are some exception
paths what would have likely resulted in a kernel panic so I decided to try
and address those.

The fourth is based on a suggestion I had given to Nick but he ignored.
Basically e1000e_up is always returning 0, there was code that was checking
for it returning non-zero so I make e1000e_up void and dropped all code
that was checking it for a non-zero return value.

The last patch is a minor tweak to an existing patch in the dev-queue.
Feel free to just fold it into the patch it fixes and add my signed-off-by
or carry it separately.  I am good either way.

---

Alexander Duyck (5):
      fm10k: Cleanup MSI-X interrupts in case of failure
      fm10k: Cleanup exception handling for mailbox interrupt
      fm10k: Cleanup exception handling for changing queues
      e1000e: Switch e1000e_up to void, drop code checking for error result
      igb: Fix unused variable warning


 drivers/net/ethernet/intel/e1000e/e1000.h       |    2 -
 drivers/net/ethernet/intel/e1000e/netdev.c      |   15 +----
 drivers/net/ethernet/intel/fm10k/fm10k_main.c   |    4 +
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c |   17 ++++--
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |   68 ++++++++++++++++++-----
 drivers/net/ethernet/intel/igb/igb_main.c       |    1 
 6 files changed, 74 insertions(+), 33 deletions(-)

--

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure
  2015-10-27 23:59 [Intel-wired-lan] [next PATCH 0/5] Misc Intel driver cleanups Alexander Duyck
@ 2015-10-27 23:59 ` Alexander Duyck
  2015-10-28 16:11   ` Allan, Bruce W
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception handling for mailbox interrupt Alexander Duyck
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Alexander Duyck @ 2015-10-27 23:59 UTC (permalink / raw)
  To: intel-wired-lan

If the q_vector allocation fails we should free the resources associated
with the MSI-X vector table.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index b8c9f58354bf..983b1f3b5f22 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -2002,8 +2002,10 @@ int fm10k_init_queueing_scheme(struct fm10k_intfc *interface)
 
 	/* Allocate memory for queues */
 	err = fm10k_alloc_q_vectors(interface);
-	if (err)
+	if (err) {
+		fm10k_reset_msix_capability(interface);
 		return err;
+	}
 
 	/* Map rings to devices, and map devices to physical queues */
 	fm10k_assign_rings(interface);


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception handling for mailbox interrupt
  2015-10-27 23:59 [Intel-wired-lan] [next PATCH 0/5] Misc Intel driver cleanups Alexander Duyck
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure Alexander Duyck
@ 2015-10-27 23:59 ` Alexander Duyck
  2015-10-28 16:11   ` Allan, Bruce W
  2015-11-03 20:47   ` Singh, Krishneil K
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues Alexander Duyck
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 19+ messages in thread
From: Alexander Duyck @ 2015-10-27 23:59 UTC (permalink / raw)
  To: intel-wired-lan

This patch addresses two issues.

First is the fact that the fm10k_mbx_free_irq was assuming msix_entries was
valid and that will not always be the case.  As such we need to add a check
for if it is NULL.

Second is the fact that we weren't freeing the IRQ if the mailbox API
returned an error on trying to connect.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 15e67a383d27..15327d274d72 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -1131,6 +1131,10 @@ void fm10k_mbx_free_irq(struct fm10k_intfc *interface)
 	struct fm10k_hw *hw = &interface->hw;
 	int itr_reg;
 
+	/* no mailbox IRQ to free if MSI-X is not enabled */
+	if (!interface->msix_entries)
+		return;
+
 	/* disconnect the mailbox */
 	hw->mbx.ops.disconnect(hw, &hw->mbx);
 
@@ -1453,10 +1457,15 @@ int fm10k_mbx_request_irq(struct fm10k_intfc *interface)
 		err = fm10k_mbx_request_irq_pf(interface);
 	else
 		err = fm10k_mbx_request_irq_vf(interface);
+	if (err)
+		return err;
 
 	/* connect mailbox */
-	if (!err)
-		err = hw->mbx.ops.connect(hw, &hw->mbx);
+	err = hw->mbx.ops.connect(hw, &hw->mbx);
+
+	/* if the mailbox failed to connect, then free IRQ */
+	if (err)
+		fm10k_mbx_free_irq(interface);
 
 	return err;
 }


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues
  2015-10-27 23:59 [Intel-wired-lan] [next PATCH 0/5] Misc Intel driver cleanups Alexander Duyck
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure Alexander Duyck
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception handling for mailbox interrupt Alexander Duyck
@ 2015-10-27 23:59 ` Alexander Duyck
  2015-10-28 16:11   ` Allan, Bruce W
                     ` (2 more replies)
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 4/5] e1000e: Switch e1000e_up to void, drop code checking for error result Alexander Duyck
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 5/5] igb: Fix unused variable warning Alexander Duyck
  4 siblings, 3 replies; 19+ messages in thread
From: Alexander Duyck @ 2015-10-27 23:59 UTC (permalink / raw)
  To: intel-wired-lan

This patch is meant to cleanup the exception handling for the paths where
we reset the interrupts and then reconfigure them.  In all of these paths
we had very different levels of exception handling.  I have updated the
driver so that all of the paths should result in a similar state if we
fail.

Specifically the driver will now unload the mailbox interrupt, free the
queue vectors and MSI-X, and then detach the interface.

In addition for any of the PCIe related resets I have added a check with
the hw_ready function to just make sure the registers are in a readable
state prior to reopening the interface.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c |   17 +++++--
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |   55 ++++++++++++++++++-----
 2 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 38fe2d8c8388..4855686d2b4e 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device *dev, u8 tc)
 
 	err = fm10k_init_queueing_scheme(interface);
 	if (err)
-		return err;
+		goto err_queueing_scheme;
 
 	err = fm10k_mbx_request_irq(interface);
 	if (err)
-		return err;
+		goto err_mbx_irq;
 
-	if (netif_running(dev))
-		fm10k_open(dev);
+	err = netif_running(dev) ? fm10k_open(dev) : 0;
+	if (err)
+		goto err_open;
 
 	/* flag to indicate SWPRI has yet to be updated */
 	interface->flags |= FM10K_FLAG_SWPRI_CONFIG;
 
 	return 0;
+err_open:
+	fm10k_mbx_free_irq(interface);
+err_mbx_irq:
+	fm10k_clear_queueing_scheme(interface);
+err_queueing_scheme:
+	netif_device_detach(dev);
+
+	return err;
 }
 
 static int fm10k_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 15327d274d72..7b33cddfc6be 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -185,7 +185,13 @@ static void fm10k_reinit(struct fm10k_intfc *interface)
 	}
 
 	/* reassociate interrupts */
-	fm10k_mbx_request_irq(interface);
+	err = fm10k_mbx_request_irq(interface);
+	if (err)
+		goto err_mbx_irq;
+
+	err = fm10k_hw_ready(interface);
+	if (err)
+		goto err_open;
 
 	/* update hardware address for VFs if perm_addr has changed */
 	if (hw->mac.type == fm10k_mac_vf) {
@@ -205,14 +211,23 @@ static void fm10k_reinit(struct fm10k_intfc *interface)
 	/* reset clock */
 	fm10k_ts_reset(interface);
 
-	if (netif_running(netdev))
-		fm10k_open(netdev);
-
+	err = netif_running(netdev) ? fm10k_open(netdev) : 0;
+	if (err)
+		goto err_open;
+		
 	fm10k_iov_resume(interface->pdev);
 
+	rtnl_unlock();
+
+	clear_bit(__FM10K_RESETTING, &interface->state);
+
+	return;
+err_open:
+	fm10k_mbx_free_irq(interface);
+err_mbx_irq:
+	fm10k_clear_queueing_scheme(interface);
 reinit_err:
-	if (err)
-		netif_device_detach(netdev);
+	netif_device_detach(netdev);
 
 	rtnl_unlock();
 
@@ -2130,16 +2145,22 @@ static int fm10k_resume(struct pci_dev *pdev)
 	rtnl_lock();
 
 	err = fm10k_init_queueing_scheme(interface);
-	if (!err) {
-		fm10k_mbx_request_irq(interface);
-		if (netif_running(netdev))
-			err = fm10k_open(netdev);
-	}
+	if (err)
+		goto err_queueing_scheme;
 
-	rtnl_unlock();
+	err = fm10k_mbx_request_irq(interface);
+	if (err)
+		goto err_mbx_irq;
 
+	err = fm10k_hw_ready(interface);
 	if (err)
-		return err;
+		goto err_open;
+
+	err = netif_running(netdev) ? fm10k_open(netdev) : 0;
+	if (err)
+		goto err_open;
+		
+	rtnl_unlock();
 
 	/* assume host is not ready, to prevent race with watchdog in case we
 	 * actually don't have connection to the switch
@@ -2157,6 +2178,14 @@ static int fm10k_resume(struct pci_dev *pdev)
 	netif_device_attach(netdev);
 
 	return 0;
+err_open:
+	fm10k_mbx_free_irq(interface);
+err_mbx_irq:
+	fm10k_clear_queueing_scheme(interface);
+err_queueing_scheme:
+	rtnl_unlock();
+
+	return err;
 }
 
 /**


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 4/5] e1000e: Switch e1000e_up to void, drop code checking for error result
  2015-10-27 23:59 [Intel-wired-lan] [next PATCH 0/5] Misc Intel driver cleanups Alexander Duyck
                   ` (2 preceding siblings ...)
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues Alexander Duyck
@ 2015-10-27 23:59 ` Alexander Duyck
  2015-11-20  4:48   ` Brown, Aaron F
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 5/5] igb: Fix unused variable warning Alexander Duyck
  4 siblings, 1 reply; 19+ messages in thread
From: Alexander Duyck @ 2015-10-27 23:59 UTC (permalink / raw)
  To: intel-wired-lan

The function e1000e_up always returns 0.  As such we can convert it to a
void and just ignore the results.  This allows us to drop some code in a
couple spots as we no longer need to worry about non-zero return values.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/e1000e/e1000.h  |    2 +-
 drivers/net/ethernet/intel/e1000e/netdev.c |   15 ++++-----------
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 0b748d1959d9..1dc293bad87b 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -480,7 +480,7 @@ extern const char e1000e_driver_version[];
 void e1000e_check_options(struct e1000_adapter *adapter);
 void e1000e_set_ethtool_ops(struct net_device *netdev);
 
-int e1000e_up(struct e1000_adapter *adapter);
+void e1000e_up(struct e1000_adapter *adapter);
 void e1000e_down(struct e1000_adapter *adapter, bool reset);
 void e1000e_reinit_locked(struct e1000_adapter *adapter);
 void e1000e_reset(struct e1000_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 7a10bb0a83f4..4ed69fee8549 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4143,7 +4143,7 @@ void e1000e_reset(struct e1000_adapter *adapter)
 
 }
 
-int e1000e_up(struct e1000_adapter *adapter)
+void e1000e_up(struct e1000_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
 
@@ -4163,8 +4163,6 @@ int e1000e_up(struct e1000_adapter *adapter)
 		ew32(ICS, E1000_ICS_LSC | E1000_ICR_OTHER);
 	else
 		ew32(ICS, E1000_ICS_LSC);
-
-	return 0;
 }
 
 static void e1000e_flush_descriptors(struct e1000_adapter *adapter)
@@ -6643,7 +6641,7 @@ static int e1000e_pm_runtime_resume(struct device *dev)
 		return rc;
 
 	if (netdev->flags & IFF_UP)
-		rc = e1000e_up(adapter);
+		e1000e_up(adapter);
 
 	return rc;
 }
@@ -6834,13 +6832,8 @@ static void e1000_io_resume(struct pci_dev *pdev)
 
 	e1000_init_manageability_pt(adapter);
 
-	if (netif_running(netdev)) {
-		if (e1000e_up(adapter)) {
-			dev_err(&pdev->dev,
-				"can't bring device back up after reset\n");
-			return;
-		}
-	}
+	if (netif_running(netdev))
+		e1000e_up(adapter);
 
 	netif_device_attach(netdev);
 


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 5/5] igb: Fix unused variable warning
  2015-10-27 23:59 [Intel-wired-lan] [next PATCH 0/5] Misc Intel driver cleanups Alexander Duyck
                   ` (3 preceding siblings ...)
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 4/5] e1000e: Switch e1000e_up to void, drop code checking for error result Alexander Duyck
@ 2015-10-27 23:59 ` Alexander Duyck
  2015-11-20  4:50   ` Brown, Aaron F
  4 siblings, 1 reply; 19+ messages in thread
From: Alexander Duyck @ 2015-10-27 23:59 UTC (permalink / raw)
  To: intel-wired-lan

I ran into an unused variable warning when building dev-queue.  It turns
out that hw isn't used anymore when setting up the q_vectors since we
switch from hw->hw_addr to adapter->io_addr in igb_request_irq.  As such
the variable can be dropped.

Fixes: 0b3659ca297fc ("igb: improve handling of disconnected adapters")
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 1dcf3ab5fe87..3d4b502b1ecf 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -946,7 +946,6 @@ static void igb_configure_msix(struct igb_adapter *adapter)
 static int igb_request_msix(struct igb_adapter *adapter)
 {
 	struct net_device *netdev = adapter->netdev;
-	struct e1000_hw *hw = &adapter->hw;
 	int i, err = 0, vector = 0, free_vector = 0;
 
 	err = request_irq(adapter->msix_entries[vector].vector,


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure Alexander Duyck
@ 2015-10-28 16:11   ` Allan, Bruce W
  2015-11-03 20:45     ` Singh, Krishneil K
  0 siblings, 1 reply; 19+ messages in thread
From: Allan, Bruce W @ 2015-10-28 16:11 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Tuesday, October 27, 2015 4:59 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts
> in case of failure
> 
> If the q_vector allocation fails we should free the resources associated
> with the MSI-X vector table.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/fm10k/fm10k_main.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Bruce Allan <bruce.w.allan@intel.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception handling for mailbox interrupt
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception handling for mailbox interrupt Alexander Duyck
@ 2015-10-28 16:11   ` Allan, Bruce W
  2015-11-03 20:47   ` Singh, Krishneil K
  1 sibling, 0 replies; 19+ messages in thread
From: Allan, Bruce W @ 2015-10-28 16:11 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Tuesday, October 27, 2015 4:59 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception
> handling for mailbox interrupt
> 
> This patch addresses two issues.
> 
> First is the fact that the fm10k_mbx_free_irq was assuming msix_entries
> was
> valid and that will not always be the case.  As such we need to add a check
> for if it is NULL.
> 
> Second is the fact that we weren't freeing the IRQ if the mailbox API
> returned an error on trying to connect.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/fm10k/fm10k_pci.c |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)

Reviewed-by: Bruce Allan <bruce.w.allan@intel.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues Alexander Duyck
@ 2015-10-28 16:11   ` Allan, Bruce W
  2015-10-29 19:12   ` Allan, Bruce W
  2015-11-03 20:48   ` Singh, Krishneil K
  2 siblings, 0 replies; 19+ messages in thread
From: Allan, Bruce W @ 2015-10-28 16:11 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Tuesday, October 27, 2015 4:59 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception
> handling for changing queues
> 
> This patch is meant to cleanup the exception handling for the paths where
> we reset the interrupts and then reconfigure them.  In all of these paths
> we had very different levels of exception handling.  I have updated the
> driver so that all of the paths should result in a similar state if we
> fail.
> 
> Specifically the driver will now unload the mailbox interrupt, free the
> queue vectors and MSI-X, and then detach the interface.
> 
> In addition for any of the PCIe related resets I have added a check with
> the hw_ready function to just make sure the registers are in a readable
> state prior to reopening the interface.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/fm10k/fm10k_netdev.c |   17 +++++--
>  drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |   55
> ++++++++++++++++++-----
>  2 files changed, 55 insertions(+), 17 deletions(-)

Reviewed-by: Bruce Allan <bruce.w.allan@intel.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues Alexander Duyck
  2015-10-28 16:11   ` Allan, Bruce W
@ 2015-10-29 19:12   ` Allan, Bruce W
  2015-10-29 19:39     ` Alexander Duyck
  2015-11-03 20:48   ` Singh, Krishneil K
  2 siblings, 1 reply; 19+ messages in thread
From: Allan, Bruce W @ 2015-10-29 19:12 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Tuesday, October 27, 2015 4:59 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception
> handling for changing queues
> 
> This patch is meant to cleanup the exception handling for the paths where
> we reset the interrupts and then reconfigure them.  In all of these paths
> we had very different levels of exception handling.  I have updated the
> driver so that all of the paths should result in a similar state if we
> fail.
> 
> Specifically the driver will now unload the mailbox interrupt, free the
> queue vectors and MSI-X, and then detach the interface.
> 
> In addition for any of the PCIe related resets I have added a check with
> the hw_ready function to just make sure the registers are in a readable
> state prior to reopening the interface.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/fm10k/fm10k_netdev.c |   17 +++++--
>  drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |   55
> ++++++++++++++++++-----
>  2 files changed, 55 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> index 38fe2d8c8388..4855686d2b4e 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> @@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device *dev, u8
> tc)
> 
>  	err = fm10k_init_queueing_scheme(interface);
>  	if (err)
> -		return err;
> +		goto err_queueing_scheme;
> 
>  	err = fm10k_mbx_request_irq(interface);
>  	if (err)
> -		return err;
> +		goto err_mbx_irq;
> 
> -	if (netif_running(dev))
> -		fm10k_open(dev);
> +	err = netif_running(dev) ? fm10k_open(dev) : 0;
> +	if (err)
> +		goto err_open;
> 
>  	/* flag to indicate SWPRI has yet to be updated */
>  	interface->flags |= FM10K_FLAG_SWPRI_CONFIG;
> 
>  	return 0;
> +err_open:
> +	fm10k_mbx_free_irq(interface);
> +err_mbx_irq:
> +	fm10k_clear_queueing_scheme(interface);
> +err_queueing_scheme:
> +	netif_device_detach(dev);
> +
> +	return err;
>  }
> 
>  static int fm10k_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> index 15327d274d72..7b33cddfc6be 100644
> --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> @@ -185,7 +185,13 @@ static void fm10k_reinit(struct fm10k_intfc
> *interface)
>  	}
> 
>  	/* reassociate interrupts */
> -	fm10k_mbx_request_irq(interface);
> +	err = fm10k_mbx_request_irq(interface);
> +	if (err)
> +		goto err_mbx_irq;
> +
> +	err = fm10k_hw_ready(interface);
> +	if (err)
> +		goto err_open;
> 
>  	/* update hardware address for VFs if perm_addr has changed */
>  	if (hw->mac.type == fm10k_mac_vf) {
> @@ -205,14 +211,23 @@ static void fm10k_reinit(struct fm10k_intfc
> *interface)
>  	/* reset clock */
>  	fm10k_ts_reset(interface);
> 
> -	if (netif_running(netdev))
> -		fm10k_open(netdev);
> -
> +	err = netif_running(netdev) ? fm10k_open(netdev) : 0;
> +	if (err)
> +		goto err_open;
> +

Jeff, the above blank-line separator line in Alex' original submission looks fine here but
in your next-queue tree dev-queue branch there is additional whitespace (2 tabs).
How did that happen?

>  	fm10k_iov_resume(interface->pdev);
> 
> +	rtnl_unlock();
> +
> +	clear_bit(__FM10K_RESETTING, &interface->state);
> +
> +	return;
> +err_open:
> +	fm10k_mbx_free_irq(interface);
> +err_mbx_irq:
> +	fm10k_clear_queueing_scheme(interface);
>  reinit_err:
> -	if (err)
> -		netif_device_detach(netdev);
> +	netif_device_detach(netdev);
> 
>  	rtnl_unlock();
> 
> @@ -2130,16 +2145,22 @@ static int fm10k_resume(struct pci_dev *pdev)
>  	rtnl_lock();
> 
>  	err = fm10k_init_queueing_scheme(interface);
> -	if (!err) {
> -		fm10k_mbx_request_irq(interface);
> -		if (netif_running(netdev))
> -			err = fm10k_open(netdev);
> -	}
> +	if (err)
> +		goto err_queueing_scheme;
> 
> -	rtnl_unlock();
> +	err = fm10k_mbx_request_irq(interface);
> +	if (err)
> +		goto err_mbx_irq;
> 
> +	err = fm10k_hw_ready(interface);
>  	if (err)
> -		return err;
> +		goto err_open;
> +
> +	err = netif_running(netdev) ? fm10k_open(netdev) : 0;
> +	if (err)
> +		goto err_open;
> +

Ditto here

> +	rtnl_unlock();
> 
>  	/* assume host is not ready, to prevent race with watchdog in case
> we
>  	 * actually don't have connection to the switch
> @@ -2157,6 +2178,14 @@ static int fm10k_resume(struct pci_dev *pdev)
>  	netif_device_attach(netdev);
> 
>  	return 0;
> +err_open:
> +	fm10k_mbx_free_irq(interface);
> +err_mbx_irq:
> +	fm10k_clear_queueing_scheme(interface);
> +err_queueing_scheme:
> +	rtnl_unlock();
> +
> +	return err;
>  }
> 
>  /**
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at lists.osuosl.org
> http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues
  2015-10-29 19:12   ` Allan, Bruce W
@ 2015-10-29 19:39     ` Alexander Duyck
  2015-11-03 23:20       ` Allan, Bruce W
  2015-11-10 17:30       ` Jeff Kirsher
  0 siblings, 2 replies; 19+ messages in thread
From: Alexander Duyck @ 2015-10-29 19:39 UTC (permalink / raw)
  To: intel-wired-lan

On 10/29/2015 12:12 PM, Allan, Bruce W wrote:
>> -----Original Message-----
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
>> Behalf Of Alexander Duyck
>> Sent: Tuesday, October 27, 2015 4:59 PM
>> To: intel-wired-lan at lists.osuosl.org
>> Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception
>> handling for changing queues
>>
>> This patch is meant to cleanup the exception handling for the paths where
>> we reset the interrupts and then reconfigure them.  In all of these paths
>> we had very different levels of exception handling.  I have updated the
>> driver so that all of the paths should result in a similar state if we
>> fail.
>>
>> Specifically the driver will now unload the mailbox interrupt, free the
>> queue vectors and MSI-X, and then detach the interface.
>>
>> In addition for any of the PCIe related resets I have added a check with
>> the hw_ready function to just make sure the registers are in a readable
>> state prior to reopening the interface.
>>
>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>> ---
>>   drivers/net/ethernet/intel/fm10k/fm10k_netdev.c |   17 +++++--
>>   drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |   55
>> ++++++++++++++++++-----
>>   2 files changed, 55 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>> b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>> index 38fe2d8c8388..4855686d2b4e 100644
>> --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
>> @@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device *dev, u8
>> tc)
>>
>>   	err = fm10k_init_queueing_scheme(interface);
>>   	if (err)
>> -		return err;
>> +		goto err_queueing_scheme;
>>
>>   	err = fm10k_mbx_request_irq(interface);
>>   	if (err)
>> -		return err;
>> +		goto err_mbx_irq;
>>
>> -	if (netif_running(dev))
>> -		fm10k_open(dev);
>> +	err = netif_running(dev) ? fm10k_open(dev) : 0;
>> +	if (err)
>> +		goto err_open;
>>
>>   	/* flag to indicate SWPRI has yet to be updated */
>>   	interface->flags |= FM10K_FLAG_SWPRI_CONFIG;
>>
>>   	return 0;
>> +err_open:
>> +	fm10k_mbx_free_irq(interface);
>> +err_mbx_irq:
>> +	fm10k_clear_queueing_scheme(interface);
>> +err_queueing_scheme:
>> +	netif_device_detach(dev);
>> +
>> +	return err;
>>   }
>>
>>   static int fm10k_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
>> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>> b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>> index 15327d274d72..7b33cddfc6be 100644
>> --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
>> @@ -185,7 +185,13 @@ static void fm10k_reinit(struct fm10k_intfc
>> *interface)
>>   	}
>>
>>   	/* reassociate interrupts */
>> -	fm10k_mbx_request_irq(interface);
>> +	err = fm10k_mbx_request_irq(interface);
>> +	if (err)
>> +		goto err_mbx_irq;
>> +
>> +	err = fm10k_hw_ready(interface);
>> +	if (err)
>> +		goto err_open;
>>
>>   	/* update hardware address for VFs if perm_addr has changed */
>>   	if (hw->mac.type == fm10k_mac_vf) {
>> @@ -205,14 +211,23 @@ static void fm10k_reinit(struct fm10k_intfc
>> *interface)
>>   	/* reset clock */
>>   	fm10k_ts_reset(interface);
>>
>> -	if (netif_running(netdev))
>> -		fm10k_open(netdev);
>> -
>> +	err = netif_running(netdev) ? fm10k_open(netdev) : 0;
>> +	if (err)
>> +		goto err_open;
>> +
> Jeff, the above blank-line separator line in Alex' original submission looks fine here but
> in your next-queue tree dev-queue branch there is additional whitespace (2 tabs).
> How did that happen?

I just went back and checked and it looks like I had included some 
trailing white space in my original patch.  Jeff do you need me to 
resubmit this patch or can you clean up the two spots where it added 
trailing white space in the blank lines?

- Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure
  2015-10-28 16:11   ` Allan, Bruce W
@ 2015-11-03 20:45     ` Singh, Krishneil K
  0 siblings, 0 replies; 19+ messages in thread
From: Singh, Krishneil K @ 2015-11-03 20:45 UTC (permalink / raw)
  To: intel-wired-lan



-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Allan, Bruce W
Sent: Wednesday, October 28, 2015 9:11 AM
To: Alexander Duyck <aduyck@mirantis.com>; intel-wired-lan at lists.osuosl.org
Subject: Re: [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure

> -----Original Message-----
> From: Intel-wired-lan 
> [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of 
> Alexander Duyck
> Sent: Tuesday, October 27, 2015 4:59 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X 
> interrupts in case of failure
> 
> If the q_vector allocation fails we should free the resources 
> associated with the MSI-X vector table.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/fm10k/fm10k_main.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception handling for mailbox interrupt
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception handling for mailbox interrupt Alexander Duyck
  2015-10-28 16:11   ` Allan, Bruce W
@ 2015-11-03 20:47   ` Singh, Krishneil K
  1 sibling, 0 replies; 19+ messages in thread
From: Singh, Krishneil K @ 2015-11-03 20:47 UTC (permalink / raw)
  To: intel-wired-lan


-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: intel-wired-lan@lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception handling for mailbox interrupt

This patch addresses two issues.

First is the fact that the fm10k_mbx_free_irq was assuming msix_entries was valid and that will not always be the case.  As such we need to add a check for if it is NULL.

Second is the fact that we weren't freeing the IRQ if the mailbox API returned an error on trying to connect.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues Alexander Duyck
  2015-10-28 16:11   ` Allan, Bruce W
  2015-10-29 19:12   ` Allan, Bruce W
@ 2015-11-03 20:48   ` Singh, Krishneil K
  2 siblings, 0 replies; 19+ messages in thread
From: Singh, Krishneil K @ 2015-11-03 20:48 UTC (permalink / raw)
  To: intel-wired-lan



-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, October 27, 2015 4:59 PM
To: intel-wired-lan@lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues

This patch is meant to cleanup the exception handling for the paths where we reset the interrupts and then reconfigure them.  In all of these paths we had very different levels of exception handling.  I have updated the driver so that all of the paths should result in a similar state if we fail.

Specifically the driver will now unload the mailbox interrupt, free the queue vectors and MSI-X, and then detach the interface.

In addition for any of the PCIe related resets I have added a check with the hw_ready function to just make sure the registers are in a readable state prior to reopening the interface.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---

Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues
  2015-10-29 19:39     ` Alexander Duyck
@ 2015-11-03 23:20       ` Allan, Bruce W
  2015-11-10 16:46         ` Allan, Bruce W
  2015-11-10 17:30       ` Jeff Kirsher
  1 sibling, 1 reply; 19+ messages in thread
From: Allan, Bruce W @ 2015-11-03 23:20 UTC (permalink / raw)
  To: intel-wired-lan

> -----Original Message-----
> From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
> Sent: Thursday, October 29, 2015 12:39 PM
> To: Allan, Bruce W; Kirsher, Jeffrey T; Alexander Duyck; intel-wired-
> lan at lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception
> handling for changing queues
> 
> On 10/29/2015 12:12 PM, Allan, Bruce W wrote:
> >> -----Original Message-----
> >> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org]
> On
> >> Behalf Of Alexander Duyck
> >> Sent: Tuesday, October 27, 2015 4:59 PM
> >> To: intel-wired-lan at lists.osuosl.org
> >> Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception
> >> handling for changing queues
> >>
> >> This patch is meant to cleanup the exception handling for the paths
> where
> >> we reset the interrupts and then reconfigure them.  In all of these paths
> >> we had very different levels of exception handling.  I have updated the
> >> driver so that all of the paths should result in a similar state if we
> >> fail.
> >>
> >> Specifically the driver will now unload the mailbox interrupt, free the
> >> queue vectors and MSI-X, and then detach the interface.
> >>
> >> In addition for any of the PCIe related resets I have added a check with
> >> the hw_ready function to just make sure the registers are in a readable
> >> state prior to reopening the interface.
> >>
> >> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> >> ---
> >>   drivers/net/ethernet/intel/fm10k/fm10k_netdev.c |   17 +++++--
> >>   drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |   55
> >> ++++++++++++++++++-----
> >>   2 files changed, 55 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> >> b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> >> index 38fe2d8c8388..4855686d2b4e 100644
> >> --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> >> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> >> @@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device *dev,
> u8
> >> tc)
> >>
> >>   	err = fm10k_init_queueing_scheme(interface);
> >>   	if (err)
> >> -		return err;
> >> +		goto err_queueing_scheme;
> >>
> >>   	err = fm10k_mbx_request_irq(interface);
> >>   	if (err)
> >> -		return err;
> >> +		goto err_mbx_irq;
> >>
> >> -	if (netif_running(dev))
> >> -		fm10k_open(dev);
> >> +	err = netif_running(dev) ? fm10k_open(dev) : 0;
> >> +	if (err)
> >> +		goto err_open;
> >>
> >>   	/* flag to indicate SWPRI has yet to be updated */
> >>   	interface->flags |= FM10K_FLAG_SWPRI_CONFIG;
> >>
> >>   	return 0;
> >> +err_open:
> >> +	fm10k_mbx_free_irq(interface);
> >> +err_mbx_irq:
> >> +	fm10k_clear_queueing_scheme(interface);
> >> +err_queueing_scheme:
> >> +	netif_device_detach(dev);
> >> +
> >> +	return err;
> >>   }
> >>
> >>   static int fm10k_ioctl(struct net_device *netdev, struct ifreq *ifr, int
> cmd)
> >> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> >> b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> >> index 15327d274d72..7b33cddfc6be 100644
> >> --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> >> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> >> @@ -185,7 +185,13 @@ static void fm10k_reinit(struct fm10k_intfc
> >> *interface)
> >>   	}
> >>
> >>   	/* reassociate interrupts */
> >> -	fm10k_mbx_request_irq(interface);
> >> +	err = fm10k_mbx_request_irq(interface);
> >> +	if (err)
> >> +		goto err_mbx_irq;
> >> +
> >> +	err = fm10k_hw_ready(interface);
> >> +	if (err)
> >> +		goto err_open;
> >>
> >>   	/* update hardware address for VFs if perm_addr has changed */
> >>   	if (hw->mac.type == fm10k_mac_vf) {
> >> @@ -205,14 +211,23 @@ static void fm10k_reinit(struct fm10k_intfc
> >> *interface)
> >>   	/* reset clock */
> >>   	fm10k_ts_reset(interface);
> >>
> >> -	if (netif_running(netdev))
> >> -		fm10k_open(netdev);
> >> -
> >> +	err = netif_running(netdev) ? fm10k_open(netdev) : 0;
> >> +	if (err)
> >> +		goto err_open;
> >> +
> > Jeff, the above blank-line separator line in Alex' original submission looks
> fine here but
> > in your next-queue tree dev-queue branch there is additional whitespace
> (2 tabs).
> > How did that happen?
> 
> I just went back and checked and it looks like I had included some
> trailing white space in my original patch.  Jeff do you need me to
> resubmit this patch or can you clean up the two spots where it added
> trailing white space in the blank lines?
> 
> - Alex

Jeff, are you going to fix the whitespace issues in this patch?

Thanks,
Bruce.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues
  2015-11-03 23:20       ` Allan, Bruce W
@ 2015-11-10 16:46         ` Allan, Bruce W
  0 siblings, 0 replies; 19+ messages in thread
From: Allan, Bruce W @ 2015-11-10 16:46 UTC (permalink / raw)
  To: intel-wired-lan

Jeff?  Ping!

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Allan, Bruce W
> Sent: Tuesday, November 03, 2015 3:20 PM
> To: Alexander Duyck; Kirsher, Jeffrey T; Alexander Duyck; intel-wired-
> lan at lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception
> handling for changing queues
> 
> > -----Original Message-----
> > From: Alexander Duyck [mailto:alexander.duyck at gmail.com]
> > Sent: Thursday, October 29, 2015 12:39 PM
> > To: Allan, Bruce W; Kirsher, Jeffrey T; Alexander Duyck; intel-wired-
> > lan at lists.osuosl.org
> > Subject: Re: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception
> > handling for changing queues
> >
> > On 10/29/2015 12:12 PM, Allan, Bruce W wrote:
> > >> -----Original Message-----
> > >> From: Intel-wired-lan [mailto:intel-wired-lan-
> bounces at lists.osuosl.org]
> > On
> > >> Behalf Of Alexander Duyck
> > >> Sent: Tuesday, October 27, 2015 4:59 PM
> > >> To: intel-wired-lan at lists.osuosl.org
> > >> Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception
> > >> handling for changing queues
> > >>
> > >> This patch is meant to cleanup the exception handling for the paths
> > where
> > >> we reset the interrupts and then reconfigure them.  In all of these
> paths
> > >> we had very different levels of exception handling.  I have updated the
> > >> driver so that all of the paths should result in a similar state if we
> > >> fail.
> > >>
> > >> Specifically the driver will now unload the mailbox interrupt, free the
> > >> queue vectors and MSI-X, and then detach the interface.
> > >>
> > >> In addition for any of the PCIe related resets I have added a check with
> > >> the hw_ready function to just make sure the registers are in a readable
> > >> state prior to reopening the interface.
> > >>
> > >> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> > >> ---
> > >>   drivers/net/ethernet/intel/fm10k/fm10k_netdev.c |   17 +++++--
> > >>   drivers/net/ethernet/intel/fm10k/fm10k_pci.c    |   55
> > >> ++++++++++++++++++-----
> > >>   2 files changed, 55 insertions(+), 17 deletions(-)
> > >>
> > >> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > >> b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > >> index 38fe2d8c8388..4855686d2b4e 100644
> > >> --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > >> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> > >> @@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device
> *dev,
> > u8
> > >> tc)
> > >>
> > >>   	err = fm10k_init_queueing_scheme(interface);
> > >>   	if (err)
> > >> -		return err;
> > >> +		goto err_queueing_scheme;
> > >>
> > >>   	err = fm10k_mbx_request_irq(interface);
> > >>   	if (err)
> > >> -		return err;
> > >> +		goto err_mbx_irq;
> > >>
> > >> -	if (netif_running(dev))
> > >> -		fm10k_open(dev);
> > >> +	err = netif_running(dev) ? fm10k_open(dev) : 0;
> > >> +	if (err)
> > >> +		goto err_open;
> > >>
> > >>   	/* flag to indicate SWPRI has yet to be updated */
> > >>   	interface->flags |= FM10K_FLAG_SWPRI_CONFIG;
> > >>
> > >>   	return 0;
> > >> +err_open:
> > >> +	fm10k_mbx_free_irq(interface);
> > >> +err_mbx_irq:
> > >> +	fm10k_clear_queueing_scheme(interface);
> > >> +err_queueing_scheme:
> > >> +	netif_device_detach(dev);
> > >> +
> > >> +	return err;
> > >>   }
> > >>
> > >>   static int fm10k_ioctl(struct net_device *netdev, struct ifreq *ifr, int
> > cmd)
> > >> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > >> b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > >> index 15327d274d72..7b33cddfc6be 100644
> > >> --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > >> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> > >> @@ -185,7 +185,13 @@ static void fm10k_reinit(struct fm10k_intfc
> > >> *interface)
> > >>   	}
> > >>
> > >>   	/* reassociate interrupts */
> > >> -	fm10k_mbx_request_irq(interface);
> > >> +	err = fm10k_mbx_request_irq(interface);
> > >> +	if (err)
> > >> +		goto err_mbx_irq;
> > >> +
> > >> +	err = fm10k_hw_ready(interface);
> > >> +	if (err)
> > >> +		goto err_open;
> > >>
> > >>   	/* update hardware address for VFs if perm_addr has changed */
> > >>   	if (hw->mac.type == fm10k_mac_vf) {
> > >> @@ -205,14 +211,23 @@ static void fm10k_reinit(struct fm10k_intfc
> > >> *interface)
> > >>   	/* reset clock */
> > >>   	fm10k_ts_reset(interface);
> > >>
> > >> -	if (netif_running(netdev))
> > >> -		fm10k_open(netdev);
> > >> -
> > >> +	err = netif_running(netdev) ? fm10k_open(netdev) : 0;
> > >> +	if (err)
> > >> +		goto err_open;
> > >> +
> > > Jeff, the above blank-line separator line in Alex' original submission looks
> > fine here but
> > > in your next-queue tree dev-queue branch there is additional
> whitespace
> > (2 tabs).
> > > How did that happen?
> >
> > I just went back and checked and it looks like I had included some
> > trailing white space in my original patch.  Jeff do you need me to
> > resubmit this patch or can you clean up the two spots where it added
> > trailing white space in the blank lines?
> >
> > - Alex
> 
> Jeff, are you going to fix the whitespace issues in this patch?
> 
> Thanks,
> Bruce.
> 
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at lists.osuosl.org
> http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues
  2015-10-29 19:39     ` Alexander Duyck
  2015-11-03 23:20       ` Allan, Bruce W
@ 2015-11-10 17:30       ` Jeff Kirsher
  1 sibling, 0 replies; 19+ messages in thread
From: Jeff Kirsher @ 2015-11-10 17:30 UTC (permalink / raw)
  To: intel-wired-lan

On Thu, 2015-10-29 at 12:39 -0700, Alexander Duyck wrote:
> On 10/29/2015 12:12 PM, Allan, Bruce W wrote:
> >> -----Original Message-----
> >> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl
> .org] On
> >> Behalf Of Alexander Duyck
> >> Sent: Tuesday, October 27, 2015 4:59 PM
> >> To: intel-wired-lan at lists.osuosl.org
> >> Subject: [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup
> exception
> >> handling for changing queues
> >>
> >> This patch is meant to cleanup the exception handling for the
> paths where
> >> we reset the interrupts and then reconfigure them.? In all of
> these paths
> >> we had very different levels of exception handling.? I have
> updated the
> >> driver so that all of the paths should result in a similar state
> if we
> >> fail.
> >>
> >> Specifically the driver will now unload the mailbox interrupt,
> free the
> >> queue vectors and MSI-X, and then detach the interface.
> >>
> >> In addition for any of the PCIe related resets I have added a
> check with
> >> the hw_ready function to just make sure the registers are in a
> readable
> >> state prior to reopening the interface.
> >>
> >> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> >> ---
> >>?? drivers/net/ethernet/intel/fm10k/fm10k_netdev.c |?? 17 +++++--
> >>?? drivers/net/ethernet/intel/fm10k/fm10k_pci.c??? |?? 55
> >> ++++++++++++++++++-----
> >>?? 2 files changed, 55 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> >> b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> >> index 38fe2d8c8388..4855686d2b4e 100644
> >> --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> >> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
> >> @@ -1176,19 +1176,28 @@ int fm10k_setup_tc(struct net_device *dev,
> u8
> >> tc)
> >>
> >>??????err = fm10k_init_queueing_scheme(interface);
> >>??????if (err)
> >> -????????????return err;
> >> +????????????goto err_queueing_scheme;
> >>
> >>??????err = fm10k_mbx_request_irq(interface);
> >>??????if (err)
> >> -????????????return err;
> >> +????????????goto err_mbx_irq;
> >>
> >> -????if (netif_running(dev))
> >> -????????????fm10k_open(dev);
> >> +????err = netif_running(dev) ? fm10k_open(dev) : 0;
> >> +????if (err)
> >> +????????????goto err_open;
> >>
> >>??????/* flag to indicate SWPRI has yet to be updated */
> >>??????interface->flags |= FM10K_FLAG_SWPRI_CONFIG;
> >>
> >>??????return 0;
> >> +err_open:
> >> +????fm10k_mbx_free_irq(interface);
> >> +err_mbx_irq:
> >> +????fm10k_clear_queueing_scheme(interface);
> >> +err_queueing_scheme:
> >> +????netif_device_detach(dev);
> >> +
> >> +????return err;
> >>?? }
> >>
> >>?? static int fm10k_ioctl(struct net_device *netdev, struct ifreq
> *ifr, int cmd)
> >> diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> >> b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> >> index 15327d274d72..7b33cddfc6be 100644
> >> --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> >> +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
> >> @@ -185,7 +185,13 @@ static void fm10k_reinit(struct fm10k_intfc
> >> *interface)
> >>??????}
> >>
> >>??????/* reassociate interrupts */
> >> -????fm10k_mbx_request_irq(interface);
> >> +????err = fm10k_mbx_request_irq(interface);
> >> +????if (err)
> >> +????????????goto err_mbx_irq;
> >> +
> >> +????err = fm10k_hw_ready(interface);
> >> +????if (err)
> >> +????????????goto err_open;
> >>
> >>??????/* update hardware address for VFs if perm_addr has changed
> */
> >>??????if (hw->mac.type == fm10k_mac_vf) {
> >> @@ -205,14 +211,23 @@ static void fm10k_reinit(struct fm10k_intfc
> >> *interface)
> >>??????/* reset clock */
> >>??????fm10k_ts_reset(interface);
> >>
> >> -????if (netif_running(netdev))
> >> -????????????fm10k_open(netdev);
> >> -
> >> +????err = netif_running(netdev) ? fm10k_open(netdev) : 0;
> >> +????if (err)
> >> +????????????goto err_open;
> >> +
> > Jeff, the above blank-line separator line in Alex' original
> submission looks fine here but
> > in your next-queue tree dev-queue branch there is additional
> whitespace (2 tabs).
> > How did that happen?
> 
> I just went back and checked and it looks like I had included some 
> trailing white space in my original patch.? Jeff do you need me to 
> resubmit this patch or can you clean up the two spots where it added 
> trailing white space in the blank lines?

I can fix it. ?I saw it when I applied it, and purposely did not fix it
to see if validation would catch it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20151110/6cd93d05/attachment.asc>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 4/5] e1000e: Switch e1000e_up to void, drop code checking for error result
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 4/5] e1000e: Switch e1000e_up to void, drop code checking for error result Alexander Duyck
@ 2015-11-20  4:48   ` Brown, Aaron F
  0 siblings, 0 replies; 19+ messages in thread
From: Brown, Aaron F @ 2015-11-20  4:48 UTC (permalink / raw)
  To: intel-wired-lan

> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Tuesday, October 27, 2015 5:00 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH 4/5] e1000e: Switch e1000e_up to
> void, drop code checking for error result
> 
> The function e1000e_up always returns 0.  As such we can convert it to a
> void and just ignore the results.  This allows us to drop some code in a
> couple spots as we no longer need to worry about non-zero return values.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/e1000e/e1000.h  |    2 +-
>  drivers/net/ethernet/intel/e1000e/netdev.c |   15 ++++-----------
>  2 files changed, 5 insertions(+), 12 deletions(-)
> 

Tested-by: Aaron Brown <aaron.f.brown@intel.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Intel-wired-lan] [next PATCH 5/5] igb: Fix unused variable warning
  2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 5/5] igb: Fix unused variable warning Alexander Duyck
@ 2015-11-20  4:50   ` Brown, Aaron F
  0 siblings, 0 replies; 19+ messages in thread
From: Brown, Aaron F @ 2015-11-20  4:50 UTC (permalink / raw)
  To: intel-wired-lan

> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Tuesday, October 27, 2015 5:00 PM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH 5/5] igb: Fix unused variable warning
> 
> I ran into an unused variable warning when building dev-queue.  It turns
> out that hw isn't used anymore when setting up the q_vectors since we
> switch from hw->hw_addr to adapter->io_addr in igb_request_irq.  As such
> the variable can be dropped.
> 
> Fixes: 0b3659ca297fc ("igb: improve handling of disconnected adapters")
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c |    1 -
>  1 file changed, 1 deletion(-)
> 

Tested-by: Aaron Brown <aaron.f.brown@intel.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-11-20  4:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-27 23:59 [Intel-wired-lan] [next PATCH 0/5] Misc Intel driver cleanups Alexander Duyck
2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 1/5] fm10k: Cleanup MSI-X interrupts in case of failure Alexander Duyck
2015-10-28 16:11   ` Allan, Bruce W
2015-11-03 20:45     ` Singh, Krishneil K
2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 2/5] fm10k: Cleanup exception handling for mailbox interrupt Alexander Duyck
2015-10-28 16:11   ` Allan, Bruce W
2015-11-03 20:47   ` Singh, Krishneil K
2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 3/5] fm10k: Cleanup exception handling for changing queues Alexander Duyck
2015-10-28 16:11   ` Allan, Bruce W
2015-10-29 19:12   ` Allan, Bruce W
2015-10-29 19:39     ` Alexander Duyck
2015-11-03 23:20       ` Allan, Bruce W
2015-11-10 16:46         ` Allan, Bruce W
2015-11-10 17:30       ` Jeff Kirsher
2015-11-03 20:48   ` Singh, Krishneil K
2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 4/5] e1000e: Switch e1000e_up to void, drop code checking for error result Alexander Duyck
2015-11-20  4:48   ` Brown, Aaron F
2015-10-27 23:59 ` [Intel-wired-lan] [next PATCH 5/5] igb: Fix unused variable warning Alexander Duyck
2015-11-20  4:50   ` Brown, Aaron F

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.