All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/6] Support hld delayed init based on Pseudo-NMI for arm64
@ 2022-04-27 16:13 ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang


As we already used hld internally for arm64 since 2020, there still
doesn't have a proper commit on the upstream and we badly need it.

This serise rework on 5.17 from [1] and the origin author is
Pingfan Liu <kernelfans@gmail.com>
Sumit Garg <sumit.garg@linaro.org>

Qoute from [1]:

> Hard lockup detector is helpful to diagnose unpaired irq enable/disable.
> But the current watchdog framework can not cope with arm64 hw perf event
> easily.

> On arm64, when lockup_detector_init()->watchdog_nmi_probe(), PMU is not
> ready until device_initcall(armv8_pmu_driver_init).  And it is deeply
> integrated with the driver model and cpuhp. Hence it is hard to push the
> initialization of armv8_pmu_driver_init() before smp_init().

> But it is easy to take an opposite approach by enabling watchdog_hld to
> get the capability of PMU async. 
> The async model is achieved by expanding watchdog_nmi_probe() with
> -EBUSY, and a re-initializing work_struct which waits on a
> wait_queue_head.

Provide an API - retry_lockup_detector_init() for anyone who needs
to delayed init lockup detector.

The original assumption is: nobody should use delayed probe after
lockup_detector_check() (which has __init attribute).
That is, anyone uses this API must call between lockup_detector_init()
and lockup_detector_check(), and the caller must have __init attribute

The delayed init flow is:
1. lockup_detector_init() -> watchdog_nmi_probe() get non-zero retun,
   then set allow_lockup_detector_init_retry to true which means it's
   able to do delayed probe later.

2. PMU arch code init done, call retry_lockup_detector_init().

3. retry_lockup_detector_init() queue the work only when
   allow_lockup_detector_init_retry is true which means nobody should call
   this before lockup_detector_init().

4. the work lockup_detector_delay_init() is doing without wait event.
   if probe success, set allow_lockup_detector_init_retry to false.

5. at late_initcall_sync(), lockup_detector_check() set
   allow_lockup_detector_init_retry to false first to avoid any later retry,
   and then flush_work() to make sure the __init section won't be freed
   before the work done.

[1] https://lore.kernel.org/lkml/20211014024155.15253-1-kernelfans@gmail.com/

v4:
  1. remove -EBUSY protocal, let all the non-zero value from
     watchdog_nmi_probe() be able to retry.
  2. separate arm64 part patch into hw_nmi_get_sample_period and retry
     delayed init
  3. tweak commit msg that we don't have to limit to -EBUSY  
  4. rebase on v5.18-rc4

v3:
  1. Tweak commit message in patch 04 
	2. Remove wait event
  3. s/lockup_detector_pending_init/allow_lockup_detector_init_retry/
  4. provide api retry_lockup_detector_init() 
https://lore.kernel.org/lkml/20220324141405.10835-1-lecopzer.chen@mediatek.com/ 

v2:
  1. Tweak commit message in patch 01/02/04/05 
  2. Remove vobose WARN in patch 04 within watchdog core.
  3. Change from three states variable: detector_delay_init_state to
     two states variable: allow_lockup_detector_init_retry

     Thanks Petr Mladek <pmladek@suse.com> for the idea.
     > 1.  lockup_detector_work() called before lockup_detector_check().
     >     In this case, wait_event() will wait until lockup_detector_check()
     >     clears detector_delay_pending_init and calls wake_up().

     > 2. lockup_detector_check() called before lockup_detector_work().
     >    In this case, wait_even() will immediately continue because
     >    it will see cleared detector_delay_pending_init.
  4. Add comment in code in patch 04/05 for two states variable changing.
https://lore.kernel.org/lkml/20220307154729.13477-1-lecopzer.chen@mediatek.com/


Lecopzer Chen (5):
  kernel/watchdog: remove WATCHDOG_DEFAULT
  kernel/watchdog: change watchdog_nmi_enable() to void
  kernel/watchdog: Adapt the watchdog_hld interface for async model
  arm64: add hw_nmi_get_sample_period for preparation of lockup detector
  arm64: Enable perf events based hard lockup detector

Pingfan Liu (1):
  kernel/watchdog_hld: Ensure CPU-bound context when creating hardlockup
    detector event

 arch/arm64/Kconfig               |  2 +
 arch/arm64/kernel/Makefile       |  1 +
 arch/arm64/kernel/perf_event.c   | 10 ++++-
 arch/arm64/kernel/watchdog_hld.c | 39 +++++++++++++++++
 arch/sparc/kernel/nmi.c          |  8 ++--
 drivers/perf/arm_pmu.c           |  5 +++
 include/linux/nmi.h              |  4 +-
 include/linux/perf/arm_pmu.h     |  2 +
 kernel/watchdog.c                | 72 +++++++++++++++++++++++++++++---
 kernel/watchdog_hld.c            |  8 +++-
 10 files changed, 137 insertions(+), 14 deletions(-)
 create mode 100644 arch/arm64/kernel/watchdog_hld.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 0/6] Support hld delayed init based on Pseudo-NMI for arm64
@ 2022-04-27 16:13 ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang


As we already used hld internally for arm64 since 2020, there still
doesn't have a proper commit on the upstream and we badly need it.

This serise rework on 5.17 from [1] and the origin author is
Pingfan Liu <kernelfans@gmail.com>
Sumit Garg <sumit.garg@linaro.org>

Qoute from [1]:

> Hard lockup detector is helpful to diagnose unpaired irq enable/disable.
> But the current watchdog framework can not cope with arm64 hw perf event
> easily.

> On arm64, when lockup_detector_init()->watchdog_nmi_probe(), PMU is not
> ready until device_initcall(armv8_pmu_driver_init).  And it is deeply
> integrated with the driver model and cpuhp. Hence it is hard to push the
> initialization of armv8_pmu_driver_init() before smp_init().

> But it is easy to take an opposite approach by enabling watchdog_hld to
> get the capability of PMU async. 
> The async model is achieved by expanding watchdog_nmi_probe() with
> -EBUSY, and a re-initializing work_struct which waits on a
> wait_queue_head.

Provide an API - retry_lockup_detector_init() for anyone who needs
to delayed init lockup detector.

The original assumption is: nobody should use delayed probe after
lockup_detector_check() (which has __init attribute).
That is, anyone uses this API must call between lockup_detector_init()
and lockup_detector_check(), and the caller must have __init attribute

The delayed init flow is:
1. lockup_detector_init() -> watchdog_nmi_probe() get non-zero retun,
   then set allow_lockup_detector_init_retry to true which means it's
   able to do delayed probe later.

2. PMU arch code init done, call retry_lockup_detector_init().

3. retry_lockup_detector_init() queue the work only when
   allow_lockup_detector_init_retry is true which means nobody should call
   this before lockup_detector_init().

4. the work lockup_detector_delay_init() is doing without wait event.
   if probe success, set allow_lockup_detector_init_retry to false.

5. at late_initcall_sync(), lockup_detector_check() set
   allow_lockup_detector_init_retry to false first to avoid any later retry,
   and then flush_work() to make sure the __init section won't be freed
   before the work done.

[1] https://lore.kernel.org/lkml/20211014024155.15253-1-kernelfans@gmail.com/

v4:
  1. remove -EBUSY protocal, let all the non-zero value from
     watchdog_nmi_probe() be able to retry.
  2. separate arm64 part patch into hw_nmi_get_sample_period and retry
     delayed init
  3. tweak commit msg that we don't have to limit to -EBUSY  
  4. rebase on v5.18-rc4

v3:
  1. Tweak commit message in patch 04 
	2. Remove wait event
  3. s/lockup_detector_pending_init/allow_lockup_detector_init_retry/
  4. provide api retry_lockup_detector_init() 
https://lore.kernel.org/lkml/20220324141405.10835-1-lecopzer.chen@mediatek.com/ 

v2:
  1. Tweak commit message in patch 01/02/04/05 
  2. Remove vobose WARN in patch 04 within watchdog core.
  3. Change from three states variable: detector_delay_init_state to
     two states variable: allow_lockup_detector_init_retry

     Thanks Petr Mladek <pmladek@suse.com> for the idea.
     > 1.  lockup_detector_work() called before lockup_detector_check().
     >     In this case, wait_event() will wait until lockup_detector_check()
     >     clears detector_delay_pending_init and calls wake_up().

     > 2. lockup_detector_check() called before lockup_detector_work().
     >    In this case, wait_even() will immediately continue because
     >    it will see cleared detector_delay_pending_init.
  4. Add comment in code in patch 04/05 for two states variable changing.
https://lore.kernel.org/lkml/20220307154729.13477-1-lecopzer.chen@mediatek.com/


Lecopzer Chen (5):
  kernel/watchdog: remove WATCHDOG_DEFAULT
  kernel/watchdog: change watchdog_nmi_enable() to void
  kernel/watchdog: Adapt the watchdog_hld interface for async model
  arm64: add hw_nmi_get_sample_period for preparation of lockup detector
  arm64: Enable perf events based hard lockup detector

Pingfan Liu (1):
  kernel/watchdog_hld: Ensure CPU-bound context when creating hardlockup
    detector event

 arch/arm64/Kconfig               |  2 +
 arch/arm64/kernel/Makefile       |  1 +
 arch/arm64/kernel/perf_event.c   | 10 ++++-
 arch/arm64/kernel/watchdog_hld.c | 39 +++++++++++++++++
 arch/sparc/kernel/nmi.c          |  8 ++--
 drivers/perf/arm_pmu.c           |  5 +++
 include/linux/nmi.h              |  4 +-
 include/linux/perf/arm_pmu.h     |  2 +
 kernel/watchdog.c                | 72 +++++++++++++++++++++++++++++---
 kernel/watchdog_hld.c            |  8 +++-
 10 files changed, 137 insertions(+), 14 deletions(-)
 create mode 100644 arch/arm64/kernel/watchdog_hld.c

-- 
2.25.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 0/6] Support hld delayed init based on Pseudo-NMI for arm64
@ 2022-04-27 16:13 ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang


As we already used hld internally for arm64 since 2020, there still
doesn't have a proper commit on the upstream and we badly need it.

This serise rework on 5.17 from [1] and the origin author is
Pingfan Liu <kernelfans@gmail.com>
Sumit Garg <sumit.garg@linaro.org>

Qoute from [1]:

> Hard lockup detector is helpful to diagnose unpaired irq enable/disable.
> But the current watchdog framework can not cope with arm64 hw perf event
> easily.

> On arm64, when lockup_detector_init()->watchdog_nmi_probe(), PMU is not
> ready until device_initcall(armv8_pmu_driver_init).  And it is deeply
> integrated with the driver model and cpuhp. Hence it is hard to push the
> initialization of armv8_pmu_driver_init() before smp_init().

> But it is easy to take an opposite approach by enabling watchdog_hld to
> get the capability of PMU async. 
> The async model is achieved by expanding watchdog_nmi_probe() with
> -EBUSY, and a re-initializing work_struct which waits on a
> wait_queue_head.

Provide an API - retry_lockup_detector_init() for anyone who needs
to delayed init lockup detector.

The original assumption is: nobody should use delayed probe after
lockup_detector_check() (which has __init attribute).
That is, anyone uses this API must call between lockup_detector_init()
and lockup_detector_check(), and the caller must have __init attribute

The delayed init flow is:
1. lockup_detector_init() -> watchdog_nmi_probe() get non-zero retun,
   then set allow_lockup_detector_init_retry to true which means it's
   able to do delayed probe later.

2. PMU arch code init done, call retry_lockup_detector_init().

3. retry_lockup_detector_init() queue the work only when
   allow_lockup_detector_init_retry is true which means nobody should call
   this before lockup_detector_init().

4. the work lockup_detector_delay_init() is doing without wait event.
   if probe success, set allow_lockup_detector_init_retry to false.

