ibmvnic: merge do_change_param_reset into do_reset
authorLijun Pan <ljp@linux.ibm.com>
Sat, 31 Oct 2020 09:46:45 +0000 (04:46 -0500)
committerJakub Kicinski <kuba@kernel.org>
Tue, 3 Nov 2020 23:08:14 +0000 (15:08 -0800)
Commit b27507bb59ed ("net/ibmvnic: unlock rtnl_lock in reset so
linkwatch_event can run") introduced do_change_param_reset function to
solve the rtnl lock issue. Majority of the code in do_change_param_reset
duplicates do_reset. Also, we can handle the rtnl lock issue in do_reset
itself. Hence merge do_change_param_reset back into do_reset to clean up
the code.

Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
Link: https://lore.kernel.org/r/20201031094645.17255-1-ljp@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/net/ethernet/ibm/ibmvnic.c

index af4dfbe..f4167de 100644 (file)
@@ -1826,86 +1826,6 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p)
        return rc;
 }
 
-/**
- * do_change_param_reset returns zero if we are able to keep processing reset
- * events, or non-zero if we hit a fatal error and must halt.
- */
-static int do_change_param_reset(struct ibmvnic_adapter *adapter,
-                                struct ibmvnic_rwi *rwi,
-                                u32 reset_state)
-{
-       struct net_device *netdev = adapter->netdev;
-       int i, rc;
-
-       netdev_dbg(adapter->netdev, "Change param resetting driver (%d)\n",
-                  rwi->reset_reason);
-
-       netif_carrier_off(netdev);
-       adapter->reset_reason = rwi->reset_reason;
-
-       ibmvnic_cleanup(netdev);
-
-       if (reset_state == VNIC_OPEN) {
-               rc = __ibmvnic_close(netdev);
-               if (rc)
-                       return rc;
-       }
-
-       release_resources(adapter);
-       release_sub_crqs(adapter, 1);
-       release_crq_queue(adapter);
-
-       adapter->state = VNIC_PROBED;
-
-       rc = init_crq_queue(adapter);
-
-       if (rc) {
-               netdev_err(adapter->netdev,
-                          "Couldn't initialize crq. rc=%d\n", rc);
-               return rc;
-       }
-
-       rc = ibmvnic_reset_init(adapter, true);
-       if (rc)
-               return IBMVNIC_INIT_FAILED;
-
-       /* If the adapter was in PROBE state prior to the reset,
-        * exit here.
-        */
-       if (reset_state == VNIC_PROBED)
-               return 0;
-
-       rc = ibmvnic_login(netdev);
-       if (rc) {
-               adapter->state = reset_state;
-               return rc;
-       }
-
-       rc = init_resources(adapter);
-       if (rc)
-               return rc;
-
-       ibmvnic_disable_irqs(adapter);
-
-       adapter->state = VNIC_CLOSED;
-
-       if (reset_state == VNIC_CLOSED)
-               return 0;
-
-       rc = __ibmvnic_open(netdev);
-       if (rc)
-               return IBMVNIC_OPEN_FAILED;
-
-       /* refresh device's multicast list */
-       ibmvnic_set_multi(netdev);
-
-       /* kick napi */
-       for (i = 0; i < adapter->req_rx_queues; i++)
-               napi_schedule(&adapter->napi[i]);
-
-       return 0;
-}
-
 /**
  * do_reset returns zero if we are able to keep processing reset events, or
  * non-zero if we hit a fatal error and must halt.
@@ -1921,10 +1841,12 @@ static int do_reset(struct ibmvnic_adapter *adapter,
        netdev_dbg(adapter->netdev, "Re-setting driver (%d)\n",
                   rwi->reset_reason);
 
-       rtnl_lock();
+       adapter->reset_reason = rwi->reset_reason;
+       /* requestor of VNIC_RESET_CHANGE_PARAM already has the rtnl lock */
+       if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM))
+               rtnl_lock();
 
        netif_carrier_off(netdev);
-       adapter->reset_reason = rwi->reset_reason;
 
        old_num_rx_queues = adapter->req_rx_queues;
        old_num_tx_queues = adapter->req_tx_queues;
@@ -1936,25 +1858,37 @@ static int do_reset(struct ibmvnic_adapter *adapter,
        if (reset_state == VNIC_OPEN &&
            adapter->reset_reason != VNIC_RESET_MOBILITY &&
            adapter->reset_reason != VNIC_RESET_FAILOVER) {
-               adapter->state = VNIC_CLOSING;
+               if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+                       rc = __ibmvnic_close(netdev);
+                       if (rc)
+                               goto out;
+               } else {
+                       adapter->state = VNIC_CLOSING;
 
-               /* Release the RTNL lock before link state change and
-                * re-acquire after the link state change to allow
-                * linkwatch_event to grab the RTNL lock and run during
-                * a reset.
-                */
-               rtnl_unlock();
-               rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
-               rtnl_lock();
-               if (rc)
-                       goto out;
+                       /* Release the RTNL lock before link state change and
+                        * re-acquire after the link state change to allow
+                        * linkwatch_event to grab the RTNL lock and run during
+                        * a reset.
+                        */
+                       rtnl_unlock();
+                       rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
+                       rtnl_lock();
+                       if (rc)
+                               goto out;
 
-               if (adapter->state != VNIC_CLOSING) {
-                       rc = -1;
-                       goto out;
+                       if (adapter->state != VNIC_CLOSING) {
+                               rc = -1;
+                               goto out;
+                       }
+
+                       adapter->state = VNIC_CLOSED;
                }
+       }
 
-               adapter->state = VNIC_CLOSED;
+       if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+               release_resources(adapter);
+               release_sub_crqs(adapter, 1);
+               release_crq_queue(adapter);
        }
 
        if (adapter->reset_reason != VNIC_RESET_NON_FATAL) {
@@ -1963,7 +1897,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
                 */
                adapter->state = VNIC_PROBED;
 
-               if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
+               if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+                       rc = init_crq_queue(adapter);
+               } else if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
                        rc = ibmvnic_reenable_crq_queue(adapter);
                        release_sub_crqs(adapter, 1);
                } else {
@@ -2003,7 +1939,11 @@ static int do_reset(struct ibmvnic_adapter *adapter,
                        goto out;
                }
 
-               if (adapter->req_rx_queues != old_num_rx_queues ||
+               if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
+                       rc = init_resources(adapter);
+                       if (rc)
+                               goto out;
+               } else if (adapter->req_rx_queues != old_num_rx_queues ||
                    adapter->req_tx_queues != old_num_tx_queues ||
                    adapter->req_rx_add_entries_per_subcrq !=
                    old_num_rx_slots ||
@@ -2064,7 +2004,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
        rc = 0;
 
 out:
-       rtnl_unlock();
+       /* requestor of VNIC_RESET_CHANGE_PARAM should still hold the rtnl lock */
+       if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM))
+               rtnl_unlock();
 
        return rc;
 }
@@ -2198,10 +2140,7 @@ static void __ibmvnic_reset(struct work_struct *work)
                }
                spin_unlock_irqrestore(&adapter->state_lock, flags);
 
-               if (rwi->reset_reason == VNIC_RESET_CHANGE_PARAM) {
-                       /* CHANGE_PARAM requestor holds rtnl_lock */
-                       rc = do_change_param_reset(adapter, rwi, reset_state);
-               } else if (adapter->force_reset_recovery) {
+               if (adapter->force_reset_recovery) {
                        /* Transport event occurred during previous reset */
                        if (adapter->wait_for_reset) {
                                /* Previous was CHANGE_PARAM; caller locked */