[PATCH 1/2] watchdog/hardlockup: avoid unused-function warning

[PATCH 1/2] watchdog/hardlockup: avoid unused-function warning

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The newly added hardlockup_all_cpu_backtrace_proc_handler() function is defined
in some configurations that don't actually call it:

kernel/watchdog.c:112:12: error: 'hardlockup_all_cpu_backtrace_proc_handler' defined but not used [-Werror=unused-function]
  112 | static int hardlockup_all_cpu_backtrace_proc_handler(struct ctl_table *table, int write,
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Hide the definition when either CONFIG_SMP or CONFIG_SYSCTL are disabled and
the function is not actually needed.

Fixes: 838b3b76220d5 ("watchdog/hardlockup: avoid large stack frames in watchdog_hardlockup_check()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/watchdog.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 25d5627a6580b..0dd6a7acacd97 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -108,6 +108,7 @@ notrace void arch_touch_nmi_watchdog(void)
 }
 EXPORT_SYMBOL(arch_touch_nmi_watchdog);
 
+#if defined(CONFIG_SMP) && defined(CONFIG_SYSCTL)
 static int hardlockup_all_cpu_backtrace_proc_handler(struct ctl_table *table, int write,
 		  void *buffer, size_t *lenp, loff_t *ppos)
 {
@@ -130,6 +131,7 @@ static int hardlockup_all_cpu_backtrace_proc_handler(struct ctl_table *table, in
 
 	return ret;
 }
+#endif
 
 void watchdog_hardlockup_touch_cpu(unsigned int cpu)
 {
-- 
2.39.2

[PATCH 2/2] watchdog/hardlockup: simplify Kconfig selection

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The use of the 'imply' keyword tends to be a bad idea, as it's at best
confusing to the reader but often actually doesn't do what the author
intended.

In this case, it seems to be used correctly, but doing the same thing
using 'default' statements as we have elsewhere in the kernel is simpler
and would be easier to understand by readers that are unfamiliar with
the special semantics of 'imply'.

Fixes: 1356d0b966e7e ("watchdog/hardlockup: make the config checks more straightforward")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I got confused by the 'imply' here myself and thought this had caused
a build failure that turned out to be unrelated, but it might help
to apply this anyway to save the next person the confusion.
---
 lib/Kconfig.debug | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 72177a80baddc..8cfb49b6974c8 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1056,9 +1056,6 @@ config HARDLOCKUP_DETECTOR
 	bool "Detect Hard Lockups"
 	depends on DEBUG_KERNEL && !S390 && !HARDLOCKUP_DETECTOR_SPARC64
 	depends on HAVE_HARDLOCKUP_DETECTOR_PERF || HAVE_HARDLOCKUP_DETECTOR_BUDDY || HAVE_HARDLOCKUP_DETECTOR_ARCH
-	imply HARDLOCKUP_DETECTOR_PERF
-	imply HARDLOCKUP_DETECTOR_BUDDY
-	imply HARDLOCKUP_DETECTOR_ARCH
 	select LOCKUP_DETECTOR
 
 	help
@@ -1090,24 +1087,21 @@ config HARDLOCKUP_DETECTOR_PREFER_BUDDY
 	  for the hardlockup detector are better used for other things.
 
 config HARDLOCKUP_DETECTOR_PERF
-	bool
+	def_bool HAVE_HARDLOCKUP_DETECTOR_PERF
 	depends on HARDLOCKUP_DETECTOR
-	depends on HAVE_HARDLOCKUP_DETECTOR_PERF && !HARDLOCKUP_DETECTOR_PREFER_BUDDY
-	depends on !HAVE_HARDLOCKUP_DETECTOR_ARCH
+	depends on !HARDLOCKUP_DETECTOR_PREFER_BUDDY && !HAVE_HARDLOCKUP_DETECTOR_ARCH
 	select HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
 
 config HARDLOCKUP_DETECTOR_BUDDY
-	bool
+	def_bool HAVE_HARDLOCKUP_DETECTOR_BUDDY
 	depends on HARDLOCKUP_DETECTOR
-	depends on HAVE_HARDLOCKUP_DETECTOR_BUDDY
 	depends on !HAVE_HARDLOCKUP_DETECTOR_PERF || HARDLOCKUP_DETECTOR_PREFER_BUDDY
 	depends on !HAVE_HARDLOCKUP_DETECTOR_ARCH
 	select HARDLOCKUP_DETECTOR_COUNTS_HRTIMER
 
 config HARDLOCKUP_DETECTOR_ARCH
-	bool
+	def_bool HAVE_HARDLOCKUP_DETECTOR_ARCH
 	depends on HARDLOCKUP_DETECTOR
-	depends on HAVE_HARDLOCKUP_DETECTOR_ARCH
 	help
 	  The arch-specific implementation of the hardlockup detector will
 	  be used.
-- 
2.39.2

Re: [PATCH 2/2] watchdog/hardlockup: simplify Kconfig selection

[Doug Anderson]

Hi,

On Fri, Aug 4, 2023 at 6:28 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> The use of the 'imply' keyword tends to be a bad idea, as it's at best
> confusing to the reader but often actually doesn't do what the author
> intended.
>
> In this case, it seems to be used correctly, but doing the same thing
> using 'default' statements as we have elsewhere in the kernel is simpler
> and would be easier to understand by readers that are unfamiliar with
> the special semantics of 'imply'.
>
> Fixes: 1356d0b966e7e ("watchdog/hardlockup: make the config checks more straightforward")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> I got confused by the 'imply' here myself and thought this had caused
> a build failure that turned out to be unrelated, but it might help
> to apply this anyway to save the next person the confusion.
> ---
>  lib/Kconfig.debug | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)

Seems reasonable to me.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH 1/2] watchdog/hardlockup: avoid unused-function warning

[Doug Anderson]

Hi,

On Fri, Aug 4, 2023 at 6:28 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> The newly added hardlockup_all_cpu_backtrace_proc_handler() function is defined
> in some configurations that don't actually call it:
>
> kernel/watchdog.c:112:12: error: 'hardlockup_all_cpu_backtrace_proc_handler' defined but not used [-Werror=unused-function]
>   112 | static int hardlockup_all_cpu_backtrace_proc_handler(struct ctl_table *table, int write,
>       |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Hide the definition when either CONFIG_SMP or CONFIG_SYSCTL are disabled and
> the function is not actually needed.
>
> Fixes: 838b3b76220d5 ("watchdog/hardlockup: avoid large stack frames in watchdog_hardlockup_check()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  kernel/watchdog.c | 2 ++
>  1 file changed, 2 insertions(+)

Thanks for the fix. In this case I'll assume that Andrew will drop the
v2 of my patch and replace it with v4 [1], which shouldn't have this
issue.

[4] https://lore.kernel.org/r/20230804065935.v4.2.I501ab68cb926ee33a7c87e063d207abf09b9943c@changeid