Re: [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset

Hi， Don

I use a state variable to replace reading directly from the register. I
hope it can alleviate your concern. Please  comment.
Sorry. My mail service can not work well with netdev maillist. So I write
this mail to make sure that you can read this patch.

If any problem, please feel free to let me know.

Zhu Yanjun

On Tue, Jul 19, 2016 at 6:30 AM, Skidmore, Donald C <
donald.c.skidmore@intel.com> wrote:

quoted

-----Original Message-----
From: zyjzyj2000@gmail.com [mailto:zyjzyj2000@gmail.com]
Sent: Monday, July 18, 2016 6:45 AM
To: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Kirsher,
Jeffrey T [off-list ref]
Subject: [E1000-devel] [PATCH 1/1] ixgbevf: avoid checking hang when
performing hardware reset

From: Zhu Yanjun <zyjzyj2000@gmail.com>

When performing hardware reset, it is not necessary to check hang.
Or else, the call trace will appear.

Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c

b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index acc2401..d563d24 100644

--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c

@@ -2792,9 +2792,14 @@ static void ixgbevf_reset_subtask(struct

ixgbevf_adapter *adapter)  static void ixgbevf_check_hang_subtask(struct
ixgbevf_adapter *adapter)  {
      struct ixgbe_hw *hw = &adapter->hw;
+     struct ixgbe_mbx_info *mbx = &hw->mbx;
      u32 eics = 0;
      int i;

+     /* When performing hardware reset, unnecessary to check hang. */
+     if (mbx->ops.check_for_rst(hw))
+             return;
+
      /* If we're down or resetting, just bail */
      if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
          test_bit(__IXGBEVF_RESETTING, &adapter->state))
--
2.7.4

My concern with this patch is that the check_for_rst does a read to clear
on the RSTD and RSTI bits.  So reading it in the service task may mean we
miss this transition in the mailbox protocol.  Likewise it is possible they
may have already been cleared and you won't even catch the state your
looking for.

-Don Skidmore [off-list ref]