leandrof/linux
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bnxt_en: Fix RTNL lock usage on bnxt_update_link().

Browse Source 
bnxt_update_link() is called from multiple code paths.  Most callers,
such as open, ethtool, already hold RTNL.  Only the caller bnxt_sp_task()
does not.  So it is a bug to take RTNL inside bnxt_update_link().

Fix it by removing the RTNL inside bnxt_update_link().  The function
now expects the caller to always hold RTNL.

In bnxt_sp_task(), call bnxt_rtnl_lock_sp() before calling
bnxt_update_link().  We also need to move the call to the end of
bnxt_sp_task() since it will be clearing the BNXT_STATE_IN_SP_TASK bit.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Michael Chan 2017-01-25 02:55:08 -05:00 committed by David S. Miller
parent a551ee94ea
commit 0eaa24b971
1 changed files with 18 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
drivers/net/ethernet/broadcom/bnxt/bnxt.c
View File

@ -5314,17 +5314,12 @@ static int bnxt_update_link(struct bnxt *bp, bool chng_link_state)
if ((link_info->support_auto_speeds | diff) != if ((link_info->support_auto_speeds | diff) !=
link_info->support_auto_speeds) { link_info->support_auto_speeds) {
/* An advertised speed is no longer supported, so we need to /* An advertised speed is no longer supported, so we need to
* update the advertisement settings. See bnxt_reset() for * update the advertisement settings. Caller holds RTNL
* comments about the rtnl_lock() sequence below. * so we can modify link settings.
*/ */
clear_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
rtnl_lock();
link_info->advertising = link_info->support_auto_speeds; link_info->advertising = link_info->support_auto_speeds;
if (test_bit(BNXT_STATE_OPEN, &bp->state) && if (link_info->autoneg & BNXT_AUTONEG_SPEED)
(link_info->autoneg & BNXT_AUTONEG_SPEED))
bnxt_hwrm_set_link_setting(bp, true, false); bnxt_hwrm_set_link_setting(bp, true, false);
set_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
rtnl_unlock();
} }
return 0; return 0;
} }
@ -6231,7 +6226,6 @@ static void bnxt_cfg_ntp_filters(struct bnxt *);
static void bnxt_sp_task(struct work_struct *work) static void bnxt_sp_task(struct work_struct *work)
{ {
struct bnxt *bp = container_of(work, struct bnxt, sp_task); struct bnxt *bp = container_of(work, struct bnxt, sp_task);
int rc;
set_bit(BNXT_STATE_IN_SP_TASK, &bp->state); set_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
smp_mb__after_atomic(); smp_mb__after_atomic();
@ -6245,16 +6239,6 @@ static void bnxt_sp_task(struct work_struct *work)
if (test_and_clear_bit(BNXT_RX_NTP_FLTR_SP_EVENT, &bp->sp_event)) if (test_and_clear_bit(BNXT_RX_NTP_FLTR_SP_EVENT, &bp->sp_event))
bnxt_cfg_ntp_filters(bp); bnxt_cfg_ntp_filters(bp);
if (test_and_clear_bit(BNXT_LINK_CHNG_SP_EVENT, &bp->sp_event)) {
if (test_and_clear_bit(BNXT_LINK_SPEED_CHNG_SP_EVENT,
&bp->sp_event))
bnxt_hwrm_phy_qcaps(bp);
rc = bnxt_update_link(bp, true);
if (rc)
netdev_err(bp->dev, "SP task can't update link (rc: %x)\n",
rc);
}
if (test_and_clear_bit(BNXT_HWRM_EXEC_FWD_REQ_SP_EVENT, &bp->sp_event)) if (test_and_clear_bit(BNXT_HWRM_EXEC_FWD_REQ_SP_EVENT, &bp->sp_event))
bnxt_hwrm_exec_fwd_req(bp); bnxt_hwrm_exec_fwd_req(bp);
if (test_and_clear_bit(BNXT_VXLAN_ADD_PORT_SP_EVENT, &bp->sp_event)) { if (test_and_clear_bit(BNXT_VXLAN_ADD_PORT_SP_EVENT, &bp->sp_event)) {
@ -6284,6 +6268,21 @@ static void bnxt_sp_task(struct work_struct *work)
/* These functions below will clear BNXT_STATE_IN_SP_TASK. They /* These functions below will clear BNXT_STATE_IN_SP_TASK. They
* must be the last functions to be called before exiting. * must be the last functions to be called before exiting.
*/ */
if (test_and_clear_bit(BNXT_LINK_CHNG_SP_EVENT, &bp->sp_event)) {
int rc = 0;
if (test_and_clear_bit(BNXT_LINK_SPEED_CHNG_SP_EVENT,
&bp->sp_event))
bnxt_hwrm_phy_qcaps(bp);
bnxt_rtnl_lock_sp(bp);
if (test_bit(BNXT_STATE_OPEN, &bp->state))
rc = bnxt_update_link(bp, true);
bnxt_rtnl_unlock_sp(bp);
if (rc)
netdev_err(bp->dev, "SP task can't update link (rc: %x)\n",
rc);
}
if (test_and_clear_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event)) if (test_and_clear_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event))
bnxt_reset(bp, false); bnxt_reset(bp, false);