[RESEND PATCH 0/3 net-next] ibmvnic: Clean up net close and fix reset bug

STALE3143d

Revision resend of 2 in this series.

5 messages, 2 authors, 2018-03-08 · open the first message on its own page

[RESEND PATCH 0/3 net-next] ibmvnic: Clean up net close and fix reset bug

From: Thomas Falcon <hidden>
Date: 2018-03-07 23:51:54

This patch set cleans up and reorganizes the driver's net_device
close function and leverages that to fix up a bug that can occur
during some device resets. Some reset cases require the backing
adapter to be disabled before continuing, but other cases, such as 
during a device failover or partition migration, do not require this
step. Since the device will not be initialized at this stage and
its command-processing queue is closed, do not send the request to
disable the device as it could result in an error or timeout
disrupting the reset.

Thomas Falcon (3):
  ibmvnic: Clean up device close
  ibmvnic: Reorganize device close
  ibmvnic: Do not disable device during failover or partition migration

 drivers/net/ethernet/ibm/ibmvnic.c | 48 ++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 25 deletions(-)

-- 
1.8.3.1

[RESEND PATCH 1/3 net-next] ibmvnic: Clean up device close

From: Thomas Falcon <hidden>
Date: 2018-03-07 23:51:55

Remove some dead code now that RX pools are being cleaned. This
was included to wait until any pending RX queue interrupts are
processed, but NAPI polling should be disabled by this point.

Another minor change is to use the net device parameter for any
print functions instead of accessing it from the adapter structure.

Signed-off-by: Thomas Falcon <redacted>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 7654071..fca0533 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1162,7 +1162,7 @@ static int __ibmvnic_close(struct net_device *netdev)
 	if (adapter->tx_scrq) {
 		for (i = 0; i < adapter->req_tx_queues; i++)
 			if (adapter->tx_scrq[i]->irq) {
-				netdev_dbg(adapter->netdev,
+				netdev_dbg(netdev,
 					   "Disabling tx_scrq[%d] irq\n", i);
 				disable_irq(adapter->tx_scrq[i]->irq);
 			}
@@ -1174,18 +1174,8 @@ static int __ibmvnic_close(struct net_device *netdev)
 
 	if (adapter->rx_scrq) {
 		for (i = 0; i < adapter->req_rx_queues; i++) {
-			int retries = 10;
-
-			while (pending_scrq(adapter, adapter->rx_scrq[i])) {
-				retries--;
-				mdelay(100);
-
-				if (retries == 0)
-					break;
-			}
-
 			if (adapter->rx_scrq[i]->irq) {
-				netdev_dbg(adapter->netdev,
+				netdev_dbg(netdev,
 					   "Disabling rx_scrq[%d] irq\n", i);
 				disable_irq(adapter->rx_scrq[i]->irq);
 			}
-- 
1.8.3.1

[RESEND PATCH 2/3 net-next] ibmvnic: Reorganize device close

From: Thomas Falcon <hidden>
Date: 2018-03-07 23:51:55

Introduce a function to halt network operations and clean up any
unused or outstanding socket buffers. Then, during device close,
disable backing adapter before halting all queues and performing
cleanup. This ensures all backing device operations will be
stopped before the driver cleans up shared resources.

Signed-off-by: Thomas Falcon <redacted>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index fca0533..d93f286 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1143,14 +1143,11 @@ static void clean_tx_pools(struct ibmvnic_adapter *adapter)
 	}
 }
 
-static int __ibmvnic_close(struct net_device *netdev)
+static void ibmvnic_cleanup(struct net_device *netdev)
 {
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
-	int rc = 0;
 	int i;
 
-	adapter->state = VNIC_CLOSING;
-
 	/* ensure that transmissions are stopped if called by do_reset */
 	if (adapter->resetting)
 		netif_tx_disable(netdev);
@@ -1168,10 +1165,6 @@ static int __ibmvnic_close(struct net_device *netdev)
 			}
 	}
 
-	rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
-	if (rc)
-		return rc;
-
 	if (adapter->rx_scrq) {
 		for (i = 0; i < adapter->req_rx_queues; i++) {
 			if (adapter->rx_scrq[i]->irq) {
@@ -1183,8 +1176,20 @@ static int __ibmvnic_close(struct net_device *netdev)
 	}
 	clean_rx_pools(adapter);
 	clean_tx_pools(adapter);
+}
+
+static int __ibmvnic_close(struct net_device *netdev)
+{
+	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+	int rc = 0;
+
+	adapter->state = VNIC_CLOSING;
+	rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
+	if (rc)
+		return rc;
+	ibmvnic_cleanup(netdev);
 	adapter->state = VNIC_CLOSED;
-	return rc;
+	return 0;
 }
 
 static int ibmvnic_close(struct net_device *netdev)
-- 
1.8.3.1

[RESEND PATCH 3/3 net-next] ibmvnic: Do not disable device during failover or partition migration

From: Thomas Falcon <hidden>
Date: 2018-03-07 23:51:57

During a device failover or partition migration reset, it is not
necessary to disable the backing adapter since it should not be
running yet and its Command-Response Queue is closed. Sending
device commands during this time could result in an error or
timeout disrupting the reset process. In these cases, just halt
transmissions, clean up resources, and continue with reset.

Signed-off-by: Thomas Falcon <redacted>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index d93f286..7be4b06 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1653,12 +1653,15 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 		rc = ibmvnic_reenable_crq_queue(adapter);
 		if (rc)
 			return 0;
+		ibmvnic_cleanup(netdev);
+	} else if (rwi->reset_reason == VNIC_RESET_FAILOVER) {
+		ibmvnic_cleanup(netdev);
+	} else {
+		rc = __ibmvnic_close(netdev);
+		if (rc)
+			return rc;
 	}
 
-	rc = __ibmvnic_close(netdev);
-	if (rc)
-		return rc;
-
 	if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
 	    adapter->wait_for_reset) {
 		release_resources(adapter);
-- 
1.8.3.1

Re: [RESEND PATCH 0/3 net-next] ibmvnic: Clean up net close and fix reset bug

From: David Miller <davem@davemloft.net>
Date: 2018-03-08 17:52:01

From: Thomas Falcon <redacted>
Date: Wed,  7 Mar 2018 17:51:44 -0600
This patch set cleans up and reorganizes the driver's net_device
close function and leverages that to fix up a bug that can occur
during some device resets. Some reset cases require the backing
adapter to be disabled before continuing, but other cases, such as 
during a device failover or partition migration, do not require this
step. Since the device will not be initialized at this stage and
its command-processing queue is closed, do not send the request to
disable the device as it could result in an error or timeout
disrupting the reset.
Series applied,thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help