e1000e: Separate signaling for link check/link up
Lennart reported the following race condition: \ e1000_watchdog_task \ e1000e_has_link \ hw->mac.ops.check_for_link() === e1000e_check_for_copper_link /* link is up */ mac->get_link_status = false; /* interrupt */ \ e1000_msix_other hw->mac.get_link_status = true; link_active = !hw->mac.get_link_status /* link_active is false, wrongly */ This problem arises because the single flag get_link_status is used to signal two different states: link status needs checking and link status is down. Avoid the problem by using the return value of .check_for_link to signal the link status to e1000e_has_link(). Reported-by: Lennart Sorensen <lsorense@csclub.uwaterloo.ca> Signed-off-by: Benjamin Poirier <bpoirier@suse.com> Tested-by: Aaron Brown <aaron.f.brown@intel.com> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
This commit is contained in:
parent
d3509f8bc7
commit
19110cfbb3
2 changed files with 9 additions and 4 deletions
|
@ -410,6 +410,9 @@ void e1000e_clear_hw_cntrs_base(struct e1000_hw *hw)
|
||||||
* Checks to see of the link status of the hardware has changed. If a
|
* Checks to see of the link status of the hardware has changed. If a
|
||||||
* change in link status has been detected, then we read the PHY registers
|
* change in link status has been detected, then we read the PHY registers
|
||||||
* to get the current speed/duplex if link exists.
|
* to get the current speed/duplex if link exists.
|
||||||
|
*
|
||||||
|
* Returns a negative error code (-E1000_ERR_*) or 0 (link down) or 1 (link
|
||||||
|
* up).
|
||||||
**/
|
**/
|
||||||
s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
|
s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
|
||||||
{
|
{
|
||||||
|
@ -423,7 +426,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
|
||||||
* Change or Rx Sequence Error interrupt.
|
* Change or Rx Sequence Error interrupt.
|
||||||
*/
|
*/
|
||||||
if (!mac->get_link_status)
|
if (!mac->get_link_status)
|
||||||
return 0;
|
return 1;
|
||||||
|
|
||||||
/* First we want to see if the MII Status Register reports
|
/* First we want to see if the MII Status Register reports
|
||||||
* link. If so, then we want to get the current speed/duplex
|
* link. If so, then we want to get the current speed/duplex
|
||||||
|
@ -461,10 +464,12 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
|
||||||
* different link partner.
|
* different link partner.
|
||||||
*/
|
*/
|
||||||
ret_val = e1000e_config_fc_after_link_up(hw);
|
ret_val = e1000e_config_fc_after_link_up(hw);
|
||||||
if (ret_val)
|
if (ret_val) {
|
||||||
e_dbg("Error configuring flow control\n");
|
e_dbg("Error configuring flow control\n");
|
||||||
|
return ret_val;
|
||||||
|
}
|
||||||
|
|
||||||
return ret_val;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -5081,7 +5081,7 @@ static bool e1000e_has_link(struct e1000_adapter *adapter)
|
||||||
case e1000_media_type_copper:
|
case e1000_media_type_copper:
|
||||||
if (hw->mac.get_link_status) {
|
if (hw->mac.get_link_status) {
|
||||||
ret_val = hw->mac.ops.check_for_link(hw);
|
ret_val = hw->mac.ops.check_for_link(hw);
|
||||||
link_active = !hw->mac.get_link_status;
|
link_active = ret_val > 0;
|
||||||
} else {
|
} else {
|
||||||
link_active = true;
|
link_active = true;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Reference in a new issue