From: <gregkh@linuxfoundation.org>
To: michael.chan@broadcom.com, alexander.levin@verizon.com,
davem@davemloft.net, gregkh@linuxfoundation.org
Cc: <stable@vger.kernel.org>, <stable-commits@vger.kernel.org>
Subject: Patch "bnxt_en: Fix RTNL lock usage on bnxt_update_link()." has been added to the 4.9-stable tree
Date: Thu, 15 Jun 2017 16:26:53 +0200 [thread overview]
Message-ID: <149753681315518@kroah.com> (raw)
This is a note to let you know that I've just added the patch titled
bnxt_en: Fix RTNL lock usage on bnxt_update_link().
to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
bnxt_en-fix-rtnl-lock-usage-on-bnxt_update_link.patch
and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
>From foo@baz Thu Jun 15 16:23:30 CEST 2017
From: Michael Chan <michael.chan@broadcom.com>
Date: Wed, 25 Jan 2017 02:55:08 -0500
Subject: bnxt_en: Fix RTNL lock usage on bnxt_update_link().
From: Michael Chan <michael.chan@broadcom.com>
[ Upstream commit 0eaa24b971ae251ae9d3be23f77662a655532063 ]
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>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 37 ++++++++++++++----------------
1 file changed, 18 insertions(+), 19 deletions(-)
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -5204,17 +5204,12 @@ static int bnxt_update_link(struct bnxt
if ((link_info->support_auto_speeds | diff) !=
link_info->support_auto_speeds) {
/* An advertised speed is no longer supported, so we need to
- * update the advertisement settings. See bnxt_reset() for
- * comments about the rtnl_lock() sequence below.
+ * update the advertisement settings. Caller holds RTNL
+ * 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;
- if (test_bit(BNXT_STATE_OPEN, &bp->state) &&
- (link_info->autoneg & BNXT_AUTONEG_SPEED))
+ if (link_info->autoneg & BNXT_AUTONEG_SPEED)
bnxt_hwrm_set_link_setting(bp, true, false);
- set_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
- rtnl_unlock();
}
return 0;
}
@@ -6130,7 +6125,6 @@ static void bnxt_cfg_ntp_filters(struct
static void bnxt_sp_task(struct work_struct *work)
{
struct bnxt *bp = container_of(work, struct bnxt, sp_task);
- int rc;
set_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
smp_mb__after_atomic();
@@ -6144,16 +6138,6 @@ static void bnxt_sp_task(struct work_str
if (test_and_clear_bit(BNXT_RX_NTP_FLTR_SP_EVENT, &bp->sp_event))
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))
bnxt_hwrm_exec_fwd_req(bp);
if (test_and_clear_bit(BNXT_VXLAN_ADD_PORT_SP_EVENT, &bp->sp_event)) {
@@ -6183,6 +6167,21 @@ static void bnxt_sp_task(struct work_str
/* 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_RESET_TASK_SP_EVENT, &bp->sp_event))
bnxt_reset(bp, false);
Patches currently in stable-queue which might be from michael.chan@broadcom.com are
queue-4.9/bnxt_en-fix-bnxt_reset-in-the-slow-path-task.patch
queue-4.9/bnxt_en-enhance-autoneg-support.patch
queue-4.9/bnxt_en-fix-rtnl-lock-usage-on-bnxt_update_link.patch
queue-4.9/bnxt_en-fix-rtnl-lock-usage-on-bnxt_get_port_module_status.patch
reply other threads:[~2017-06-15 14:27 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=149753681315518@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=alexander.levin@verizon.com \
--cc=davem@davemloft.net \
--cc=michael.chan@broadcom.com \
--cc=stable-commits@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox