diff mbox series

[net-next,1/1] bnx2x: Collect the device debug information during Tx timeout.

Message ID 20180524062145.29009-1-sudarsana.kalluru@cavium.com
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series [net-next,1/1] bnx2x: Collect the device debug information during Tx timeout. | expand

Commit Message

Sudarsana Reddy Kalluru May 24, 2018, 6:21 a.m. UTC
Tx-timeout mostly happens due to some issue in the device. In such cases,
debug dump would be helpful for identifying the cause of the issue.
This patch adds support to spill debug data during the Tx timeout. Here
bnx2x_panic_dump() API is used instead of bnx2x_panic(), since we still
want to allow the Tx-timeout recovery a chance to succeed.

Please consider applying this to "net-next".

Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Yuval Mintz May 24, 2018, 11:25 a.m. UTC | #1
> Tx-timeout mostly happens due to some issue in the device. In such cases,
> debug dump would be helpful for identifying the cause of the issue.
> This patch adds support to spill debug data during the Tx timeout. Here
> bnx2x_panic_dump() API is used instead of bnx2x_panic(), since we still
> want to allow the Tx-timeout recovery a chance to succeed.
> 
> Please consider applying this to "net-next".
> 
> Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index 95871576..182d5e1 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -4962,8 +4962,13 @@ void bnx2x_tx_timeout(struct net_device *dev)
>  {
>  	struct bnx2x *bp = netdev_priv(dev);
> 
> -#ifdef BNX2X_STOP_ON_ERROR
> +	/* We want the information of the dump logged,
> +	 * but calling bnx2x_panic() would kill all chances of recovery.
> +	 */
>  	if (!bp->panic)
> +#ifdef BNX2X_STOP_ON_ERROR
> +		bnx2x_panic_dump(bp, false);
> +#else
>  		bnx2x_panic();
>  #endif

This looks backward to me; When BNX2X_STOP_ON_ERROR is defined
you *want* bnx2x_panic() to fatally stop the device, not the other way
around.
I.e., s/ifdef/ifndef/ 

> 
> --
> 1.8.3.1
Sudarsana Reddy Kalluru May 24, 2018, 11:40 a.m. UTC | #2
-----Original Message-----
From: Yuval Mintz [mailto:yuvalm@mellanox.com] 
Sent: 24 May 2018 16:55
To: Kalluru, Sudarsana <Sudarsana.Kalluru@cavium.com>; davem@davemloft.net
Cc: netdev@vger.kernel.org
Subject: RE: [PATCH net-next 1/1] bnx2x: Collect the device debug information during Tx timeout.

> Tx-timeout mostly happens due to some issue in the device. In such 
> cases, debug dump would be helpful for identifying the cause of the issue.
> This patch adds support to spill debug data during the Tx timeout. 
> Here
> bnx2x_panic_dump() API is used instead of bnx2x_panic(), since we 
> still want to allow the Tx-timeout recovery a chance to succeed.
> 
> Please consider applying this to "net-next".
> 
> Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index 95871576..182d5e1 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -4962,8 +4962,13 @@ void bnx2x_tx_timeout(struct net_device *dev)  
> {
>  	struct bnx2x *bp = netdev_priv(dev);
> 
> -#ifdef BNX2X_STOP_ON_ERROR
> +	/* We want the information of the dump logged,
> +	 * but calling bnx2x_panic() would kill all chances of recovery.
> +	 */
>  	if (!bp->panic)
> +#ifdef BNX2X_STOP_ON_ERROR
> +		bnx2x_panic_dump(bp, false);
> +#else
>  		bnx2x_panic();
>  #endif

This looks backward to me; When BNX2X_STOP_ON_ERROR is defined you *want* bnx2x_panic() to fatally stop the device, not the other way around.
I.e., s/ifdef/ifndef/ 


Yuval, thanks a lot for your review. Will post the updated changes.

> 
> --
> 1.8.3.1
diff mbox series

Patch

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 95871576..182d5e1 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -4962,8 +4962,13 @@  void bnx2x_tx_timeout(struct net_device *dev)
 {
 	struct bnx2x *bp = netdev_priv(dev);
 
-#ifdef BNX2X_STOP_ON_ERROR
+	/* We want the information of the dump logged,
+	 * but calling bnx2x_panic() would kill all chances of recovery.
+	 */
 	if (!bp->panic)
+#ifdef BNX2X_STOP_ON_ERROR
+		bnx2x_panic_dump(bp, false);
+#else
 		bnx2x_panic();
 #endif