Re: [PATCH v5 12/18] watchdog/hardlockup: Rename some "NMI watchdog" constants/function
From: Petr Mladek <pmladek@suse.com>
Date: 2023-05-24 13:38:49
Also in:
linux-perf-users, linuxppc-dev, lkml, sparclinux
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Fri 2023-05-19 10:18:36, Douglas Anderson wrote:
quoted hunk ↗ jump to hunk
Do a search and replace of: - NMI_WATCHDOG_ENABLED => WATCHDOG_HARDLOCKUP_ENABLED - SOFT_WATCHDOG_ENABLED => WATCHDOG_SOFTOCKUP_ENABLED - watchdog_nmi_ => watchdog_hardlockup_ - nmi_watchdog_available => watchdog_hardlockup_available - nmi_watchdog_user_enabled => watchdog_hardlockup_user_enabled - soft_watchdog_user_enabled => watchdog_softlockup_user_enabled - NMI_WATCHDOG_DEFAULT => WATCHDOG_HARDLOCKUP_DEFAULT Then update a few comments near where names were changed. This is specifically to make it less confusing when we want to introduce the buddy hardlockup detector, which isn't using NMIs. As part of this, we sanitized a few names for consistency.--- a/kernel/watchdog.c +++ b/kernel/watchdog.c@@ -30,17 +30,17 @@ static DEFINE_MUTEX(watchdog_mutex); #if defined(CONFIG_HARDLOCKUP_DETECTOR) || defined(CONFIG_HAVE_NMI_WATCHDOG) -# define NMI_WATCHDOG_DEFAULT 1 +# define WATCHDOG_HARDLOCKUP_DEFAULT 1 #else -# define NMI_WATCHDOG_DEFAULT 0 +# define WATCHDOG_HARDLOCKUP_DEFAULT 0 #endif unsigned long __read_mostly watchdog_enabled; int __read_mostly watchdog_user_enabled = 1; -int __read_mostly nmi_watchdog_user_enabled = NMI_WATCHDOG_DEFAULT; -int __read_mostly soft_watchdog_user_enabled = 1; +int __read_mostly watchdog_hardlockup_user_enabled = WATCHDOG_HARDLOCKUP_DEFAULT; +int __read_mostly watchdog_softlockup_user_enabled = 1;
I still see nmi_watchdog_user_enabled and soft_watchdog_user_enabled
in include/linux/nmi.h. They are declared there and also mentioned
in a comment.
It seems that they do not actually need to be declared there.
I guess that it was need for the /proc stuff. But it was
moved into kernel/watchdog.c by the commit commit dd0693fdf054f2ed37
("watchdog: move watchdog sysctl interface to watchdog.c").
int __read_mostly watchdog_thresh = 10; -static int __read_mostly nmi_watchdog_available; +static int __read_mostly watchdog_hardlockup_available; struct cpumask watchdog_cpumask __read_mostly; unsigned long *watchdog_cpumask_bits = cpumask_bits(&watchdog_cpumask);
Otherwise, I like the changes. With the following:
diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 83076bf70ce8..d14fe345eae9 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h@@ -17,8 +17,6 @@ void lockup_detector_soft_poweroff(void); void lockup_detector_cleanup(void); extern int watchdog_user_enabled; -extern int nmi_watchdog_user_enabled; -extern int soft_watchdog_user_enabled; extern int watchdog_thresh; extern unsigned long watchdog_enabled;
@@ -68,8 +66,8 @@ static inline void reset_hung_task_detector(void) { } * 'watchdog_enabled' variable. Each lockup detector has its dedicated bit - * bit 0 for the hard lockup detector and bit 1 for the soft lockup detector. * - * 'watchdog_user_enabled', 'nmi_watchdog_user_enabled' and - * 'soft_watchdog_user_enabled' are variables that are only used as an + * 'watchdog_user_enabled', 'watchdog_hardlockup_user_enabled' and + * 'watchdog_softlockup_user_enabled' are variables that are only used as an * 'interface' between the parameters in /proc/sys/kernel and the internal * state bits in 'watchdog_enabled'. The 'watchdog_thresh' variable is * handled differently because its value is not boolean, and the lockup
Reviewed-by: Petr Mladek <pmladek@suse.com> Even better might be to remove the unused declaration in a separate patch. I think that more declarations are not needed after moving the /proc stuff into kernel/watchdog.c. But it might also be fixed later. Best Regards, Petr _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel