Skip to contentSkip to conversation

Patch "bnxt_en: Fix bnxt_reset() in the slow path task." has been added to the 4.9-stable tree

PATCH [email protected] 1 participant 1 message Jun 15, 2017

Subsystems (3)
Network drivers Stable branch Broadcom bnxt_en 50 gigabit Ethernet driver
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

bnxt_en: Fix bnxt_reset() in the slow path task.

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-bnxt_reset-in-the-slow-path-task.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 <[email protected]> know about it.

Quoted 1 line

From foo@baz Thu Jun 15 16:23:30 CEST 2017

From: Michael Chan <[email protected]>
Date: Wed, 25 Jan 2017 02:55:07 -0500
Subject: bnxt_en: Fix bnxt_reset() in the slow path task.

From: Michael Chan <[email protected]>

[ Upstream commit a551ee94ea723b4af9b827c7460f108bc13425ee ]

In bnxt_sp_task(), we set a bit BNXT_STATE_IN_SP_TASK so that bnxt_close() will synchronize and wait for bnxt_sp_task() to finish. Some functions in bnxt_sp_task() require us to clear BNXT_STATE_IN_SP_TASK and then acquire rtnl_lock() to prevent race conditions.

There are some bugs related to this logic. This patch refactors the code to have common bnxt_rtnl_lock_sp() and bnxt_rtnl_unlock_sp() to handle the RTNL and the clearing/setting of the bit. Multiple functions will need the same logic. We also need to move bnxt_reset() to the end of bnxt_sp_task(). Functions that clear BNXT_STATE_IN_SP_TASK must be the last functions to be called in bnxt_sp_task(). The common scheme will handle the condition properly.

Signed-off-by Michael Chan <[email protected]>
Signed-off-by David S. Miller <[email protected]>
Signed-off-by Sasha Levin <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c |   38 +++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 13 deletions(-)
drivers/net/ethernet/broadcom/bnxt/bnxt.c +25 -13
@@ -6080,23 +6080,32 @@ bnxt_restart_timer:
 	mod_timer(&bp->timer, jiffies + bp->current_interval);
 }
 
-/* Only called from bnxt_sp_task() */
-static void bnxt_reset(struct bnxt *bp, bool silent)
+static void bnxt_rtnl_lock_sp(struct bnxt *bp)
 {
-	/* bnxt_reset_task() calls bnxt_close_nic() which waits
-	 * for BNXT_STATE_IN_SP_TASK to clear.
-	 * If there is a parallel dev_close(), bnxt_close() may be holding
+	/* We are called from bnxt_sp_task which has BNXT_STATE_IN_SP_TASK
+	 * set.  If the device is being closed, bnxt_close() may be holding
 	 * rtnl() and waiting for BNXT_STATE_IN_SP_TASK to clear.  So we
 	 * must clear BNXT_STATE_IN_SP_TASK before holding rtnl().
 	 */
 	clear_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
 	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);
 	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_sp_task(struct work_struct *work)

@@ -6142,18 +6151,21 @@ static void bnxt_sp_task(struct work_str
 		bnxt_hwrm_tunnel_dst_port_free(
 			bp, TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_GENEVE);
 	}
-	if (test_and_clear_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event))
-		bnxt_reset(bp, false);
-
-	if (test_and_clear_bit(BNXT_RESET_TASK_SILENT_SP_EVENT, &bp->sp_event))
-		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);
 
+	/* 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_RESET_TASK_SP_EVENT, &bp->sp_event))
+		bnxt_reset(bp, false);
+
+	if (test_and_clear_bit(BNXT_RESET_TASK_SILENT_SP_EVENT, &bp->sp_event))
+		bnxt_reset(bp, true);
+
 	smp_mb__before_atomic();
 	clear_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
 }

Patches currently in stable-queue which might be from [email protected] 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