Re: [PATCH net-next] Revert ibmvnic merge do_change_param_reset into do_reset

[ljp <ljp@linux.vnet.ibm.com>]

On 2020-11-06 13:17, Dany Madden wrote:
> This reverts commit 16b5f5ce351f8709a6b518cc3cbf240c378305bf
> where it restructures do_reset. There are patches being tested that
> would require major rework if this is committed first.
> 
> We will resend this after the other patches have been applied.

I discussed with my manager, and he has agreed not revert this one
since it is in the net-next tree and will not affect net tree for
current bug fix patches.

Sorry for the confusion.

Thanks,
Lijun

> 
> Signed-off-by: Dany Madden <drt@linux.ibm.com>
> ---
>  drivers/net/ethernet/ibm/ibmvnic.c | 147 ++++++++++++++++++++---------
>  1 file changed, 104 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index f4167de30461..af4dfbe28d56 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -1826,6 +1826,86 @@ 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.
> @@ -1841,12 +1921,10 @@ static int do_reset(struct ibmvnic_adapter 
> *adapter,
>  	netdev_dbg(adapter->netdev, "Re-setting driver (%d)\n",
>  		   rwi->reset_reason);
> 
> -	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();
> +	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;
> @@ -1858,37 +1936,25 @@ 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) {
> -		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;
> +		adapter->state = VNIC_CLOSING;
> 
> -			if (adapter->state != VNIC_CLOSING) {
> -				rc = -1;
> -				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;
> 
> -			adapter->state = VNIC_CLOSED;
> +		if (adapter->state != VNIC_CLOSING) {
> +			rc = -1;
> +			goto out;
>  		}
> -	}
> 
> -	if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
> -		release_resources(adapter);
> -		release_sub_crqs(adapter, 1);
> -		release_crq_queue(adapter);
> +		adapter->state = VNIC_CLOSED;
>  	}
> 
>  	if (adapter->reset_reason != VNIC_RESET_NON_FATAL) {
> @@ -1897,9 +1963,7 @@ static int do_reset(struct ibmvnic_adapter 
> *adapter,
>  		 */
>  		adapter->state = VNIC_PROBED;
> 
> -		if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM) {
> -			rc = init_crq_queue(adapter);
> -		} else if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
> +		if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
>  			rc = ibmvnic_reenable_crq_queue(adapter);
>  			release_sub_crqs(adapter, 1);
>  		} else {
> @@ -1939,11 +2003,7 @@ static int do_reset(struct ibmvnic_adapter 
> *adapter,
>  			goto out;
>  		}
> 
> -		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 ||
> +		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 ||
> @@ -2004,9 +2064,7 @@ static int do_reset(struct ibmvnic_adapter 
> *adapter,
>  	rc = 0;
> 
>  out:
> -	/* requestor of VNIC_RESET_CHANGE_PARAM should still hold the rtnl 
> lock */
> -	if (!(adapter->reset_reason == VNIC_RESET_CHANGE_PARAM))
> -		rtnl_unlock();
> +	rtnl_unlock();
> 
>  	return rc;
>  }
> @@ -2140,7 +2198,10 @@ static void __ibmvnic_reset(struct work_struct 
> *work)
>  		}
>  		spin_unlock_irqrestore(&adapter->state_lock, flags);
> 
> -		if (adapter->force_reset_recovery) {
> +		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) {
>  			/* Transport event occurred during previous reset */
>  			if (adapter->wait_for_reset) {
>  				/* Previous was CHANGE_PARAM; caller locked */