5. at late_initcall_sync(), lockup_detector_check() set
   allow_lockup_detector_init_retry to false first to avoid any later retry,
   and then flush_work() to make sure the __init section won't be freed
   before the work done.

[1] https://lore.kernel.org/lkml/20211014024155.15253-1-kernelfans@gmail.com/

v4:
  1. remove -EBUSY protocal, let all the non-zero value from
     watchdog_nmi_probe() be able to retry.
  2. separate arm64 part patch into hw_nmi_get_sample_period and retry
     delayed init
  3. tweak commit msg that we don't have to limit to -EBUSY  
  4. rebase on v5.18-rc4

v3:
  1. Tweak commit message in patch 04 
	2. Remove wait event
  3. s/lockup_detector_pending_init/allow_lockup_detector_init_retry/
  4. provide api retry_lockup_detector_init() 
https://lore.kernel.org/lkml/20220324141405.10835-1-lecopzer.chen@mediatek.com/ 

v2:
  1. Tweak commit message in patch 01/02/04/05 
  2. Remove vobose WARN in patch 04 within watchdog core.
  3. Change from three states variable: detector_delay_init_state to
     two states variable: allow_lockup_detector_init_retry

     Thanks Petr Mladek <pmladek@suse.com> for the idea.
     > 1.  lockup_detector_work() called before lockup_detector_check().
     >     In this case, wait_event() will wait until lockup_detector_check()
     >     clears detector_delay_pending_init and calls wake_up().

     > 2. lockup_detector_check() called before lockup_detector_work().
     >    In this case, wait_even() will immediately continue because
     >    it will see cleared detector_delay_pending_init.
  4. Add comment in code in patch 04/05 for two states variable changing.
https://lore.kernel.org/lkml/20220307154729.13477-1-lecopzer.chen@mediatek.com/


Lecopzer Chen (5):
  kernel/watchdog: remove WATCHDOG_DEFAULT
  kernel/watchdog: change watchdog_nmi_enable() to void
  kernel/watchdog: Adapt the watchdog_hld interface for async model
  arm64: add hw_nmi_get_sample_period for preparation of lockup detector
  arm64: Enable perf events based hard lockup detector

Pingfan Liu (1):
  kernel/watchdog_hld: Ensure CPU-bound context when creating hardlockup
    detector event

 arch/arm64/Kconfig               |  2 +
 arch/arm64/kernel/Makefile       |  1 +
 arch/arm64/kernel/perf_event.c   | 10 ++++-
 arch/arm64/kernel/watchdog_hld.c | 39 +++++++++++++++++
 arch/sparc/kernel/nmi.c          |  8 ++--
 drivers/perf/arm_pmu.c           |  5 +++
 include/linux/nmi.h              |  4 +-
 include/linux/perf/arm_pmu.h     |  2 +
 kernel/watchdog.c                | 72 +++++++++++++++++++++++++++++---
 kernel/watchdog_hld.c            |  8 +++-
 10 files changed, 137 insertions(+), 14 deletions(-)
 create mode 100644 arch/arm64/kernel/watchdog_hld.c

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 1/6] kernel/watchdog: remove WATCHDOG_DEFAULT
  2022-04-27 16:13 ` Lecopzer Chen
  (?)
@ 2022-04-27 16:13   ` Lecopzer Chen
  -1 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

No reference to WATCHDOG_DEFAULT, remove it.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 kernel/watchdog.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 9166220457bc..e932331826eb 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -30,10 +30,8 @@
 static DEFINE_MUTEX(watchdog_mutex);
 
 #if defined(CONFIG_HARDLOCKUP_DETECTOR) || defined(CONFIG_HAVE_NMI_WATCHDOG)
-# define WATCHDOG_DEFAULT	(SOFT_WATCHDOG_ENABLED | NMI_WATCHDOG_ENABLED)
 # define NMI_WATCHDOG_DEFAULT	1
 #else
-# define WATCHDOG_DEFAULT	(SOFT_WATCHDOG_ENABLED)
 # define NMI_WATCHDOG_DEFAULT	0
 #endif
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 1/6] kernel/watchdog: remove WATCHDOG_DEFAULT
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

No reference to WATCHDOG_DEFAULT, remove it.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 kernel/watchdog.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 9166220457bc..e932331826eb 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -30,10 +30,8 @@
 static DEFINE_MUTEX(watchdog_mutex);
 
 #if defined(CONFIG_HARDLOCKUP_DETECTOR) || defined(CONFIG_HAVE_NMI_WATCHDOG)
-# define WATCHDOG_DEFAULT	(SOFT_WATCHDOG_ENABLED | NMI_WATCHDOG_ENABLED)
 # define NMI_WATCHDOG_DEFAULT	1
 #else
-# define WATCHDOG_DEFAULT	(SOFT_WATCHDOG_ENABLED)
 # define NMI_WATCHDOG_DEFAULT	0
 #endif
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 1/6] kernel/watchdog: remove WATCHDOG_DEFAULT
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

No reference to WATCHDOG_DEFAULT, remove it.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 kernel/watchdog.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 9166220457bc..e932331826eb 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -30,10 +30,8 @@
 static DEFINE_MUTEX(watchdog_mutex);
 
 #if defined(CONFIG_HARDLOCKUP_DETECTOR) || defined(CONFIG_HAVE_NMI_WATCHDOG)
-# define WATCHDOG_DEFAULT	(SOFT_WATCHDOG_ENABLED | NMI_WATCHDOG_ENABLED)
 # define NMI_WATCHDOG_DEFAULT	1
 #else
-# define WATCHDOG_DEFAULT	(SOFT_WATCHDOG_ENABLED)
 # define NMI_WATCHDOG_DEFAULT	0
 #endif
 
-- 
2.25.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 2/6] kernel/watchdog: change watchdog_nmi_enable() to void
  2022-04-27 16:13 ` Lecopzer Chen
  (?)
@ 2022-04-27 16:13   ` Lecopzer Chen
  -1 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

Nobody cares about the return value of watchdog_nmi_enable(),
changing its prototype to void.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 arch/sparc/kernel/nmi.c | 8 +++-----
 include/linux/nmi.h     | 2 +-
 kernel/watchdog.c       | 3 +--
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/sparc/kernel/nmi.c b/arch/sparc/kernel/nmi.c
index 060fff95a305..5dcf31f7e81f 100644
--- a/arch/sparc/kernel/nmi.c
+++ b/arch/sparc/kernel/nmi.c
@@ -282,11 +282,11 @@ __setup("nmi_watchdog=", setup_nmi_watchdog);
  * sparc specific NMI watchdog enable function.
  * Enables watchdog if it is not enabled already.
  */
-int watchdog_nmi_enable(unsigned int cpu)
+void watchdog_nmi_enable(unsigned int cpu)
 {
 	if (atomic_read(&nmi_active) == -1) {
 		pr_warn("NMI watchdog cannot be enabled or disabled\n");
-		return -1;
+		return;
 	}
 
 	/*
@@ -295,11 +295,9 @@ int watchdog_nmi_enable(unsigned int cpu)
 	 * process first.
 	 */
 	if (!nmi_init_done)
-		return 0;
+		return;
 
 	smp_call_function_single(cpu, start_nmi_watchdog, NULL, 1);
-
-	return 0;
 }
 /*
  * sparc specific NMI watchdog disable function.
diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 750c7f395ca9..b7bcd63c36b4 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -119,7 +119,7 @@ static inline int hardlockup_detector_perf_init(void) { return 0; }
 void watchdog_nmi_stop(void);
 void watchdog_nmi_start(void);
 int watchdog_nmi_probe(void);
-int watchdog_nmi_enable(unsigned int cpu);
+void watchdog_nmi_enable(unsigned int cpu);
 void watchdog_nmi_disable(unsigned int cpu);
 
 /**
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index e932331826eb..ca85ca5fdeff 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -93,10 +93,9 @@ __setup("nmi_watchdog=", hardlockup_panic_setup);
  * softlockup watchdog start and stop. The arch must select the
  * SOFTLOCKUP_DETECTOR Kconfig.
  */
-int __weak watchdog_nmi_enable(unsigned int cpu)
+void __weak watchdog_nmi_enable(unsigned int cpu)
 {
 	hardlockup_detector_perf_enable();
-	return 0;
 }
 
 void __weak watchdog_nmi_disable(unsigned int cpu)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 2/6] kernel/watchdog: change watchdog_nmi_enable() to void
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

Nobody cares about the return value of watchdog_nmi_enable(),
changing its prototype to void.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 arch/sparc/kernel/nmi.c | 8 +++-----
 include/linux/nmi.h     | 2 +-
 kernel/watchdog.c       | 3 +--
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/sparc/kernel/nmi.c b/arch/sparc/kernel/nmi.c
index 060fff95a305..5dcf31f7e81f 100644
--- a/arch/sparc/kernel/nmi.c
+++ b/arch/sparc/kernel/nmi.c
@@ -282,11 +282,11 @@ __setup("nmi_watchdog=", setup_nmi_watchdog);
  * sparc specific NMI watchdog enable function.
  * Enables watchdog if it is not enabled already.
  */
-int watchdog_nmi_enable(unsigned int cpu)
+void watchdog_nmi_enable(unsigned int cpu)
 {
 	if (atomic_read(&nmi_active) == -1) {
 		pr_warn("NMI watchdog cannot be enabled or disabled\n");
-		return -1;
+		return;
 	}
 
 	/*
@@ -295,11 +295,9 @@ int watchdog_nmi_enable(unsigned int cpu)
 	 * process first.
 	 */
 	if (!nmi_init_done)
-		return 0;
+		return;
 
 	smp_call_function_single(cpu, start_nmi_watchdog, NULL, 1);
-
-	return 0;
 }
 /*
  * sparc specific NMI watchdog disable function.
diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 750c7f395ca9..b7bcd63c36b4 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -119,7 +119,7 @@ static inline int hardlockup_detector_perf_init(void) { return 0; }
 void watchdog_nmi_stop(void);
 void watchdog_nmi_start(void);
 int watchdog_nmi_probe(void);
-int watchdog_nmi_enable(unsigned int cpu);
+void watchdog_nmi_enable(unsigned int cpu);
 void watchdog_nmi_disable(unsigned int cpu);
 
 /**
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index e932331826eb..ca85ca5fdeff 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -93,10 +93,9 @@ __setup("nmi_watchdog=", hardlockup_panic_setup);
  * softlockup watchdog start and stop. The arch must select the
  * SOFTLOCKUP_DETECTOR Kconfig.
  */
-int __weak watchdog_nmi_enable(unsigned int cpu)
+void __weak watchdog_nmi_enable(unsigned int cpu)
 {
 	hardlockup_detector_perf_enable();
-	return 0;
 }
 
 void __weak watchdog_nmi_disable(unsigned int cpu)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 2/6] kernel/watchdog: change watchdog_nmi_enable() to void
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

Nobody cares about the return value of watchdog_nmi_enable(),
changing its prototype to void.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 arch/sparc/kernel/nmi.c | 8 +++-----
 include/linux/nmi.h     | 2 +-
 kernel/watchdog.c       | 3 +--
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/sparc/kernel/nmi.c b/arch/sparc/kernel/nmi.c
index 060fff95a305..5dcf31f7e81f 100644
--- a/arch/sparc/kernel/nmi.c
+++ b/arch/sparc/kernel/nmi.c
@@ -282,11 +282,11 @@ __setup("nmi_watchdog=", setup_nmi_watchdog);
  * sparc specific NMI watchdog enable function.
  * Enables watchdog if it is not enabled already.
  */
-int watchdog_nmi_enable(unsigned int cpu)
+void watchdog_nmi_enable(unsigned int cpu)
 {
 	if (atomic_read(&nmi_active) == -1) {
 		pr_warn("NMI watchdog cannot be enabled or disabled\n");
-		return -1;
+		return;
 	}
 
 	/*
@@ -295,11 +295,9 @@ int watchdog_nmi_enable(unsigned int cpu)
 	 * process first.
 	 */
 	if (!nmi_init_done)
-		return 0;
+		return;
 
 	smp_call_function_single(cpu, start_nmi_watchdog, NULL, 1);
-
-	return 0;
 }
 /*
  * sparc specific NMI watchdog disable function.
diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 750c7f395ca9..b7bcd63c36b4 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -119,7 +119,7 @@ static inline int hardlockup_detector_perf_init(void) { return 0; }
 void watchdog_nmi_stop(void);
 void watchdog_nmi_start(void);
 int watchdog_nmi_probe(void);
-int watchdog_nmi_enable(unsigned int cpu);
+void watchdog_nmi_enable(unsigned int cpu);
 void watchdog_nmi_disable(unsigned int cpu);
 
 /**
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index e932331826eb..ca85ca5fdeff 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -93,10 +93,9 @@ __setup("nmi_watchdog=", hardlockup_panic_setup);
  * softlockup watchdog start and stop. The arch must select the
  * SOFTLOCKUP_DETECTOR Kconfig.
  */
-int __weak watchdog_nmi_enable(unsigned int cpu)
+void __weak watchdog_nmi_enable(unsigned int cpu)
 {
 	hardlockup_detector_perf_enable();
-	return 0;
 }
 
 void __weak watchdog_nmi_disable(unsigned int cpu)
-- 
2.25.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 3/6] kernel/watchdog_hld: Ensure CPU-bound context when creating hardlockup detector event
  2022-04-27 16:13 ` Lecopzer Chen
  (?)
@ 2022-04-27 16:13   ` Lecopzer Chen
  -1 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

From: Pingfan Liu <kernelfans@gmail.com>

hardlockup_detector_event_create() should create perf_event on the
current CPU. Preemption could not get disabled because
perf_event_create_kernel_counter() allocates memory. Instead,
the CPU locality is achieved by processing the code in a per-CPU
bound kthread.

Add a check to prevent mistakes when calling the code in another
code path.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Co-developed-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 kernel/watchdog_hld.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
index 247bf0b1582c..96b717205952 100644
--- a/kernel/watchdog_hld.c
+++ b/kernel/watchdog_hld.c
@@ -165,10 +165,16 @@ static void watchdog_overflow_callback(struct perf_event *event,
 
 static int hardlockup_detector_event_create(void)
 {
-	unsigned int cpu = smp_processor_id();
+	unsigned int cpu;
 	struct perf_event_attr *wd_attr;
 	struct perf_event *evt;
 
+	/*
+	 * Preemption is not disabled because memory will be allocated.
+	 * Ensure CPU-locality by calling this in per-CPU kthread.
+	 */
+	WARN_ON(!is_percpu_thread());
+	cpu = raw_smp_processor_id();
 	wd_attr = &wd_hw_attr;
 	wd_attr->sample_period = hw_nmi_get_sample_period(watchdog_thresh);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 3/6] kernel/watchdog_hld: Ensure CPU-bound context when creating hardlockup detector event
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

From: Pingfan Liu <kernelfans@gmail.com>

hardlockup_detector_event_create() should create perf_event on the
current CPU. Preemption could not get disabled because
perf_event_create_kernel_counter() allocates memory. Instead,
the CPU locality is achieved by processing the code in a per-CPU
bound kthread.

Add a check to prevent mistakes when calling the code in another
code path.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Co-developed-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 kernel/watchdog_hld.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
index 247bf0b1582c..96b717205952 100644
--- a/kernel/watchdog_hld.c
+++ b/kernel/watchdog_hld.c
@@ -165,10 +165,16 @@ static void watchdog_overflow_callback(struct perf_event *event,
 
 static int hardlockup_detector_event_create(void)
 {
-	unsigned int cpu = smp_processor_id();
+	unsigned int cpu;
 	struct perf_event_attr *wd_attr;
 	struct perf_event *evt;
 
+	/*
+	 * Preemption is not disabled because memory will be allocated.
+	 * Ensure CPU-locality by calling this in per-CPU kthread.
+	 */
+	WARN_ON(!is_percpu_thread());
+	cpu = raw_smp_processor_id();
 	wd_attr = &wd_hw_attr;
 	wd_attr->sample_period = hw_nmi_get_sample_period(watchdog_thresh);
 
-- 
2.25.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 3/6] kernel/watchdog_hld: Ensure CPU-bound context when creating hardlockup detector event
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

From: Pingfan Liu <kernelfans@gmail.com>

hardlockup_detector_event_create() should create perf_event on the
current CPU. Preemption could not get disabled because
perf_event_create_kernel_counter() allocates memory. Instead,
the CPU locality is achieved by processing the code in a per-CPU
bound kthread.

Add a check to prevent mistakes when calling the code in another
code path.

Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Co-developed-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 kernel/watchdog_hld.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c
index 247bf0b1582c..96b717205952 100644
--- a/kernel/watchdog_hld.c
+++ b/kernel/watchdog_hld.c
@@ -165,10 +165,16 @@ static void watchdog_overflow_callback(struct perf_event *event,
 
 static int hardlockup_detector_event_create(void)
 {
-	unsigned int cpu = smp_processor_id();
+	unsigned int cpu;
 	struct perf_event_attr *wd_attr;
 	struct perf_event *evt;
 
+	/*
+	 * Preemption is not disabled because memory will be allocated.
+	 * Ensure CPU-locality by calling this in per-CPU kthread.
+	 */
+	WARN_ON(!is_percpu_thread());
+	cpu = raw_smp_processor_id();
 	wd_attr = &wd_hw_attr;
 	wd_attr->sample_period = hw_nmi_get_sample_period(watchdog_thresh);
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 4/6] kernel/watchdog: Adapt the watchdog_hld interface for async model
  2022-04-27 16:13 ` Lecopzer Chen
  (?)
@ 2022-04-27 16:13   ` Lecopzer Chen
  -1 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

When lockup_detector_init()->watchdog_nmi_probe(), PMU may be not ready
yet. E.g. on arm64, PMU is not ready until
device_initcall(armv8_pmu_driver_init).  And it is deeply integrated
with the driver model and cpuhp. Hence it is hard to push this
initialization before smp_init().

But it is easy to take an opposite approach and try to initialize
the watchdog once again later.
The delayed probe is called using workqueues. It need to allocate
memory and must be proceed in a normal context.
The delayed probe is able to use if watchdog_nmi_probe() returns
non-zero which means the return code returned when PMU is not ready yet.

Provide an API - retry_lockup_detector_init() for anyone who needs
to delayed init lockup detector if they had ever failed at
lockup_detector_init().

The original assumption is: nobody should use delayed probe after
lockup_detector_check() which has __init attribute.
That is, anyone uses this API must call between lockup_detector_init()
and lockup_detector_check(), and the caller must have __init attribute

Co-developed-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Suggested-by: Petr Mladek <pmladek@suse.com>
---
 include/linux/nmi.h |  2 ++
 kernel/watchdog.c   | 67 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index b7bcd63c36b4..10f2a305fe0d 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -118,6 +118,8 @@ static inline int hardlockup_detector_perf_init(void) { return 0; }
 
 void watchdog_nmi_stop(void);
 void watchdog_nmi_start(void);
+
+void retry_lockup_detector_init(void);
 int watchdog_nmi_probe(void);
 void watchdog_nmi_enable(unsigned int cpu);
 void watchdog_nmi_disable(unsigned int cpu);
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index ca85ca5fdeff..1edc23af3b80 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -103,7 +103,13 @@ void __weak watchdog_nmi_disable(unsigned int cpu)
 	hardlockup_detector_perf_disable();
 }
 
-/* Return 0, if a NMI watchdog is available. Error code otherwise */
+/*
+ * Arch specific API.
+ *
+ * Return 0 when NMI watchdog is available, negative value otherwise.
+ * Note that the negative value means that a delayed probe might
+ * succeed later.
+ */
 int __weak __init watchdog_nmi_probe(void)
 {
 	return hardlockup_detector_perf_init();
@@ -831,6 +837,62 @@ static struct ctl_table watchdog_sysctls[] = {
 	{}
 };
 
+static void __init lockup_detector_delay_init(struct work_struct *work);
+static bool allow_lockup_detector_init_retry __initdata;
+
+static struct work_struct detector_work __initdata =
+		__WORK_INITIALIZER(detector_work, lockup_detector_delay_init);
+
+static void __init lockup_detector_delay_init(struct work_struct *work)
+{
+	int ret;
+
+	ret = watchdog_nmi_probe();
+	if (ret) {
+		pr_info("Delayed init of the lockup detector failed: %d\n", ret);
+		pr_info("Perf NMI watchdog permanently disabled\n");
+		return;
+	}
+
+	allow_lockup_detector_init_retry = false;
+
+	nmi_watchdog_available = true;
+	lockup_detector_setup();
+}
+
+/*
+ * retry_lockup_detector_init - retry init lockup detector if possible.
+ *
+ * Retry hardlockup detector init. It is useful when it requires some
+ * functionality that has to be initialized later on a particular
+ * platform.
+ */
+void __init retry_lockup_detector_init(void)
+{
+	/* Must be called before late init calls */
+	if (!allow_lockup_detector_init_retry)
+		return;
+
+	queue_work_on(__smp_processor_id(), system_wq, &detector_work);
+}
+
+/*
+ * Ensure that optional delayed hardlockup init is proceed before
+ * the init code and memory is freed.
+ */
+static int __init lockup_detector_check(void)
+{
+	/* Prevent any later retry. */
+	allow_lockup_detector_init_retry = false;
+
+	/* Make sure no work is pending. */
+	flush_work(&detector_work);
+
+	return 0;
+
+}
+late_initcall_sync(lockup_detector_check);
+
 static void __init watchdog_sysctl_init(void)
 {
 	register_sysctl_init("kernel", watchdog_sysctls);
@@ -849,6 +911,9 @@ void __init lockup_detector_init(void)
 
 	if (!watchdog_nmi_probe())
 		nmi_watchdog_available = true;
+	else
+		allow_lockup_detector_init_retry = true;
+
 	lockup_detector_setup();
 	watchdog_sysctl_init();
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 4/6] kernel/watchdog: Adapt the watchdog_hld interface for async model
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

When lockup_detector_init()->watchdog_nmi_probe(), PMU may be not ready
yet. E.g. on arm64, PMU is not ready until
device_initcall(armv8_pmu_driver_init).  And it is deeply integrated
with the driver model and cpuhp. Hence it is hard to push this
initialization before smp_init().

But it is easy to take an opposite approach and try to initialize
the watchdog once again later.
The delayed probe is called using workqueues. It need to allocate
memory and must be proceed in a normal context.
The delayed probe is able to use if watchdog_nmi_probe() returns
non-zero which means the return code returned when PMU is not ready yet.

Provide an API - retry_lockup_detector_init() for anyone who needs
to delayed init lockup detector if they had ever failed at
lockup_detector_init().

The original assumption is: nobody should use delayed probe after
lockup_detector_check() which has __init attribute.
That is, anyone uses this API must call between lockup_detector_init()
and lockup_detector_check(), and the caller must have __init attribute

Co-developed-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Suggested-by: Petr Mladek <pmladek@suse.com>
---
 include/linux/nmi.h |  2 ++
 kernel/watchdog.c   | 67 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index b7bcd63c36b4..10f2a305fe0d 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -118,6 +118,8 @@ static inline int hardlockup_detector_perf_init(void) { return 0; }
 
 void watchdog_nmi_stop(void);
 void watchdog_nmi_start(void);
+
+void retry_lockup_detector_init(void);
 int watchdog_nmi_probe(void);
 void watchdog_nmi_enable(unsigned int cpu);
 void watchdog_nmi_disable(unsigned int cpu);
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index ca85ca5fdeff..1edc23af3b80 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -103,7 +103,13 @@ void __weak watchdog_nmi_disable(unsigned int cpu)
 	hardlockup_detector_perf_disable();
 }
 
-/* Return 0, if a NMI watchdog is available. Error code otherwise */
+/*
+ * Arch specific API.
+ *
+ * Return 0 when NMI watchdog is available, negative value otherwise.
+ * Note that the negative value means that a delayed probe might
+ * succeed later.
+ */
 int __weak __init watchdog_nmi_probe(void)
 {
 	return hardlockup_detector_perf_init();
@@ -831,6 +837,62 @@ static struct ctl_table watchdog_sysctls[] = {
 	{}
 };
 
+static void __init lockup_detector_delay_init(struct work_struct *work);
+static bool allow_lockup_detector_init_retry __initdata;
+
+static struct work_struct detector_work __initdata =
+		__WORK_INITIALIZER(detector_work, lockup_detector_delay_init);
+
+static void __init lockup_detector_delay_init(struct work_struct *work)
+{
+	int ret;
+
+	ret = watchdog_nmi_probe();
+	if (ret) {
+		pr_info("Delayed init of the lockup detector failed: %d\n", ret);
+		pr_info("Perf NMI watchdog permanently disabled\n");
+		return;
+	}
+
+	allow_lockup_detector_init_retry = false;
+
+	nmi_watchdog_available = true;
+	lockup_detector_setup();
+}
+
+/*
+ * retry_lockup_detector_init - retry init lockup detector if possible.
+ *
+ * Retry hardlockup detector init. It is useful when it requires some
+ * functionality that has to be initialized later on a particular
+ * platform.
+ */
+void __init retry_lockup_detector_init(void)
+{
+	/* Must be called before late init calls */
+	if (!allow_lockup_detector_init_retry)
+		return;
+
+	queue_work_on(__smp_processor_id(), system_wq, &detector_work);
+}
+
+/*
+ * Ensure that optional delayed hardlockup init is proceed before
+ * the init code and memory is freed.
+ */
+static int __init lockup_detector_check(void)
+{
+	/* Prevent any later retry. */
+	allow_lockup_detector_init_retry = false;
+
+	/* Make sure no work is pending. */
+	flush_work(&detector_work);
+
+	return 0;
+
+}
+late_initcall_sync(lockup_detector_check);
+
 static void __init watchdog_sysctl_init(void)
 {
 	register_sysctl_init("kernel", watchdog_sysctls);
@@ -849,6 +911,9 @@ void __init lockup_detector_init(void)
 
 	if (!watchdog_nmi_probe())
 		nmi_watchdog_available = true;
+	else
+		allow_lockup_detector_init_retry = true;
+
 	lockup_detector_setup();
 	watchdog_sysctl_init();
 }
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 4/6] kernel/watchdog: Adapt the watchdog_hld interface for async model
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

When lockup_detector_init()->watchdog_nmi_probe(), PMU may be not ready
yet. E.g. on arm64, PMU is not ready until
device_initcall(armv8_pmu_driver_init).  And it is deeply integrated
with the driver model and cpuhp. Hence it is hard to push this
initialization before smp_init().

But it is easy to take an opposite approach and try to initialize
the watchdog once again later.
The delayed probe is called using workqueues. It need to allocate
memory and must be proceed in a normal context.
The delayed probe is able to use if watchdog_nmi_probe() returns
non-zero which means the return code returned when PMU is not ready yet.

Provide an API - retry_lockup_detector_init() for anyone who needs
to delayed init lockup detector if they had ever failed at
lockup_detector_init().

The original assumption is: nobody should use delayed probe after
lockup_detector_check() which has __init attribute.
That is, anyone uses this API must call between lockup_detector_init()
and lockup_detector_check(), and the caller must have __init attribute

Co-developed-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Suggested-by: Petr Mladek <pmladek@suse.com>
---
 include/linux/nmi.h |  2 ++
 kernel/watchdog.c   | 67 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index b7bcd63c36b4..10f2a305fe0d 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -118,6 +118,8 @@ static inline int hardlockup_detector_perf_init(void) { return 0; }
 
 void watchdog_nmi_stop(void);
 void watchdog_nmi_start(void);
+
+void retry_lockup_detector_init(void);
 int watchdog_nmi_probe(void);
 void watchdog_nmi_enable(unsigned int cpu);
 void watchdog_nmi_disable(unsigned int cpu);
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index ca85ca5fdeff..1edc23af3b80 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -103,7 +103,13 @@ void __weak watchdog_nmi_disable(unsigned int cpu)
 	hardlockup_detector_perf_disable();
 }
 
-/* Return 0, if a NMI watchdog is available. Error code otherwise */
+/*
+ * Arch specific API.
+ *
+ * Return 0 when NMI watchdog is available, negative value otherwise.
+ * Note that the negative value means that a delayed probe might
+ * succeed later.
+ */
 int __weak __init watchdog_nmi_probe(void)
 {
 	return hardlockup_detector_perf_init();
@@ -831,6 +837,62 @@ static struct ctl_table watchdog_sysctls[] = {
 	{}
 };
 
+static void __init lockup_detector_delay_init(struct work_struct *work);
+static bool allow_lockup_detector_init_retry __initdata;
+
+static struct work_struct detector_work __initdata =
+		__WORK_INITIALIZER(detector_work, lockup_detector_delay_init);
+
+static void __init lockup_detector_delay_init(struct work_struct *work)
+{
+	int ret;
+
+	ret = watchdog_nmi_probe();
+	if (ret) {
+		pr_info("Delayed init of the lockup detector failed: %d\n", ret);
+		pr_info("Perf NMI watchdog permanently disabled\n");
+		return;
+	}
+
+	allow_lockup_detector_init_retry = false;
+
+	nmi_watchdog_available = true;
+	lockup_detector_setup();
+}
+
+/*
+ * retry_lockup_detector_init - retry init lockup detector if possible.
+ *
+ * Retry hardlockup detector init. It is useful when it requires some
+ * functionality that has to be initialized later on a particular
+ * platform.
+ */
+void __init retry_lockup_detector_init(void)
+{
+	/* Must be called before late init calls */
+	if (!allow_lockup_detector_init_retry)
+		return;
+
+	queue_work_on(__smp_processor_id(), system_wq, &detector_work);
+}
+
+/*
+ * Ensure that optional delayed hardlockup init is proceed before
+ * the init code and memory is freed.
+ */
+static int __init lockup_detector_check(void)
+{
+	/* Prevent any later retry. */
+	allow_lockup_detector_init_retry = false;
+
+	/* Make sure no work is pending. */
+	flush_work(&detector_work);
+
+	return 0;
+
+}
+late_initcall_sync(lockup_detector_check);
+
 static void __init watchdog_sysctl_init(void)
 {
 	register_sysctl_init("kernel", watchdog_sysctls);
@@ -849,6 +911,9 @@ void __init lockup_detector_init(void)
 
 	if (!watchdog_nmi_probe())
 		nmi_watchdog_available = true;
+	else
+		allow_lockup_detector_init_retry = true;
+
 	lockup_detector_setup();
 	watchdog_sysctl_init();
 }
-- 
2.25.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 5/6] arm64: add hw_nmi_get_sample_period for preparation of lockup detector
  2022-04-27 16:13 ` Lecopzer Chen
  (?)
@ 2022-04-27 16:13   ` Lecopzer Chen
  -1 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

Set safe maximum CPU frequency to 5 GHz in case a particular platform
doesn't implement cpufreq driver.
Although, architecture doesn't put any restrictions on
maximum frequency but 5 GHz seems to be safe maximum given the available
Arm CPUs in the market which are clocked much less than 5 GHz.

On the other hand, we can't make it much higher as it would lead to
a large hard-lockup detection timeout on parts which are running
slower (eg. 1GHz on Developerbox) and doesn't possess a cpufreq driver.

Co-developed-by: Sumit Garg <sumit.garg@linaro.org>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Co-developed-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
---
 arch/arm64/kernel/Makefile       |  1 +
 arch/arm64/kernel/watchdog_hld.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 arch/arm64/kernel/watchdog_hld.c

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 986837d7ec82..cd238b92d6d3 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_MODULES)			+= module.o
 obj-$(CONFIG_ARM64_MODULE_PLTS)		+= module-plts.o
 obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o perf_callchain.o
 obj-$(CONFIG_HW_PERF_EVENTS)		+= perf_event.o
+obj-$(CONFIG_HARDLOCKUP_DETECTOR_PERF)	+= watchdog_hld.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
 obj-$(CONFIG_CPU_PM)			+= sleep.o suspend.o
 obj-$(CONFIG_CPU_IDLE)			+= cpuidle.o
diff --git a/arch/arm64/kernel/watchdog_hld.c b/arch/arm64/kernel/watchdog_hld.c
new file mode 100644
index 000000000000..de43318e4dd6
--- /dev/null
+++ b/arch/arm64/kernel/watchdog_hld.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/cpufreq.h>
+
+/*
+ * Safe maximum CPU frequency in case a particular platform doesn't implement
+ * cpufreq driver. Although, architecture doesn't put any restrictions on
+ * maximum frequency but 5 GHz seems to be safe maximum given the available
+ * Arm CPUs in the market which are clocked much less than 5 GHz. On the other
+ * hand, we can't make it much higher as it would lead to a large hard-lockup
+ * detection timeout on parts which are running slower (eg. 1GHz on
+ * Developerbox) and doesn't possess a cpufreq driver.
+ */
+#define SAFE_MAX_CPU_FREQ	5000000000UL // 5 GHz
+u64 hw_nmi_get_sample_period(int watchdog_thresh)
+{
+	unsigned int cpu = smp_processor_id();
+	unsigned long max_cpu_freq;
+
+	max_cpu_freq = cpufreq_get_hw_max_freq(cpu) * 1000UL;
+	if (!max_cpu_freq)
+		max_cpu_freq = SAFE_MAX_CPU_FREQ;
+
+	return (u64)max_cpu_freq * watchdog_thresh;
+}
+
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 5/6] arm64: add hw_nmi_get_sample_period for preparation of lockup detector
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

Set safe maximum CPU frequency to 5 GHz in case a particular platform
doesn't implement cpufreq driver.
Although, architecture doesn't put any restrictions on
maximum frequency but 5 GHz seems to be safe maximum given the available
Arm CPUs in the market which are clocked much less than 5 GHz.

On the other hand, we can't make it much higher as it would lead to
a large hard-lockup detection timeout on parts which are running
slower (eg. 1GHz on Developerbox) and doesn't possess a cpufreq driver.

Co-developed-by: Sumit Garg <sumit.garg@linaro.org>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Co-developed-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
---
 arch/arm64/kernel/Makefile       |  1 +
 arch/arm64/kernel/watchdog_hld.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 arch/arm64/kernel/watchdog_hld.c

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 986837d7ec82..cd238b92d6d3 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_MODULES)			+= module.o
 obj-$(CONFIG_ARM64_MODULE_PLTS)		+= module-plts.o
 obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o perf_callchain.o
 obj-$(CONFIG_HW_PERF_EVENTS)		+= perf_event.o
+obj-$(CONFIG_HARDLOCKUP_DETECTOR_PERF)	+= watchdog_hld.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
 obj-$(CONFIG_CPU_PM)			+= sleep.o suspend.o
 obj-$(CONFIG_CPU_IDLE)			+= cpuidle.o
diff --git a/arch/arm64/kernel/watchdog_hld.c b/arch/arm64/kernel/watchdog_hld.c
new file mode 100644
index 000000000000..de43318e4dd6
--- /dev/null
+++ b/arch/arm64/kernel/watchdog_hld.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/cpufreq.h>
+
+/*
+ * Safe maximum CPU frequency in case a particular platform doesn't implement
+ * cpufreq driver. Although, architecture doesn't put any restrictions on
+ * maximum frequency but 5 GHz seems to be safe maximum given the available
+ * Arm CPUs in the market which are clocked much less than 5 GHz. On the other
+ * hand, we can't make it much higher as it would lead to a large hard-lockup
+ * detection timeout on parts which are running slower (eg. 1GHz on
+ * Developerbox) and doesn't possess a cpufreq driver.
+ */
+#define SAFE_MAX_CPU_FREQ	5000000000UL // 5 GHz
+u64 hw_nmi_get_sample_period(int watchdog_thresh)
+{
+	unsigned int cpu = smp_processor_id();
+	unsigned long max_cpu_freq;
+
+	max_cpu_freq = cpufreq_get_hw_max_freq(cpu) * 1000UL;
+	if (!max_cpu_freq)
+		max_cpu_freq = SAFE_MAX_CPU_FREQ;
+
+	return (u64)max_cpu_freq * watchdog_thresh;
+}
+
-- 
2.25.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 5/6] arm64: add hw_nmi_get_sample_period for preparation of lockup detector
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

Set safe maximum CPU frequency to 5 GHz in case a particular platform
doesn't implement cpufreq driver.
Although, architecture doesn't put any restrictions on
maximum frequency but 5 GHz seems to be safe maximum given the available
Arm CPUs in the market which are clocked much less than 5 GHz.

On the other hand, we can't make it much higher as it would lead to
a large hard-lockup detection timeout on parts which are running
slower (eg. 1GHz on Developerbox) and doesn't possess a cpufreq driver.

Co-developed-by: Sumit Garg <sumit.garg@linaro.org>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Co-developed-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
---
 arch/arm64/kernel/Makefile       |  1 +
 arch/arm64/kernel/watchdog_hld.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 arch/arm64/kernel/watchdog_hld.c

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 986837d7ec82..cd238b92d6d3 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_MODULES)			+= module.o
 obj-$(CONFIG_ARM64_MODULE_PLTS)		+= module-plts.o
 obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o perf_callchain.o
 obj-$(CONFIG_HW_PERF_EVENTS)		+= perf_event.o
+obj-$(CONFIG_HARDLOCKUP_DETECTOR_PERF)	+= watchdog_hld.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
 obj-$(CONFIG_CPU_PM)			+= sleep.o suspend.o
 obj-$(CONFIG_CPU_IDLE)			+= cpuidle.o
diff --git a/arch/arm64/kernel/watchdog_hld.c b/arch/arm64/kernel/watchdog_hld.c
new file mode 100644
index 000000000000..de43318e4dd6
--- /dev/null
+++ b/arch/arm64/kernel/watchdog_hld.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/cpufreq.h>
+
+/*
+ * Safe maximum CPU frequency in case a particular platform doesn't implement
+ * cpufreq driver. Although, architecture doesn't put any restrictions on
+ * maximum frequency but 5 GHz seems to be safe maximum given the available
+ * Arm CPUs in the market which are clocked much less than 5 GHz. On the other
+ * hand, we can't make it much higher as it would lead to a large hard-lockup
+ * detection timeout on parts which are running slower (eg. 1GHz on
+ * Developerbox) and doesn't possess a cpufreq driver.
+ */
+#define SAFE_MAX_CPU_FREQ	5000000000UL // 5 GHz
+u64 hw_nmi_get_sample_period(int watchdog_thresh)
+{
+	unsigned int cpu = smp_processor_id();
+	unsigned long max_cpu_freq;
+
+	max_cpu_freq = cpufreq_get_hw_max_freq(cpu) * 1000UL;
+	if (!max_cpu_freq)
+		max_cpu_freq = SAFE_MAX_CPU_FREQ;
+
+	return (u64)max_cpu_freq * watchdog_thresh;
+}
+
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 6/6] arm64: Enable perf events based hard lockup detector
  2022-04-27 16:13 ` Lecopzer Chen
  (?)
@ 2022-04-27 16:13   ` Lecopzer Chen
  -1 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

With the recent feature added to enable perf events to use pseudo NMIs
as interrupts on platforms which support GICv3 or later, its now been
possible to enable hard lockup detector (or NMI watchdog) on arm64
platforms. So enable corresponding support.

One thing to note here is that normally lockup detector is initialized
just after the early initcalls but PMU on arm64 comes up much later as
device_initcall(). To cope with that, overriding watchdog_nmi_probe() to
let the watchdog framework know PMU not ready, and inform the framework
to re-initialize lockup detection once PMU has been initialized.

[1]: http://lore.kernel.org/linux-arm-kernel/1610712101-14929-1-git-send-email-sumit.garg@linaro.org

Co-developed-by: Sumit Garg <sumit.garg@linaro.org>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Co-developed-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
---
 arch/arm64/Kconfig               |  2 ++
 arch/arm64/kernel/perf_event.c   | 10 ++++++++--
 arch/arm64/kernel/watchdog_hld.c | 14 ++++++++++++++
 drivers/perf/arm_pmu.c           |  5 +++++
 include/linux/perf/arm_pmu.h     |  2 ++
 5 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 20ea89d9ac2f..9d48fe4ce041 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -184,12 +184,14 @@ config ARM64
 	select HAVE_FUNCTION_ERROR_INJECTION
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_GCC_PLUGINS
+	select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI
 	select HAVE_HW_BREAKPOINT if PERF_EVENTS
 	select HAVE_IRQ_TIME_ACCOUNTING
 	select HAVE_KVM
 	select HAVE_NMI
 	select HAVE_PATA_PLATFORM
 	select HAVE_PERF_EVENTS
+	select HAVE_PERF_EVENTS_NMI if ARM64_PSEUDO_NMI
 	select HAVE_PERF_REGS
 	select HAVE_PERF_USER_STACK_DUMP
 	select HAVE_PREEMPT_DYNAMIC_KEY
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index cb69ff1e6138..d51ee09592d7 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/sched_clock.h>
 #include <linux/smp.h>
+#include <linux/nmi.h>
 
 /* ARMv8 Cortex-A53 specific event types. */
 #define ARMV8_A53_PERFCTR_PREF_LINEFILL				0xC2
@@ -1390,10 +1391,15 @@ static struct platform_driver armv8_pmu_driver = {
 
 static int __init armv8_pmu_driver_init(void)
 {
+	int ret;
+
 	if (acpi_disabled)
-		return platform_driver_register(&armv8_pmu_driver);
+		ret = platform_driver_register(&armv8_pmu_driver);
 	else
-		return arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
+		ret = arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
+
+	retry_lockup_detector_init();
+	return ret;
 }
 device_initcall(armv8_pmu_driver_init)
 
diff --git a/arch/arm64/kernel/watchdog_hld.c b/arch/arm64/kernel/watchdog_hld.c
index de43318e4dd6..c9c6ec889c15 100644
--- a/arch/arm64/kernel/watchdog_hld.c
+++ b/arch/arm64/kernel/watchdog_hld.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/nmi.h>
 #include <linux/cpufreq.h>
+#include <linux/perf/arm_pmu.h>
 
 /*
  * Safe maximum CPU frequency in case a particular platform doesn't implement
@@ -23,3 +25,15 @@ u64 hw_nmi_get_sample_period(int watchdog_thresh)
 	return (u64)max_cpu_freq * watchdog_thresh;
 }
 
+int __init watchdog_nmi_probe(void)
+{
+	/*
+	 * hardlockup_detector_perf_init() will success even if Pseudo-NMI turns off,
+	 * however, the pmu interrupts will act like a normal interrupt instead of
+	 * NMI and the hardlockup detector would be broken.
+	 */
+	if (!arm_pmu_irq_is_nmi())
+		return -ENODEV;
+
+	return hardlockup_detector_perf_init();
+}
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 59d3980b8ca2..ceee2c55d436 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -697,6 +697,11 @@ static int armpmu_get_cpu_irq(struct arm_pmu *pmu, int cpu)
 	return per_cpu(hw_events->irq, cpu);
 }
 
+bool arm_pmu_irq_is_nmi(void)
+{
+	return has_nmi;
+}
+
 /*
  * PMU hardware loses all context when a CPU goes offline.
  * When a CPU is hotplugged back in, since some hardware registers are
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 0407a38b470a..29c56c92bab7 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -171,6 +171,8 @@ void kvm_host_pmu_init(struct arm_pmu *pmu);
 #define kvm_host_pmu_init(x)	do { } while(0)
 #endif
 
+bool arm_pmu_irq_is_nmi(void);
+
 /* Internal functions only for core arm_pmu code */
 struct arm_pmu *armpmu_alloc(void);
 struct arm_pmu *armpmu_alloc_atomic(void);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 6/6] arm64: Enable perf events based hard lockup detector
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

With the recent feature added to enable perf events to use pseudo NMIs
as interrupts on platforms which support GICv3 or later, its now been
possible to enable hard lockup detector (or NMI watchdog) on arm64
platforms. So enable corresponding support.

One thing to note here is that normally lockup detector is initialized
just after the early initcalls but PMU on arm64 comes up much later as
device_initcall(). To cope with that, overriding watchdog_nmi_probe() to
let the watchdog framework know PMU not ready, and inform the framework
to re-initialize lockup detection once PMU has been initialized.

[1]: http://lore.kernel.org/linux-arm-kernel/1610712101-14929-1-git-send-email-sumit.garg@linaro.org

Co-developed-by: Sumit Garg <sumit.garg@linaro.org>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Co-developed-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
---
 arch/arm64/Kconfig               |  2 ++
 arch/arm64/kernel/perf_event.c   | 10 ++++++++--
 arch/arm64/kernel/watchdog_hld.c | 14 ++++++++++++++
 drivers/perf/arm_pmu.c           |  5 +++++
 include/linux/perf/arm_pmu.h     |  2 ++
 5 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 20ea89d9ac2f..9d48fe4ce041 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -184,12 +184,14 @@ config ARM64
 	select HAVE_FUNCTION_ERROR_INJECTION
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_GCC_PLUGINS
+	select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI
 	select HAVE_HW_BREAKPOINT if PERF_EVENTS
 	select HAVE_IRQ_TIME_ACCOUNTING
 	select HAVE_KVM
 	select HAVE_NMI
 	select HAVE_PATA_PLATFORM
 	select HAVE_PERF_EVENTS
+	select HAVE_PERF_EVENTS_NMI if ARM64_PSEUDO_NMI
 	select HAVE_PERF_REGS
 	select HAVE_PERF_USER_STACK_DUMP
 	select HAVE_PREEMPT_DYNAMIC_KEY
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index cb69ff1e6138..d51ee09592d7 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/sched_clock.h>
 #include <linux/smp.h>
+#include <linux/nmi.h>
 
 /* ARMv8 Cortex-A53 specific event types. */
 #define ARMV8_A53_PERFCTR_PREF_LINEFILL				0xC2
@@ -1390,10 +1391,15 @@ static struct platform_driver armv8_pmu_driver = {
 
 static int __init armv8_pmu_driver_init(void)
 {
+	int ret;
+
 	if (acpi_disabled)
-		return platform_driver_register(&armv8_pmu_driver);
+		ret = platform_driver_register(&armv8_pmu_driver);
 	else
-		return arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
+		ret = arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
+
+	retry_lockup_detector_init();
+	return ret;
 }
 device_initcall(armv8_pmu_driver_init)
 
diff --git a/arch/arm64/kernel/watchdog_hld.c b/arch/arm64/kernel/watchdog_hld.c
index de43318e4dd6..c9c6ec889c15 100644
--- a/arch/arm64/kernel/watchdog_hld.c
+++ b/arch/arm64/kernel/watchdog_hld.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/nmi.h>
 #include <linux/cpufreq.h>
+#include <linux/perf/arm_pmu.h>
 
 /*
  * Safe maximum CPU frequency in case a particular platform doesn't implement
@@ -23,3 +25,15 @@ u64 hw_nmi_get_sample_period(int watchdog_thresh)
 	return (u64)max_cpu_freq * watchdog_thresh;
 }
 
+int __init watchdog_nmi_probe(void)
+{
+	/*
+	 * hardlockup_detector_perf_init() will success even if Pseudo-NMI turns off,
+	 * however, the pmu interrupts will act like a normal interrupt instead of
+	 * NMI and the hardlockup detector would be broken.
+	 */
+	if (!arm_pmu_irq_is_nmi())
+		return -ENODEV;
+
+	return hardlockup_detector_perf_init();
+}
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 59d3980b8ca2..ceee2c55d436 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -697,6 +697,11 @@ static int armpmu_get_cpu_irq(struct arm_pmu *pmu, int cpu)
 	return per_cpu(hw_events->irq, cpu);
 }
 
+bool arm_pmu_irq_is_nmi(void)
+{
+	return has_nmi;
+}
+
 /*
  * PMU hardware loses all context when a CPU goes offline.
  * When a CPU is hotplugged back in, since some hardware registers are
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 0407a38b470a..29c56c92bab7 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -171,6 +171,8 @@ void kvm_host_pmu_init(struct arm_pmu *pmu);
 #define kvm_host_pmu_init(x)	do { } while(0)
 #endif
 
+bool arm_pmu_irq_is_nmi(void);
+
 /* Internal functions only for core arm_pmu code */
 struct arm_pmu *armpmu_alloc(void);
 struct arm_pmu *armpmu_alloc_atomic(void);
-- 
2.25.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 6/6] arm64: Enable perf events based hard lockup detector
@ 2022-04-27 16:13   ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-04-27 16:13 UTC (permalink / raw)
  To: linux-kernel, pmladek
  Cc: lecopzer.chen, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

With the recent feature added to enable perf events to use pseudo NMIs
as interrupts on platforms which support GICv3 or later, its now been
possible to enable hard lockup detector (or NMI watchdog) on arm64
platforms. So enable corresponding support.

One thing to note here is that normally lockup detector is initialized
just after the early initcalls but PMU on arm64 comes up much later as
device_initcall(). To cope with that, overriding watchdog_nmi_probe() to
let the watchdog framework know PMU not ready, and inform the framework
to re-initialize lockup detection once PMU has been initialized.

[1]: http://lore.kernel.org/linux-arm-kernel/1610712101-14929-1-git-send-email-sumit.garg@linaro.org

Co-developed-by: Sumit Garg <sumit.garg@linaro.org>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Co-developed-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
---
 arch/arm64/Kconfig               |  2 ++
 arch/arm64/kernel/perf_event.c   | 10 ++++++++--
 arch/arm64/kernel/watchdog_hld.c | 14 ++++++++++++++
 drivers/perf/arm_pmu.c           |  5 +++++
 include/linux/perf/arm_pmu.h     |  2 ++
 5 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 20ea89d9ac2f..9d48fe4ce041 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -184,12 +184,14 @@ config ARM64
 	select HAVE_FUNCTION_ERROR_INJECTION
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_GCC_PLUGINS
+	select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI
 	select HAVE_HW_BREAKPOINT if PERF_EVENTS
 	select HAVE_IRQ_TIME_ACCOUNTING
 	select HAVE_KVM
 	select HAVE_NMI
 	select HAVE_PATA_PLATFORM
 	select HAVE_PERF_EVENTS
+	select HAVE_PERF_EVENTS_NMI if ARM64_PSEUDO_NMI
 	select HAVE_PERF_REGS
 	select HAVE_PERF_USER_STACK_DUMP
 	select HAVE_PREEMPT_DYNAMIC_KEY
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index cb69ff1e6138..d51ee09592d7 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/sched_clock.h>
 #include <linux/smp.h>
+#include <linux/nmi.h>
 
 /* ARMv8 Cortex-A53 specific event types. */
 #define ARMV8_A53_PERFCTR_PREF_LINEFILL				0xC2
@@ -1390,10 +1391,15 @@ static struct platform_driver armv8_pmu_driver = {
 
 static int __init armv8_pmu_driver_init(void)
 {
+	int ret;
+
 	if (acpi_disabled)
-		return platform_driver_register(&armv8_pmu_driver);
+		ret = platform_driver_register(&armv8_pmu_driver);
 	else
-		return arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
+		ret = arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
+
+	retry_lockup_detector_init();
+	return ret;
 }
 device_initcall(armv8_pmu_driver_init)
 
diff --git a/arch/arm64/kernel/watchdog_hld.c b/arch/arm64/kernel/watchdog_hld.c
index de43318e4dd6..c9c6ec889c15 100644
--- a/arch/arm64/kernel/watchdog_hld.c
+++ b/arch/arm64/kernel/watchdog_hld.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/nmi.h>
 #include <linux/cpufreq.h>
+#include <linux/perf/arm_pmu.h>
 
 /*
  * Safe maximum CPU frequency in case a particular platform doesn't implement
@@ -23,3 +25,15 @@ u64 hw_nmi_get_sample_period(int watchdog_thresh)
 	return (u64)max_cpu_freq * watchdog_thresh;
 }
 
+int __init watchdog_nmi_probe(void)
+{
+	/*
+	 * hardlockup_detector_perf_init() will success even if Pseudo-NMI turns off,
+	 * however, the pmu interrupts will act like a normal interrupt instead of
+	 * NMI and the hardlockup detector would be broken.
+	 */
+	if (!arm_pmu_irq_is_nmi())
+		return -ENODEV;
+
+	return hardlockup_detector_perf_init();
+}
diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 59d3980b8ca2..ceee2c55d436 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -697,6 +697,11 @@ static int armpmu_get_cpu_irq(struct arm_pmu *pmu, int cpu)
 	return per_cpu(hw_events->irq, cpu);
 }
 
+bool arm_pmu_irq_is_nmi(void)
+{
+	return has_nmi;
+}
+
 /*
  * PMU hardware loses all context when a CPU goes offline.
  * When a CPU is hotplugged back in, since some hardware registers are
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 0407a38b470a..29c56c92bab7 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -171,6 +171,8 @@ void kvm_host_pmu_init(struct arm_pmu *pmu);
 #define kvm_host_pmu_init(x)	do { } while(0)
 #endif
 
+bool arm_pmu_irq_is_nmi(void);
+
 /* Internal functions only for core arm_pmu code */
 struct arm_pmu *armpmu_alloc(void);
 struct arm_pmu *armpmu_alloc_atomic(void);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 4/6] kernel/watchdog: Adapt the watchdog_hld interface for async model
  2022-04-27 16:13   ` Lecopzer Chen
  (?)
@ 2022-05-20  9:38     ` Petr Mladek
  -1 siblings, 0 replies; 33+ messages in thread
From: Petr Mladek @ 2022-05-20  9:38 UTC (permalink / raw)
  To: Lecopzer Chen
  Cc: linux-kernel, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

On Thu 2022-04-28 00:13:38, Lecopzer Chen wrote:
> When lockup_detector_init()->watchdog_nmi_probe(), PMU may be not ready
> yet. E.g. on arm64, PMU is not ready until
> device_initcall(armv8_pmu_driver_init).  And it is deeply integrated
> with the driver model and cpuhp. Hence it is hard to push this
> initialization before smp_init().
> 
> But it is easy to take an opposite approach and try to initialize
> the watchdog once again later.
> The delayed probe is called using workqueues. It need to allocate
> memory and must be proceed in a normal context.
> The delayed probe is able to use if watchdog_nmi_probe() returns
> non-zero which means the return code returned when PMU is not ready yet.
> 
> Provide an API - retry_lockup_detector_init() for anyone who needs
> to delayed init lockup detector if they had ever failed at
> lockup_detector_init().
> 
> The original assumption is: nobody should use delayed probe after
> lockup_detector_check() which has __init attribute.
> That is, anyone uses this API must call between lockup_detector_init()
> and lockup_detector_check(), and the caller must have __init attribute
> 
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> +/*
> + * retry_lockup_detector_init - retry init lockup detector if possible.
> + *
> + * Retry hardlockup detector init. It is useful when it requires some
> + * functionality that has to be initialized later on a particular
> + * platform.
> + */
> +void __init retry_lockup_detector_init(void)
> +{
> +	/* Must be called before late init calls */
> +	if (!allow_lockup_detector_init_retry)
> +		return;
> +
> +	queue_work_on(__smp_processor_id(), system_wq, &detector_work);

Just a small nit. This can be simplified by calling:

	schedule_work(&detector_work);

It uses "system_wq" that uses CPU-bound workers. It prefers
the current CPU. But the exact CPU is not important. Any CPU-bound
worker is enough.

> +}
> +

With the above change, feel free to use:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

PS: I am sorry for the late review. I had busy weeks.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 4/6] kernel/watchdog: Adapt the watchdog_hld interface for async model
@ 2022-05-20  9:38     ` Petr Mladek
  0 siblings, 0 replies; 33+ messages in thread
From: Petr Mladek @ 2022-05-20  9:38 UTC (permalink / raw)
  To: Lecopzer Chen
  Cc: linux-kernel, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

On Thu 2022-04-28 00:13:38, Lecopzer Chen wrote:
> When lockup_detector_init()->watchdog_nmi_probe(), PMU may be not ready
> yet. E.g. on arm64, PMU is not ready until
> device_initcall(armv8_pmu_driver_init).  And it is deeply integrated
> with the driver model and cpuhp. Hence it is hard to push this
> initialization before smp_init().
> 
> But it is easy to take an opposite approach and try to initialize
> the watchdog once again later.
> The delayed probe is called using workqueues. It need to allocate
> memory and must be proceed in a normal context.
> The delayed probe is able to use if watchdog_nmi_probe() returns
> non-zero which means the return code returned when PMU is not ready yet.
> 
> Provide an API - retry_lockup_detector_init() for anyone who needs
> to delayed init lockup detector if they had ever failed at
> lockup_detector_init().
> 
> The original assumption is: nobody should use delayed probe after
> lockup_detector_check() which has __init attribute.
> That is, anyone uses this API must call between lockup_detector_init()
> and lockup_detector_check(), and the caller must have __init attribute
> 
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> +/*
> + * retry_lockup_detector_init - retry init lockup detector if possible.
> + *
> + * Retry hardlockup detector init. It is useful when it requires some
> + * functionality that has to be initialized later on a particular
> + * platform.
> + */
> +void __init retry_lockup_detector_init(void)
> +{
> +	/* Must be called before late init calls */
> +	if (!allow_lockup_detector_init_retry)
> +		return;
> +
> +	queue_work_on(__smp_processor_id(), system_wq, &detector_work);

Just a small nit. This can be simplified by calling:

	schedule_work(&detector_work);

It uses "system_wq" that uses CPU-bound workers. It prefers
the current CPU. But the exact CPU is not important. Any CPU-bound
worker is enough.

> +}
> +

With the above change, feel free to use:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

PS: I am sorry for the late review. I had busy weeks.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 4/6] kernel/watchdog: Adapt the watchdog_hld interface for async model
@ 2022-05-20  9:38     ` Petr Mladek
  0 siblings, 0 replies; 33+ messages in thread
From: Petr Mladek @ 2022-05-20  9:38 UTC (permalink / raw)
  To: Lecopzer Chen
  Cc: linux-kernel, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

On Thu 2022-04-28 00:13:38, Lecopzer Chen wrote:
> When lockup_detector_init()->watchdog_nmi_probe(), PMU may be not ready
> yet. E.g. on arm64, PMU is not ready until
> device_initcall(armv8_pmu_driver_init).  And it is deeply integrated
> with the driver model and cpuhp. Hence it is hard to push this
> initialization before smp_init().
> 
> But it is easy to take an opposite approach and try to initialize
> the watchdog once again later.
> The delayed probe is called using workqueues. It need to allocate
> memory and must be proceed in a normal context.
> The delayed probe is able to use if watchdog_nmi_probe() returns
> non-zero which means the return code returned when PMU is not ready yet.
> 
> Provide an API - retry_lockup_detector_init() for anyone who needs
> to delayed init lockup detector if they had ever failed at
> lockup_detector_init().
> 
> The original assumption is: nobody should use delayed probe after
> lockup_detector_check() which has __init attribute.
> That is, anyone uses this API must call between lockup_detector_init()
> and lockup_detector_check(), and the caller must have __init attribute
> 
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> +/*
> + * retry_lockup_detector_init - retry init lockup detector if possible.
> + *
> + * Retry hardlockup detector init. It is useful when it requires some
> + * functionality that has to be initialized later on a particular
> + * platform.
> + */
> +void __init retry_lockup_detector_init(void)
> +{
> +	/* Must be called before late init calls */
> +	if (!allow_lockup_detector_init_retry)
> +		return;
> +
> +	queue_work_on(__smp_processor_id(), system_wq, &detector_work);

Just a small nit. This can be simplified by calling:

	schedule_work(&detector_work);

It uses "system_wq" that uses CPU-bound workers. It prefers
the current CPU. But the exact CPU is not important. Any CPU-bound
worker is enough.

> +}
> +

With the above change, feel free to use:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

PS: I am sorry for the late review. I had busy weeks.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 6/6] arm64: Enable perf events based hard lockup detector
  2022-04-27 16:13   ` Lecopzer Chen
  (?)
@ 2022-05-20  9:47     ` Petr Mladek
  -1 siblings, 0 replies; 33+ messages in thread
From: Petr Mladek @ 2022-05-20  9:47 UTC (permalink / raw)
  To: Lecopzer Chen
  Cc: linux-kernel, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

On Thu 2022-04-28 00:13:40, Lecopzer Chen wrote:
> With the recent feature added to enable perf events to use pseudo NMIs
> as interrupts on platforms which support GICv3 or later, its now been
> possible to enable hard lockup detector (or NMI watchdog) on arm64
> platforms. So enable corresponding support.
> 
> One thing to note here is that normally lockup detector is initialized
> just after the early initcalls but PMU on arm64 comes up much later as
> device_initcall(). To cope with that, overriding watchdog_nmi_probe() to
> let the watchdog framework know PMU not ready, and inform the framework
> to re-initialize lockup detection once PMU has been initialized.
> 
> [1]: http://lore.kernel.org/linux-arm-kernel/1610712101-14929-1-git-send-email-sumit.garg@linaro.org
> 
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -1390,10 +1391,15 @@ static struct platform_driver armv8_pmu_driver = {
>  
>  static int __init armv8_pmu_driver_init(void)
>  {
> +	int ret;
> +
>  	if (acpi_disabled)
> -		return platform_driver_register(&armv8_pmu_driver);
> +		ret = platform_driver_register(&armv8_pmu_driver);
>  	else
> -		return arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> +		ret = arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> +
> +	retry_lockup_detector_init();

Does it makes sense to call retry_lockup_detector_init() when
the above returned an error? Should it be?

	if (!ret)
		retry_lockup_detector_init();

> +	return ret;
>  }
>  device_initcall(armv8_pmu_driver_init)


I am not qualified to ack the arm-specific code. But otherwise
the change looks reasonable.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 6/6] arm64: Enable perf events based hard lockup detector
@ 2022-05-20  9:47     ` Petr Mladek
  0 siblings, 0 replies; 33+ messages in thread
From: Petr Mladek @ 2022-05-20  9:47 UTC (permalink / raw)
  To: Lecopzer Chen
  Cc: linux-kernel, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

On Thu 2022-04-28 00:13:40, Lecopzer Chen wrote:
> With the recent feature added to enable perf events to use pseudo NMIs
> as interrupts on platforms which support GICv3 or later, its now been
> possible to enable hard lockup detector (or NMI watchdog) on arm64
> platforms. So enable corresponding support.
> 
> One thing to note here is that normally lockup detector is initialized
> just after the early initcalls but PMU on arm64 comes up much later as
> device_initcall(). To cope with that, overriding watchdog_nmi_probe() to
> let the watchdog framework know PMU not ready, and inform the framework
> to re-initialize lockup detection once PMU has been initialized.
> 
> [1]: http://lore.kernel.org/linux-arm-kernel/1610712101-14929-1-git-send-email-sumit.garg@linaro.org
> 
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -1390,10 +1391,15 @@ static struct platform_driver armv8_pmu_driver = {
>  
>  static int __init armv8_pmu_driver_init(void)
>  {
> +	int ret;
> +
>  	if (acpi_disabled)
> -		return platform_driver_register(&armv8_pmu_driver);
> +		ret = platform_driver_register(&armv8_pmu_driver);
>  	else
> -		return arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> +		ret = arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> +
> +	retry_lockup_detector_init();

Does it makes sense to call retry_lockup_detector_init() when
the above returned an error? Should it be?

	if (!ret)
		retry_lockup_detector_init();

> +	return ret;
>  }
>  device_initcall(armv8_pmu_driver_init)


I am not qualified to ack the arm-specific code. But otherwise
the change looks reasonable.

Best Regards,
Petr

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 6/6] arm64: Enable perf events based hard lockup detector
@ 2022-05-20  9:47     ` Petr Mladek
  0 siblings, 0 replies; 33+ messages in thread
From: Petr Mladek @ 2022-05-20  9:47 UTC (permalink / raw)
  To: Lecopzer Chen
  Cc: linux-kernel, acme, akpm, alexander.shishkin, catalin.marinas,
	davem, jolsa, jthierry, keescook, kernelfans, linux-arm-kernel,
	linux-mediatek, linux-perf-users, mark.rutland, masahiroy,
	matthias.bgg, maz, mcgrof, mingo, namhyung, nixiaoming, peterz,
	sparclinux, sumit.garg, wangqing, will, yj.chiang

On Thu 2022-04-28 00:13:40, Lecopzer Chen wrote:
> With the recent feature added to enable perf events to use pseudo NMIs
> as interrupts on platforms which support GICv3 or later, its now been
> possible to enable hard lockup detector (or NMI watchdog) on arm64
> platforms. So enable corresponding support.
> 
> One thing to note here is that normally lockup detector is initialized
> just after the early initcalls but PMU on arm64 comes up much later as
> device_initcall(). To cope with that, overriding watchdog_nmi_probe() to
> let the watchdog framework know PMU not ready, and inform the framework
> to re-initialize lockup detection once PMU has been initialized.
> 
> [1]: http://lore.kernel.org/linux-arm-kernel/1610712101-14929-1-git-send-email-sumit.garg@linaro.org
> 
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -1390,10 +1391,15 @@ static struct platform_driver armv8_pmu_driver = {
>  
>  static int __init armv8_pmu_driver_init(void)
>  {
> +	int ret;
> +
>  	if (acpi_disabled)
> -		return platform_driver_register(&armv8_pmu_driver);
> +		ret = platform_driver_register(&armv8_pmu_driver);
>  	else
> -		return arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> +		ret = arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> +
> +	retry_lockup_detector_init();

Does it makes sense to call retry_lockup_detector_init() when
the above returned an error? Should it be?

	if (!ret)
		retry_lockup_detector_init();

> +	return ret;
>  }
>  device_initcall(armv8_pmu_driver_init)


I am not qualified to ack the arm-specific code. But otherwise
the change looks reasonable.

Best Regards,
Petr

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 6/6] arm64: Enable perf events based hard lockup detector
  2022-05-20  9:47     ` Petr Mladek
  (?)
@ 2022-05-26  9:35       ` Lecopzer Chen
  -1 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-05-26  9:35 UTC (permalink / raw)
  To: pmladek
  Cc: acme, akpm, alexander.shishkin, catalin.marinas, davem, jolsa,
	jthierry, keescook, kernelfans, lecopzer.chen, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-perf-users, mark.rutland,
	masahiroy, matthias.bgg, maz, mcgrof, mingo, namhyung,
	nixiaoming, peterz, sparclinux, sumit.garg, wangqing, will,
	yj.chiang

> > to re-initialize lockup detection once PMU has been initialized.
> > 
> > [1]: http://lore.kernel.org/linux-arm-kernel/1610712101-14929-1-git-send-email-sumit.garg@linaro.org
> > 
> > --- a/arch/arm64/kernel/perf_event.c
> > +++ b/arch/arm64/kernel/perf_event.c
> > @@ -1390,10 +1391,15 @@ static struct platform_driver armv8_pmu_driver = {
> >  
> >  static int __init armv8_pmu_driver_init(void)
> >  {
> > +	int ret;
> > +
> >  	if (acpi_disabled)
> > -		return platform_driver_register(&armv8_pmu_driver);
> > +		ret = platform_driver_register(&armv8_pmu_driver);
> >  	else
> > -		return arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> > +		ret = arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> > +
> > +	retry_lockup_detector_init();
> 
> Does it makes sense to call retry_lockup_detector_init() when
> the above returned an error? Should it be?
> 
> 	if (!ret)
> 		retry_lockup_detector_init();

Oh I think you're right, I'll add a checking here.
> 
> > +	return ret;
> >  }
> >  device_initcall(armv8_pmu_driver_init)
> 
> 
> I am not qualified to ack the arm-specific code. But otherwise
> the change looks reasonable.

Thanks for your help, I'l rebase on 5.19 -rc1 and seek reviewing for
ARM relative part.


thanks
BRs,
Lecopzer



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 6/6] arm64: Enable perf events based hard lockup detector
@ 2022-05-26  9:35       ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-05-26  9:35 UTC (permalink / raw)
  To: pmladek
  Cc: acme, akpm, alexander.shishkin, catalin.marinas, davem, jolsa,
	jthierry, keescook, kernelfans, lecopzer.chen, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-perf-users, mark.rutland,
	masahiroy, matthias.bgg, maz, mcgrof, mingo, namhyung,
	nixiaoming, peterz, sparclinux, sumit.garg, wangqing, will,
	yj.chiang

> > to re-initialize lockup detection once PMU has been initialized.
> > 
> > [1]: http://lore.kernel.org/linux-arm-kernel/1610712101-14929-1-git-send-email-sumit.garg@linaro.org
> > 
> > --- a/arch/arm64/kernel/perf_event.c
> > +++ b/arch/arm64/kernel/perf_event.c
> > @@ -1390,10 +1391,15 @@ static struct platform_driver armv8_pmu_driver = {
> >  
> >  static int __init armv8_pmu_driver_init(void)
> >  {
> > +	int ret;
> > +
> >  	if (acpi_disabled)
> > -		return platform_driver_register(&armv8_pmu_driver);
> > +		ret = platform_driver_register(&armv8_pmu_driver);
> >  	else
> > -		return arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> > +		ret = arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> > +
> > +	retry_lockup_detector_init();
> 
> Does it makes sense to call retry_lockup_detector_init() when
> the above returned an error? Should it be?
> 
> 	if (!ret)
> 		retry_lockup_detector_init();

Oh I think you're right, I'll add a checking here.
> 
> > +	return ret;
> >  }
> >  device_initcall(armv8_pmu_driver_init)
> 
> 
> I am not qualified to ack the arm-specific code. But otherwise
> the change looks reasonable.

Thanks for your help, I'l rebase on 5.19 -rc1 and seek reviewing for
ARM relative part.


thanks
BRs,
Lecopzer



_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 6/6] arm64: Enable perf events based hard lockup detector
@ 2022-05-26  9:35       ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-05-26  9:35 UTC (permalink / raw)
  To: pmladek
  Cc: acme, akpm, alexander.shishkin, catalin.marinas, davem, jolsa,
	jthierry, keescook, kernelfans, lecopzer.chen, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-perf-users, mark.rutland,
	masahiroy, matthias.bgg, maz, mcgrof, mingo, namhyung,
	nixiaoming, peterz, sparclinux, sumit.garg, wangqing, will,
	yj.chiang

> > to re-initialize lockup detection once PMU has been initialized.
> > 
> > [1]: http://lore.kernel.org/linux-arm-kernel/1610712101-14929-1-git-send-email-sumit.garg@linaro.org
> > 
> > --- a/arch/arm64/kernel/perf_event.c
> > +++ b/arch/arm64/kernel/perf_event.c
> > @@ -1390,10 +1391,15 @@ static struct platform_driver armv8_pmu_driver = {
> >  
> >  static int __init armv8_pmu_driver_init(void)
> >  {
> > +	int ret;
> > +
> >  	if (acpi_disabled)
> > -		return platform_driver_register(&armv8_pmu_driver);
> > +		ret = platform_driver_register(&armv8_pmu_driver);
> >  	else
> > -		return arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> > +		ret = arm_pmu_acpi_probe(armv8_pmuv3_pmu_init);
> > +
> > +	retry_lockup_detector_init();
> 
> Does it makes sense to call retry_lockup_detector_init() when
> the above returned an error? Should it be?
> 
> 	if (!ret)
> 		retry_lockup_detector_init();

Oh I think you're right, I'll add a checking here.
> 
> > +	return ret;
> >  }
> >  device_initcall(armv8_pmu_driver_init)
> 
> 
> I am not qualified to ack the arm-specific code. But otherwise
> the change looks reasonable.

Thanks for your help, I'l rebase on 5.19 -rc1 and seek reviewing for
ARM relative part.


thanks
BRs,
Lecopzer



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 4/6] kernel/watchdog: Adapt the watchdog_hld interface for async model
  2022-05-20  9:38     ` Petr Mladek
  (?)
@ 2022-05-26  9:39       ` Lecopzer Chen
  -1 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-05-26  9:39 UTC (permalink / raw)
  To: pmladek
  Cc: acme, akpm, alexander.shishkin, catalin.marinas, davem, jolsa,
	jthierry, keescook, kernelfans, lecopzer.chen, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-perf-users, mark.rutland,
	masahiroy, matthias.bgg, maz, mcgrof, mingo, namhyung,
	nixiaoming, peterz, sparclinux, sumit.garg, wangqing, will,
	yj.chiang

> On Thu 2022-04-28 00:13:38, Lecopzer Chen wrote:
> > When lockup_detector_init()->watchdog_nmi_probe(), PMU may be not ready
> > yet. E.g. on arm64, PMU is not ready until
> > device_initcall(armv8_pmu_driver_init).  And it is deeply integrated
> > with the driver model and cpuhp. Hence it is hard to push this
> > initialization before smp_init().
> > 
> > But it is easy to take an opposite approach and try to initialize
> > the watchdog once again later.
> > The delayed probe is called using workqueues. It need to allocate
> > memory and must be proceed in a normal context.
> > The delayed probe is able to use if watchdog_nmi_probe() returns
> > non-zero which means the return code returned when PMU is not ready yet.
> > 
> > Provide an API - retry_lockup_detector_init() for anyone who needs
> > to delayed init lockup detector if they had ever failed at
> > lockup_detector_init().
> > 
> > The original assumption is: nobody should use delayed probe after
> > lockup_detector_check() which has __init attribute.
> > That is, anyone uses this API must call between lockup_detector_init()
> > and lockup_detector_check(), and the caller must have __init attribute
> > 
> > --- a/kernel/watchdog.c
> > +++ b/kernel/watchdog.c
> > +/*
> > + * retry_lockup_detector_init - retry init lockup detector if possible.
> > + *
> > + * Retry hardlockup detector init. It is useful when it requires some
> > + * functionality that has to be initialized later on a particular
> > + * platform.
> > + */
> > +void __init retry_lockup_detector_init(void)
> > +{
> > +	/* Must be called before late init calls */
> > +	if (!allow_lockup_detector_init_retry)
> > +		return;
> > +
> > +	queue_work_on(__smp_processor_id(), system_wq, &detector_work);
> 
> Just a small nit. This can be simplified by calling:
> 
> 	schedule_work(&detector_work);
> 
> It uses "system_wq" that uses CPU-bound workers. It prefers
> the current CPU. But the exact CPU is not important. Any CPU-bound
> worker is enough.

Thanks!! I'll tweak this on -rc1
> 
> > +}
> > +
> 
> With the above change, feel free to use:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

Really appreciate your review and idea, thank you ver much.


BRs,
Lecopzer




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 4/6] kernel/watchdog: Adapt the watchdog_hld interface for async model
@ 2022-05-26  9:39       ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-05-26  9:39 UTC (permalink / raw)
  To: pmladek
  Cc: acme, akpm, alexander.shishkin, catalin.marinas, davem, jolsa,
	jthierry, keescook, kernelfans, lecopzer.chen, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-perf-users, mark.rutland,
	masahiroy, matthias.bgg, maz, mcgrof, mingo, namhyung,
	nixiaoming, peterz, sparclinux, sumit.garg, wangqing, will,
	yj.chiang

> On Thu 2022-04-28 00:13:38, Lecopzer Chen wrote:
> > When lockup_detector_init()->watchdog_nmi_probe(), PMU may be not ready
> > yet. E.g. on arm64, PMU is not ready until
> > device_initcall(armv8_pmu_driver_init).  And it is deeply integrated
> > with the driver model and cpuhp. Hence it is hard to push this
> > initialization before smp_init().
> > 
> > But it is easy to take an opposite approach and try to initialize
> > the watchdog once again later.
> > The delayed probe is called using workqueues. It need to allocate
> > memory and must be proceed in a normal context.
> > The delayed probe is able to use if watchdog_nmi_probe() returns
> > non-zero which means the return code returned when PMU is not ready yet.
> > 
> > Provide an API - retry_lockup_detector_init() for anyone who needs
> > to delayed init lockup detector if they had ever failed at
> > lockup_detector_init().
> > 
> > The original assumption is: nobody should use delayed probe after
> > lockup_detector_check() which has __init attribute.
> > That is, anyone uses this API must call between lockup_detector_init()
> > and lockup_detector_check(), and the caller must have __init attribute
> > 
> > --- a/kernel/watchdog.c
> > +++ b/kernel/watchdog.c
> > +/*
> > + * retry_lockup_detector_init - retry init lockup detector if possible.
> > + *
> > + * Retry hardlockup detector init. It is useful when it requires some
> > + * functionality that has to be initialized later on a particular
> > + * platform.
> > + */
> > +void __init retry_lockup_detector_init(void)
> > +{
> > +	/* Must be called before late init calls */
> > +	if (!allow_lockup_detector_init_retry)
> > +		return;
> > +
> > +	queue_work_on(__smp_processor_id(), system_wq, &detector_work);
> 
> Just a small nit. This can be simplified by calling:
> 
> 	schedule_work(&detector_work);
> 
> It uses "system_wq" that uses CPU-bound workers. It prefers
> the current CPU. But the exact CPU is not important. Any CPU-bound
> worker is enough.

Thanks!! I'll tweak this on -rc1
> 
> > +}
> > +
> 
> With the above change, feel free to use:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

Really appreciate your review and idea, thank you ver much.


BRs,
Lecopzer




_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 4/6] kernel/watchdog: Adapt the watchdog_hld interface for async model
@ 2022-05-26  9:39       ` Lecopzer Chen
  0 siblings, 0 replies; 33+ messages in thread
From: Lecopzer Chen @ 2022-05-26  9:39 UTC (permalink / raw)
  To: pmladek
  Cc: acme, akpm, alexander.shishkin, catalin.marinas, davem, jolsa,
	jthierry, keescook, kernelfans, lecopzer.chen, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-perf-users, mark.rutland,
	masahiroy, matthias.bgg, maz, mcgrof, mingo, namhyung,
	nixiaoming, peterz, sparclinux, sumit.garg, wangqing, will,
	yj.chiang

> On Thu 2022-04-28 00:13:38, Lecopzer Chen wrote:
> > When lockup_detector_init()->watchdog_nmi_probe(), PMU may be not ready
> > yet. E.g. on arm64, PMU is not ready until
> > device_initcall(armv8_pmu_driver_init).  And it is deeply integrated
> > with the driver model and cpuhp. Hence it is hard to push this
> > initialization before smp_init().
> > 
> > But it is easy to take an opposite approach and try to initialize
> > the watchdog once again later.
> > The delayed probe is called using workqueues. It need to allocate
> > memory and must be proceed in a normal context.
> > The delayed probe is able to use if watchdog_nmi_probe() returns
> > non-zero which means the return code returned when PMU is not ready yet.
> > 
> > Provide an API - retry_lockup_detector_init() for anyone who needs
> > to delayed init lockup detector if they had ever failed at
> > lockup_detector_init().
> > 
> > The original assumption is: nobody should use delayed probe after
> > lockup_detector_check() which has __init attribute.
> > That is, anyone uses this API must call between lockup_detector_init()
> > and lockup_detector_check(), and the caller must have __init attribute
> > 
> > --- a/kernel/watchdog.c
> > +++ b/kernel/watchdog.c
> > +/*
> > + * retry_lockup_detector_init - retry init lockup detector if possible.
> > + *
> > + * Retry hardlockup detector init. It is useful when it requires some
> > + * functionality that has to be initialized later on a particular
> > + * platform.
> > + */
> > +void __init retry_lockup_detector_init(void)
> > +{
> > +	/* Must be called before late init calls */
> > +	if (!allow_lockup_detector_init_retry)
> > +		return;
> > +
> > +	queue_work_on(__smp_processor_id(), system_wq, &detector_work);
> 
> Just a small nit. This can be simplified by calling:
> 
> 	schedule_work(&detector_work);
> 
> It uses "system_wq" that uses CPU-bound workers. It prefers
> the current CPU. But the exact CPU is not important. Any CPU-bound
> worker is enough.

Thanks!! I'll tweak this on -rc1
> 
> > +}
> > +
> 
> With the above change, feel free to use:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

Really appreciate your review and idea, thank you ver much.


BRs,
Lecopzer




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2022-05-26  9:45 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-27 16:13 [PATCH v4 0/6] Support hld delayed init based on Pseudo-NMI for arm64 Lecopzer Chen
2022-04-27 16:13 ` Lecopzer Chen
2022-04-27 16:13 ` Lecopzer Chen
2022-04-27 16:13 ` [PATCH v4 1/6] kernel/watchdog: remove WATCHDOG_DEFAULT Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-04-27 16:13 ` [PATCH v4 2/6] kernel/watchdog: change watchdog_nmi_enable() to void Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-04-27 16:13 ` [PATCH v4 3/6] kernel/watchdog_hld: Ensure CPU-bound context when creating hardlockup detector event Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-04-27 16:13 ` [PATCH v4 4/6] kernel/watchdog: Adapt the watchdog_hld interface for async model Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-05-20  9:38   ` Petr Mladek
2022-05-20  9:38     ` Petr Mladek
2022-05-20  9:38     ` Petr Mladek
2022-05-26  9:39     ` Lecopzer Chen
2022-05-26  9:39       ` Lecopzer Chen
2022-05-26  9:39       ` Lecopzer Chen
2022-04-27 16:13 ` [PATCH v4 5/6] arm64: add hw_nmi_get_sample_period for preparation of lockup detector Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-04-27 16:13 ` [PATCH v4 6/6] arm64: Enable perf events based hard " Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-04-27 16:13   ` Lecopzer Chen
2022-05-20  9:47   ` Petr Mladek
2022-05-20  9:47     ` Petr Mladek
2022-05-20  9:47     ` Petr Mladek
2022-05-26  9:35     ` Lecopzer Chen
2022-05-26  9:35       ` Lecopzer Chen
2022-05-26  9:35       ` Lecopzer Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.