* [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset
@ 2016-07-18 13:44 zyjzyj2000
2016-07-18 22:30 ` [E1000-devel] " Skidmore, Donald C
0 siblings, 1 reply; 6+ messages in thread
From: zyjzyj2000 @ 2016-07-18 13:44 UTC (permalink / raw)
To: e1000-devel, netdev, jeffrey.t.kirsher; +Cc: Zhu Yanjun
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* RE: [E1000-devel] [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset
2016-07-18 13:44 [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset zyjzyj2000
@ 2016-07-18 22:30 ` Skidmore, Donald C
2016-07-19 13:31 ` (unknown), zyjzyj2000
2016-07-20 2:42 ` zhuyj
0 siblings, 2 replies; 6+ messages in thread
From: Skidmore, Donald C @ 2016-07-18 22:30 UTC (permalink / raw)
To: zyjzyj2000@gmail.com, e1000-devel@lists.sourceforge.net,
netdev@vger.kernel.org, Kirsher, Jeffrey T
> -----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 <jeffrey.t.kirsher@intel.com>
> 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 <donald.c.skidmore@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* (unknown),
2016-07-18 22:30 ` [E1000-devel] " Skidmore, Donald C
@ 2016-07-19 13:31 ` zyjzyj2000
2016-07-19 13:31 ` [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset zyjzyj2000
2016-07-20 2:42 ` zhuyj
1 sibling, 1 reply; 6+ messages in thread
From: zyjzyj2000 @ 2016-07-19 13:31 UTC (permalink / raw)
To: e1000-devel, netdev, jeffrey.t.kirsher, donald.c.skidmore
v1->v2:
Follow the advice from Donald, replacing read directly from RSTD and
RSTI register with a state variable __IXGBEVF_HW_RESETTING;
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset
2016-07-19 13:31 ` (unknown), zyjzyj2000
@ 2016-07-19 13:31 ` zyjzyj2000
0 siblings, 0 replies; 6+ messages in thread
From: zyjzyj2000 @ 2016-07-19 13:31 UTC (permalink / raw)
To: e1000-devel, netdev, jeffrey.t.kirsher, donald.c.skidmore; +Cc: Zhu Yanjun
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.h | 1 +
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 5 +++--
drivers/net/ethernet/intel/ixgbevf/vf.c | 5 +++++
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index d5944c3..60fc63b 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -446,6 +446,7 @@ enum ixbgevf_state_t {
__IXGBEVF_SERVICE_INITED,
__IXGBEVF_RESET_REQUESTED,
__IXGBEVF_QUEUE_RESET_REQUESTED,
+ __IXGBEVF_HW_RESETTING,
};
enum ixgbevf_boards {
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index acc2401..530005b 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -2795,9 +2795,10 @@ static void ixgbevf_check_hang_subtask(struct ixgbevf_adapter *adapter)
u32 eics = 0;
int i;
- /* If we're down or resetting, just bail */
+ /* If we're down, resetting or hw resetting, just bail */
if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
- test_bit(__IXGBEVF_RESETTING, &adapter->state))
+ test_bit(__IXGBEVF_RESETTING, &adapter->state) ||
+ test_bit(__IXGBEVF_HW_RESETTING, &adapter->state))
return;
/* Force detection of hung controller */
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
index e670d3b..4ec4484 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -80,6 +80,9 @@ static s32 ixgbevf_reset_hw_vf(struct ixgbe_hw *hw)
s32 ret_val = IXGBE_ERR_INVALID_MAC_ADDR;
u32 msgbuf[IXGBE_VF_PERMADDR_MSG_LEN];
u8 *addr = (u8 *)(&msgbuf[1]);
+ struct ixgbevf_adapter *adapter = hw->back;
+
+ set_bit(__IXGBEVF_HW_RESETTING, &adapter->state);
/* Call adapter stop to disable tx/rx and clear interrupts */
hw->mac.ops.stop_adapter(hw);
@@ -128,6 +131,8 @@ static s32 ixgbevf_reset_hw_vf(struct ixgbe_hw *hw)
hw->mac.mc_filter_type = msgbuf[IXGBE_VF_MC_TYPE_WORD];
+ clear_bit(__IXGBEVF_HW_RESETTING, &adapter->state);
+
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset
2016-07-18 22:30 ` [E1000-devel] " Skidmore, Donald C
2016-07-19 13:31 ` (unknown), zyjzyj2000
@ 2016-07-20 2:42 ` zhuyj
1 sibling, 0 replies; 6+ messages in thread
From: zhuyj @ 2016-07-20 2:42 UTC (permalink / raw)
To: Skidmore, Donald C
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
[-- Attachment #1.1: Type: text/plain, Size: 2491 bytes --]
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:
> > -----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 <jeffrey.t.kirsher@intel.com>
> > 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 <donald.c.skidmore@intel.com>
>
>
>
[-- Attachment #2: Type: text/plain, Size: 422 bytes --]
------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev
[-- Attachment #3: Type: text/plain, Size: 257 bytes --]
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 6+ messages in thread
* [V2 PATCH 1/1] ixgbevf: avoid checking hang when performing hardware
@ 2016-07-19 23:23 zyjzyj2000
2016-07-19 23:23 ` [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset zyjzyj2000
0 siblings, 1 reply; 6+ messages in thread
From: zyjzyj2000 @ 2016-07-19 23:23 UTC (permalink / raw)
To: e1000-devel, netdev, jeffrey.t.kirsher, donald.c.skidmore,
intel-wired-lan
v1->v2:
Follow the advice from Donald, replacing read directly from RSTD and RSTI register with a state variable __IXGBEVF_HW_RESETTING;
------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset
2016-07-19 23:23 [V2 PATCH 1/1] ixgbevf: avoid checking hang when performing hardware zyjzyj2000
@ 2016-07-19 23:23 ` zyjzyj2000
0 siblings, 0 replies; 6+ messages in thread
From: zyjzyj2000 @ 2016-07-19 23:23 UTC (permalink / raw)
To: e1000-devel, netdev, jeffrey.t.kirsher, donald.c.skidmore,
intel-wired-lan
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.h | 1 +
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 5 +++--
drivers/net/ethernet/intel/ixgbevf/vf.c | 5 +++++
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index d5944c3..60fc63b 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -446,6 +446,7 @@ enum ixbgevf_state_t {
__IXGBEVF_SERVICE_INITED,
__IXGBEVF_RESET_REQUESTED,
__IXGBEVF_QUEUE_RESET_REQUESTED,
+ __IXGBEVF_HW_RESETTING,
};
enum ixgbevf_boards {
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index acc2401..530005b 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -2795,9 +2795,10 @@ static void ixgbevf_check_hang_subtask(struct ixgbevf_adapter *adapter)
u32 eics = 0;
int i;
- /* If we're down or resetting, just bail */
+ /* If we're down, resetting or hw resetting, just bail */
if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||
- test_bit(__IXGBEVF_RESETTING, &adapter->state))
+ test_bit(__IXGBEVF_RESETTING, &adapter->state) ||
+ test_bit(__IXGBEVF_HW_RESETTING, &adapter->state))
return;
/* Force detection of hung controller */
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
index e670d3b..4ec4484 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -80,6 +80,9 @@ static s32 ixgbevf_reset_hw_vf(struct ixgbe_hw *hw)
s32 ret_val = IXGBE_ERR_INVALID_MAC_ADDR;
u32 msgbuf[IXGBE_VF_PERMADDR_MSG_LEN];
u8 *addr = (u8 *)(&msgbuf[1]);
+ struct ixgbevf_adapter *adapter = hw->back;
+
+ set_bit(__IXGBEVF_HW_RESETTING, &adapter->state);
/* Call adapter stop to disable tx/rx and clear interrupts */
hw->mac.ops.stop_adapter(hw);
@@ -128,6 +131,8 @@ static s32 ixgbevf_reset_hw_vf(struct ixgbe_hw *hw)
hw->mac.mc_filter_type = msgbuf[IXGBE_VF_MC_TYPE_WORD];
+ clear_bit(__IXGBEVF_HW_RESETTING, &adapter->state);
+
return 0;
}
--
2.7.4
------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-20 2:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-18 13:44 [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset zyjzyj2000
2016-07-18 22:30 ` [E1000-devel] " Skidmore, Donald C
2016-07-19 13:31 ` (unknown), zyjzyj2000
2016-07-19 13:31 ` [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset zyjzyj2000
2016-07-20 2:42 ` zhuyj
-- strict thread matches above, loose matches on Subject: below --
2016-07-19 23:23 [V2 PATCH 1/1] ixgbevf: avoid checking hang when performing hardware zyjzyj2000
2016-07-19 23:23 ` [PATCH 1/1] ixgbevf: avoid checking hang when performing hardware reset zyjzyj2000
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).