diff mbox series

[net,15/15] ibmvnic: add some debugs

Message ID 20201120224049.46933-16-ljp@linux.ibm.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series ibmvnic: assorted bug fixes | expand

Checks

Context Check Description
netdev/apply fail Patch does not apply to net
netdev/tree_selection success Clearly marked for net

Commit Message

Lijun Pan Nov. 20, 2020, 10:40 p.m. UTC 
From: Sukadev Bhattiprolu <sukadev@linux.ibm.com>

We sometimes run into situations where a soft/hard reset of the adapter
takes a long time or fails to complete. Having additional messages that
include important adapter state info will hopefully help understand what
is happening, reduce the guess work and minimize requests to reproduce
problems with debug patches.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

Comments

Jakub Kicinski Nov. 21, 2020, 11:45 p.m. UTC | #1 
On Fri, 20 Nov 2020 16:40:49 -0600 Lijun Pan wrote:
> From: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
> 
> We sometimes run into situations where a soft/hard reset of the adapter
> takes a long time or fails to complete. Having additional messages that
> include important adapter state info will hopefully help understand what
> is happening, reduce the guess work and minimize requests to reproduce
> problems with debug patches.

This doesn't qualify as a bug fix, please send it to net-next.

> +	netdev_err(adapter->netdev,
> +		   "[S:%d FRR:%d WFR:%d] Done processing resets\n",
> +		   adapter->state, adapter->force_reset_recovery,
> +		   adapter->wait_for_reset);

Does reset only happen as a result of an error? Should this be a
netdev_info() instead?
Sukadev Bhattiprolu Nov. 23, 2020, 7:48 p.m. UTC | #2 
Jakub Kicinski [kuba@kernel.org] wrote:
> On Fri, 20 Nov 2020 16:40:49 -0600 Lijun Pan wrote:
> > From: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
> > 
> > We sometimes run into situations where a soft/hard reset of the adapter
> > takes a long time or fails to complete. Having additional messages that
> > include important adapter state info will hopefully help understand what
> > is happening, reduce the guess work and minimize requests to reproduce
> > problems with debug patches.
> 
> This doesn't qualify as a bug fix, please send it to net-next.

Ok.

> 
> > +	netdev_err(adapter->netdev,
> > +		   "[S:%d FRR:%d WFR:%d] Done processing resets\n",
> > +		   adapter->state, adapter->force_reset_recovery,
> > +		   adapter->wait_for_reset);
> 
> Does reset only happen as a result of an error? Should this be a
> netdev_info() instead?

It is an informational message, will change to netdev_info().

Thanks,

Sukadev
diff mbox series

Patch

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index b1519e92ccce..1abeb3edee33 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -406,6 +406,8 @@  static void replenish_pools(struct ibmvnic_adapter *adapter)
 		if (adapter->rx_pool[i].active)
 			replenish_rx_pool(adapter, &adapter->rx_pool[i]);
 	}
+
+	netdev_dbg(adapter->netdev, "Replenished %d pools\n", i);
 }
 
 static void release_stats_buffers(struct ibmvnic_adapter *adapter)
@@ -911,6 +913,7 @@  static int ibmvnic_login(struct net_device *netdev)
 
 	__ibmvnic_set_mac(netdev, adapter->mac_addr);
 
+	netdev_dbg(netdev, "[S:%d] Login succeeded\n", adapter->state);
 	return 0;
 }
 
@@ -1377,6 +1380,10 @@  static int ibmvnic_close(struct net_device *netdev)
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
 	int rc;
 
+	netdev_dbg(netdev, "[S:%d FOP:%d FRR:%d] Closing\n",
+		   adapter->state, adapter->failover_pending,
+		   adapter->force_reset_recovery);
+
 	/* If device failover is pending, just set device state and return.
 	 * Device operation will be handled by reset routine.
 	 */
@@ -1969,8 +1976,10 @@  static int do_reset(struct ibmvnic_adapter *adapter,
 	struct net_device *netdev = adapter->netdev;
 	int i, rc;
 
-	netdev_dbg(adapter->netdev, "Re-setting driver (%d)\n",
-		   rwi->reset_reason);
+	netdev_dbg(adapter->netdev,
+		   "[S:%d FOP:%d] Reset reason %d, reset_state %d\n",
+		   adapter->state, adapter->failover_pending,
+		   rwi->reset_reason, reset_state);
 
 	rtnl_lock();
 	/*
@@ -2129,6 +2138,8 @@  static int do_reset(struct ibmvnic_adapter *adapter,
 		adapter->state = reset_state;
 	rtnl_unlock();
 
+	netdev_dbg(adapter->netdev, "[S:%d FOP:%d] Reset done, rc %d\n",
+		   adapter->state, adapter->failover_pending, rc);
 	return rc;
 }
 
@@ -2198,6 +2209,8 @@  static int do_hard_reset(struct ibmvnic_adapter *adapter,
 	/* restore adapter state if reset failed */
 	if (rc)
 		adapter->state = reset_state;
+	netdev_dbg(adapter->netdev, "[S:%d FOP:%d] Hard reset done, rc %d\n",
+		   adapter->state, adapter->failover_pending, rc);
 	return rc;
 }
 
@@ -2307,7 +2320,14 @@  static void __ibmvnic_reset(struct work_struct *work)
 		complete(&adapter->reset_done);
 	}
 
+	netdev_dbg(adapter->netdev, "FRR=%d\n", adapter->force_reset_recovery);
+
 	clear_bit_unlock(0, &adapter->resetting);
+
+	netdev_err(adapter->netdev,
+		   "[S:%d FRR:%d WFR:%d] Done processing resets\n",
+		   adapter->state, adapter->force_reset_recovery,
+		   adapter->wait_for_reset);
 }
 
 static void __ibmvnic_delayed_reset(struct work_struct *work)
@@ -2353,7 +2373,8 @@  static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 	list_for_each(entry, &adapter->rwi_list) {
 		tmp = list_entry(entry, struct ibmvnic_rwi, list);
 		if (tmp->reset_reason == reason) {
-			netdev_dbg(netdev, "Skipping matching reset\n");
+			netdev_dbg(netdev, "Skipping matching reset, reason=%d\n",
+				   reason);
 			spin_unlock_irqrestore(&adapter->rwi_lock, flags);
 			ret = EBUSY;
 			goto err;