diff mbox series

[net-next,1/8] bnxt_en: Do not call sleeping hwmon_notify_event() from NAPI

Message ID 20231020212757.173551-2-michael.chan@broadcom.com (mailing list archive)
State Accepted
Commit 55862094a9d0d14f607eec00b925401407d61070
Delegated to: Netdev Maintainers
Headers show
Series bnxt_en: Update for net-next | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1363 this patch: 1363
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 1388 this patch: 1388
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1392 this patch: 1392
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 96 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Michael Chan Oct. 20, 2023, 9:27 p.m. UTC 
From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

Defer hwmon_notify_event() to bnxt_sp_task() workqueue because
hwmon_notify_event() can try to acquire a mutex shown in the stack trace
below.  Modify bnxt_event_error_report() to return true if we need to
schedule bnxt_sp_task() to notify hwmon.

  __schedule+0x68/0x520
  hwmon_notify_event+0xe8/0x114
  schedule+0x60/0xe0
  schedule_preempt_disabled+0x28/0x40
  __mutex_lock.constprop.0+0x534/0x550
  __mutex_lock_slowpath+0x18/0x20
  mutex_lock+0x5c/0x70
  kobject_uevent_env+0x2f4/0x3d0
  kobject_uevent+0x10/0x20
  hwmon_notify_event+0x94/0x114
  bnxt_hwmon_notify_event+0x40/0x70 [bnxt_en]
  bnxt_event_error_report+0x260/0x290 [bnxt_en]
  bnxt_async_event_process.isra.0+0x250/0x850 [bnxt_en]
  bnxt_hwrm_handler.isra.0+0xc8/0x120 [bnxt_en]
  bnxt_poll_p5+0x150/0x350 [bnxt_en]
  __napi_poll+0x3c/0x210
  net_rx_action+0x308/0x3b0
  __do_softirq+0x120/0x3e0

Cc: Guenter Roeck <linux@roeck-us.net>
Fixes: a19b4801457b ("bnxt_en: Event handler for Thermal event")
Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c       | 17 ++++++++++++-----
 drivers/net/ethernet/broadcom/bnxt/bnxt.h       |  2 ++
 drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c |  4 ++--
 drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.h |  4 ++--
 4 files changed, 18 insertions(+), 9 deletions(-)

Comments

Guenter Roeck Oct. 22, 2023, 3:51 p.m. UTC | #1 
On 10/20/23 14:27, Michael Chan wrote:
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> 
> Defer hwmon_notify_event() to bnxt_sp_task() workqueue because
> hwmon_notify_event() can try to acquire a mutex shown in the stack trace
> below.  Modify bnxt_event_error_report() to return true if we need to
> schedule bnxt_sp_task() to notify hwmon.
> 
>    __schedule+0x68/0x520
>    hwmon_notify_event+0xe8/0x114
>    schedule+0x60/0xe0
>    schedule_preempt_disabled+0x28/0x40
>    __mutex_lock.constprop.0+0x534/0x550
>    __mutex_lock_slowpath+0x18/0x20
>    mutex_lock+0x5c/0x70
>    kobject_uevent_env+0x2f4/0x3d0
>    kobject_uevent+0x10/0x20
>    hwmon_notify_event+0x94/0x114
>    bnxt_hwmon_notify_event+0x40/0x70 [bnxt_en]
>    bnxt_event_error_report+0x260/0x290 [bnxt_en]
>    bnxt_async_event_process.isra.0+0x250/0x850 [bnxt_en]
>    bnxt_hwrm_handler.isra.0+0xc8/0x120 [bnxt_en]
>    bnxt_poll_p5+0x150/0x350 [bnxt_en]
>    __napi_poll+0x3c/0x210
>    net_rx_action+0x308/0x3b0
>    __do_softirq+0x120/0x3e0
> 
> Cc: Guenter Roeck <linux@roeck-us.net>
> Fixes: a19b4801457b ("bnxt_en: Event handler for Thermal event")
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/net/ethernet/broadcom/bnxt/bnxt.c       | 17 ++++++++++++-----
>   drivers/net/ethernet/broadcom/bnxt/bnxt.h       |  2 ++
>   drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c |  4 ++--
>   drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.h |  4 ++--
>   4 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 16eb7a7af970..7837e22f237b 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -2147,7 +2147,8 @@ static u16 bnxt_agg_ring_id_to_grp_idx(struct bnxt *bp, u16 ring_id)
>   	  ASYNC_EVENT_CMPL_ERROR_REPORT_THERMAL_EVENT_DATA1_TRANSITION_DIR) ==\
>   	 ASYNC_EVENT_CMPL_ERROR_REPORT_THERMAL_EVENT_DATA1_TRANSITION_DIR_INCREASING)
>   
> -static void bnxt_event_error_report(struct bnxt *bp, u32 data1, u32 data2)
> +/* Return true if the workqueue has to be scheduled */
> +static bool bnxt_event_error_report(struct bnxt *bp, u32 data1, u32 data2)
>   {
>   	u32 err_type = BNXT_EVENT_ERROR_REPORT_TYPE(data1);
>   
> @@ -2182,7 +2183,7 @@ static void bnxt_event_error_report(struct bnxt *bp, u32 data1, u32 data2)
>   			break;
>   		default:
>   			netdev_err(bp->dev, "Unknown Thermal threshold type event\n");
> -			return;
> +			return false;
>   		}
>   		if (EVENT_DATA1_THERMAL_THRESHOLD_DIR_INCREASING(data1))
>   			dir_str = "above";
> @@ -2193,14 +2194,16 @@ static void bnxt_event_error_report(struct bnxt *bp, u32 data1, u32 data2)
>   		netdev_warn(bp->dev, "Temperature (In Celsius), Current: %lu, threshold: %lu\n",
>   			    BNXT_EVENT_THERMAL_CURRENT_TEMP(data2),
>   			    BNXT_EVENT_THERMAL_THRESHOLD_TEMP(data2));
> -		bnxt_hwmon_notify_event(bp, type);
> -		break;
> +		bp->thermal_threshold_type = type;
> +		set_bit(BNXT_THERMAL_THRESHOLD_SP_EVENT, &bp->sp_event);
> +		return true;
>   	}
>   	default:
>   		netdev_err(bp->dev, "FW reported unknown error type %u\n",
>   			   err_type);
>   		break;
>   	}
> +	return false;
>   }
>   
>   #define BNXT_GET_EVENT_PORT(data)	\
> @@ -2401,7 +2404,8 @@ static int bnxt_async_event_process(struct bnxt *bp,
>   		goto async_event_process_exit;
>   	}
>   	case ASYNC_EVENT_CMPL_EVENT_ID_ERROR_REPORT: {
> -		bnxt_event_error_report(bp, data1, data2);
> +		if (bnxt_event_error_report(bp, data1, data2))
> +			break;
>   		goto async_event_process_exit;
>   	}
>   	case ASYNC_EVENT_CMPL_EVENT_ID_PHC_UPDATE: {
> @@ -12085,6 +12089,9 @@ static void bnxt_sp_task(struct work_struct *work)
>   	if (test_and_clear_bit(BNXT_FW_ECHO_REQUEST_SP_EVENT, &bp->sp_event))
>   		bnxt_fw_echo_reply(bp);
>   
> +	if (test_and_clear_bit(BNXT_THERMAL_THRESHOLD_SP_EVENT, &bp->sp_event))
> +		bnxt_hwmon_notify_event(bp);
> +
>   	/* These functions below will clear BNXT_STATE_IN_SP_TASK.  They
>   	 * must be the last functions to be called before exiting.
>   	 */
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index 9ce0193798d4..80846c3ca9fc 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -2094,6 +2094,7 @@ struct bnxt {
>   #define BNXT_FW_RESET_NOTIFY_SP_EVENT	18
>   #define BNXT_FW_EXCEPTION_SP_EVENT	19
>   #define BNXT_LINK_CFG_CHANGE_SP_EVENT	21
> +#define BNXT_THERMAL_THRESHOLD_SP_EVENT	22
>   #define BNXT_FW_ECHO_REQUEST_SP_EVENT	23
>   
>   	struct delayed_work	fw_reset_task;
> @@ -2196,6 +2197,7 @@ struct bnxt {
>   	u8			fatal_thresh_temp;
>   	u8			shutdown_thresh_temp;
>   #endif
> +	u32			thermal_threshold_type;
>   	enum board_idx		board_idx;
>   };
>   
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c
> index e48094043c3b..669d24ba0e87 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c
> @@ -18,14 +18,14 @@
>   #include "bnxt_hwrm.h"
>   #include "bnxt_hwmon.h"
>   
> -void bnxt_hwmon_notify_event(struct bnxt *bp, u32 type)
> +void bnxt_hwmon_notify_event(struct bnxt *bp)
>   {
>   	u32 attr;
>   
>   	if (!bp->hwmon_dev)
>   		return;
>   
> -	switch (type) {
> +	switch (bp->thermal_threshold_type) {
>   	case ASYNC_EVENT_CMPL_ERROR_REPORT_THERMAL_EVENT_DATA1_THRESHOLD_TYPE_WARN:
>   		attr = hwmon_temp_max_alarm;
>   		break;
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.h
> index 76d9f599ebc0..de54a562e06a 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.h
> @@ -11,11 +11,11 @@
>   #define BNXT_HWMON_H
>   
>   #ifdef CONFIG_BNXT_HWMON
> -void bnxt_hwmon_notify_event(struct bnxt *bp, u32 type);
> +void bnxt_hwmon_notify_event(struct bnxt *bp);
>   void bnxt_hwmon_uninit(struct bnxt *bp);
>   void bnxt_hwmon_init(struct bnxt *bp);
>   #else
> -static inline void bnxt_hwmon_notify_event(struct bnxt *bp, u32 type)
> +static inline void bnxt_hwmon_notify_event(struct bnxt *bp)
>   {
>   }
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 16eb7a7af970..7837e22f237b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -2147,7 +2147,8 @@  static u16 bnxt_agg_ring_id_to_grp_idx(struct bnxt *bp, u16 ring_id)
 	  ASYNC_EVENT_CMPL_ERROR_REPORT_THERMAL_EVENT_DATA1_TRANSITION_DIR) ==\
 	 ASYNC_EVENT_CMPL_ERROR_REPORT_THERMAL_EVENT_DATA1_TRANSITION_DIR_INCREASING)
 
-static void bnxt_event_error_report(struct bnxt *bp, u32 data1, u32 data2)
+/* Return true if the workqueue has to be scheduled */
+static bool bnxt_event_error_report(struct bnxt *bp, u32 data1, u32 data2)
 {
 	u32 err_type = BNXT_EVENT_ERROR_REPORT_TYPE(data1);
 
@@ -2182,7 +2183,7 @@  static void bnxt_event_error_report(struct bnxt *bp, u32 data1, u32 data2)
 			break;
 		default:
 			netdev_err(bp->dev, "Unknown Thermal threshold type event\n");
-			return;
+			return false;
 		}
 		if (EVENT_DATA1_THERMAL_THRESHOLD_DIR_INCREASING(data1))
 			dir_str = "above";
@@ -2193,14 +2194,16 @@  static void bnxt_event_error_report(struct bnxt *bp, u32 data1, u32 data2)
 		netdev_warn(bp->dev, "Temperature (In Celsius), Current: %lu, threshold: %lu\n",
 			    BNXT_EVENT_THERMAL_CURRENT_TEMP(data2),
 			    BNXT_EVENT_THERMAL_THRESHOLD_TEMP(data2));
-		bnxt_hwmon_notify_event(bp, type);
-		break;
+		bp->thermal_threshold_type = type;
+		set_bit(BNXT_THERMAL_THRESHOLD_SP_EVENT, &bp->sp_event);
+		return true;
 	}
 	default:
 		netdev_err(bp->dev, "FW reported unknown error type %u\n",
 			   err_type);
 		break;
 	}
+	return false;
 }
 
 #define BNXT_GET_EVENT_PORT(data)	\
@@ -2401,7 +2404,8 @@  static int bnxt_async_event_process(struct bnxt *bp,
 		goto async_event_process_exit;
 	}
 	case ASYNC_EVENT_CMPL_EVENT_ID_ERROR_REPORT: {
-		bnxt_event_error_report(bp, data1, data2);
+		if (bnxt_event_error_report(bp, data1, data2))
+			break;
 		goto async_event_process_exit;
 	}
 	case ASYNC_EVENT_CMPL_EVENT_ID_PHC_UPDATE: {
@@ -12085,6 +12089,9 @@  static void bnxt_sp_task(struct work_struct *work)
 	if (test_and_clear_bit(BNXT_FW_ECHO_REQUEST_SP_EVENT, &bp->sp_event))
 		bnxt_fw_echo_reply(bp);
 
+	if (test_and_clear_bit(BNXT_THERMAL_THRESHOLD_SP_EVENT, &bp->sp_event))
+		bnxt_hwmon_notify_event(bp);
+
 	/* These functions below will clear BNXT_STATE_IN_SP_TASK.  They
 	 * must be the last functions to be called before exiting.
 	 */
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 9ce0193798d4..80846c3ca9fc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2094,6 +2094,7 @@  struct bnxt {
 #define BNXT_FW_RESET_NOTIFY_SP_EVENT	18
 #define BNXT_FW_EXCEPTION_SP_EVENT	19
 #define BNXT_LINK_CFG_CHANGE_SP_EVENT	21
+#define BNXT_THERMAL_THRESHOLD_SP_EVENT	22
 #define BNXT_FW_ECHO_REQUEST_SP_EVENT	23
 
 	struct delayed_work	fw_reset_task;
@@ -2196,6 +2197,7 @@  struct bnxt {
 	u8			fatal_thresh_temp;
 	u8			shutdown_thresh_temp;
 #endif
+	u32			thermal_threshold_type;
 	enum board_idx		board_idx;
 };
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c
index e48094043c3b..669d24ba0e87 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.c
@@ -18,14 +18,14 @@ 
 #include "bnxt_hwrm.h"
 #include "bnxt_hwmon.h"
 
-void bnxt_hwmon_notify_event(struct bnxt *bp, u32 type)
+void bnxt_hwmon_notify_event(struct bnxt *bp)
 {
 	u32 attr;
 
 	if (!bp->hwmon_dev)
 		return;
 
-	switch (type) {
+	switch (bp->thermal_threshold_type) {
 	case ASYNC_EVENT_CMPL_ERROR_REPORT_THERMAL_EVENT_DATA1_THRESHOLD_TYPE_WARN:
 		attr = hwmon_temp_max_alarm;
 		break;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.h
index 76d9f599ebc0..de54a562e06a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwmon.h
@@ -11,11 +11,11 @@ 
 #define BNXT_HWMON_H
 
 #ifdef CONFIG_BNXT_HWMON
-void bnxt_hwmon_notify_event(struct bnxt *bp, u32 type);
+void bnxt_hwmon_notify_event(struct bnxt *bp);
 void bnxt_hwmon_uninit(struct bnxt *bp);
 void bnxt_hwmon_init(struct bnxt *bp);
 #else
-static inline void bnxt_hwmon_notify_event(struct bnxt *bp, u32 type)
+static inline void bnxt_hwmon_notify_event(struct bnxt *bp)
 {
 }