Commit c0d9665f authored by David S. Miller's avatar David S. Miller

Merge branch 'bnxt_en-rtnl-fixes'

Michael Chan says:

====================
bnxt_en: Fix RTNL lock usage in bnxt_sp_task().

There are 2 function calls from bnxt_sp_task() that have buggy RTNL
usage.  These 2 functions take RTNL lock under some conditions, but
some callers (such as open, ethtool) have already taken RTNL.  These
3 patches fix the issue by making it clear that callers must take
RTNL.  If the caller is bnxt_sp_task() which does not automatically
take RTNL, we add a common scheme for bnxt_sp_task() to call these
functions properly under RTNL.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 56d80622 90c694bb
...@@ -5314,17 +5314,12 @@ static int bnxt_update_link(struct bnxt *bp, bool chng_link_state) ...@@ -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;
} }
...@@ -6200,29 +6195,37 @@ static void bnxt_timer(unsigned long data) ...@@ -6200,29 +6195,37 @@ static void bnxt_timer(unsigned long data)
mod_timer(&bp->timer, jiffies + bp->current_interval); mod_timer(&bp->timer, jiffies + bp->current_interval);
} }
/* Only called from bnxt_sp_task() */ static void bnxt_rtnl_lock_sp(struct bnxt *bp)
static void bnxt_reset(struct bnxt *bp, bool silent)
{ {
/* bnxt_reset_task() calls bnxt_close_nic() which waits /* We are called from bnxt_sp_task which has BNXT_STATE_IN_SP_TASK
* for BNXT_STATE_IN_SP_TASK to clear. * set. If the device is being closed, bnxt_close() may be holding
* If there is a parallel dev_close(), bnxt_close() may be holding
* rtnl() and waiting for BNXT_STATE_IN_SP_TASK to clear. So we * rtnl() and waiting for BNXT_STATE_IN_SP_TASK to clear. So we
* must clear BNXT_STATE_IN_SP_TASK before holding rtnl(). * must clear BNXT_STATE_IN_SP_TASK before holding rtnl().
*/ */
clear_bit(BNXT_STATE_IN_SP_TASK, &bp->state); clear_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
rtnl_lock(); rtnl_lock();
if (test_bit(BNXT_STATE_OPEN, &bp->state)) }
bnxt_reset_task(bp, silent);
static void bnxt_rtnl_unlock_sp(struct bnxt *bp)
{
set_bit(BNXT_STATE_IN_SP_TASK, &bp->state); set_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
rtnl_unlock(); rtnl_unlock();
} }
/* Only called from bnxt_sp_task() */
static void bnxt_reset(struct bnxt *bp, bool silent)
{
bnxt_rtnl_lock_sp(bp);
if (test_bit(BNXT_STATE_OPEN, &bp->state))
bnxt_reset_task(bp, silent);
bnxt_rtnl_unlock_sp(bp);
}
static void bnxt_cfg_ntp_filters(struct bnxt *); 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();
...@@ -6236,16 +6239,6 @@ static void bnxt_sp_task(struct work_struct *work) ...@@ -6236,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)) {
...@@ -6266,18 +6259,39 @@ static void bnxt_sp_task(struct work_struct *work) ...@@ -6266,18 +6259,39 @@ static void bnxt_sp_task(struct work_struct *work)
bnxt_hwrm_tunnel_dst_port_free( bnxt_hwrm_tunnel_dst_port_free(
bp, TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_GENEVE); bp, TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_GENEVE);
} }
if (test_and_clear_bit(BNXT_PERIODIC_STATS_SP_EVENT, &bp->sp_event))
bnxt_hwrm_port_qstats(bp);
/* These functions below will clear BNXT_STATE_IN_SP_TASK. They
* 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_HWRM_PORT_MODULE_SP_EVENT, &bp->sp_event)) {
bnxt_rtnl_lock_sp(bp);
if (test_bit(BNXT_STATE_OPEN, &bp->state))
bnxt_get_port_module_status(bp);
bnxt_rtnl_unlock_sp(bp);
}
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);
if (test_and_clear_bit(BNXT_RESET_TASK_SILENT_SP_EVENT, &bp->sp_event)) if (test_and_clear_bit(BNXT_RESET_TASK_SILENT_SP_EVENT, &bp->sp_event))
bnxt_reset(bp, true); bnxt_reset(bp, true);
if (test_and_clear_bit(BNXT_HWRM_PORT_MODULE_SP_EVENT, &bp->sp_event))
bnxt_get_port_module_status(bp);
if (test_and_clear_bit(BNXT_PERIODIC_STATS_SP_EVENT, &bp->sp_event))
bnxt_hwrm_port_qstats(bp);
smp_mb__before_atomic(); smp_mb__before_atomic();
clear_bit(BNXT_STATE_IN_SP_TASK, &bp->state); clear_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment