| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20130520175303.GJ133453@redhat.com>
Date: Mon, 20 May 2013 13:53:03 -0400
From: Don Zickus <dzickus@...hat.com>
To: Frederic Weisbecker <fweisbec@...il.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>
Subject: Re: [RFC PATCH 7/8] watchdog: Rename confusing state variable
On Mon, May 20, 2013 at 06:01:55PM +0200, Frederic Weisbecker wrote:
> We have two very conflicting state variable names in the
> watchdog:
>
> * watchdog_enabled: This one reflects the user interface. It's
> set to 1 by default and can be overriden with boot options
> or sysctl/procfs interface.
>
> * watchdog_disabled: This is the internal toggle state that
> tells if watchdog threads, timers and NMI events are currently
> running or not. This state depends on the user settings and
> hardware support. It's a convenient state latch.
>
> Now we really need to find clearer names because those
> are just too confusing to encourage deep review.
>
> watchdog_enabled now becomes watchdog_user_enabled to reflect
> its purpose as an interface.
>
> watchdog_disabled becomes watchdog_running to suggest its
> role as a pure internal state.
Heh. Thanks for the cleanup.
Acked-by: Don Zickus <dzickus@...hat.com>
>
> Signed-off-by: Frederic Weisbecker <fweisbec@...il.com>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Don Zickus <dzickus@...hat.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> ---
> include/linux/nmi.h | 2 +-
> kernel/sysctl.c | 4 ++--
> kernel/watchdog.c | 32 ++++++++++++++++----------------
> 3 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/nmi.h b/include/linux/nmi.h
> index db50840..6a45fb5 100644
> --- a/include/linux/nmi.h
> +++ b/include/linux/nmi.h
> @@ -46,7 +46,7 @@ static inline bool trigger_all_cpu_backtrace(void)
> #ifdef CONFIG_LOCKUP_DETECTOR
> int hw_nmi_is_cpu_stuck(struct pt_regs *);
> u64 hw_nmi_get_sample_period(int watchdog_thresh);
> -extern int watchdog_enabled;
> +extern int watchdog_user_enabled;
> extern int watchdog_thresh;
> struct ctl_table;
> extern int proc_dowatchdog(struct ctl_table *, int ,
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 9edcf45..b080565 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -801,7 +801,7 @@ static struct ctl_table kern_table[] = {
> #if defined(CONFIG_LOCKUP_DETECTOR)
> {
> .procname = "watchdog",
> - .data = &watchdog_enabled,
> + .data = &watchdog_user_enabled,
> .maxlen = sizeof (int),
> .mode = 0644,
> .proc_handler = proc_dowatchdog,
> @@ -828,7 +828,7 @@ static struct ctl_table kern_table[] = {
> },
> {
> .procname = "nmi_watchdog",
> - .data = &watchdog_enabled,
> + .data = &watchdog_user_enabled,
> .maxlen = sizeof (int),
> .mode = 0644,
> .proc_handler = proc_dowatchdog,
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 7e1a021..10776fe 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -29,9 +29,9 @@
> #include <linux/kvm_para.h>
> #include <linux/perf_event.h>
>
> -int watchdog_enabled = 1;
> +int watchdog_user_enabled = 1;
> int __read_mostly watchdog_thresh = 10;
> -static int __read_mostly watchdog_disabled;
> +static int __read_mostly watchdog_running = 1;
> static u64 __read_mostly sample_period;
>
> static DEFINE_PER_CPU(unsigned long, watchdog_touch_ts);
> @@ -63,7 +63,7 @@ static int __init hardlockup_panic_setup(char *str)
> else if (!strncmp(str, "nopanic", 7))
> hardlockup_panic = 0;
> else if (!strncmp(str, "0", 1))
> - watchdog_enabled = 0;
> + watchdog_user_enabled = 0;
> return 1;
> }
> __setup("nmi_watchdog=", hardlockup_panic_setup);
> @@ -82,7 +82,7 @@ __setup("softlockup_panic=", softlockup_panic_setup);
>
> static int __init nowatchdog_setup(char *str)
> {
> - watchdog_enabled = 0;
> + watchdog_user_enabled = 0;
> return 1;
> }
> __setup("nowatchdog", nowatchdog_setup);
> @@ -90,7 +90,7 @@ __setup("nowatchdog", nowatchdog_setup);
> /* deprecated */
> static int __init nosoftlockup_setup(char *str)
> {
> - watchdog_enabled = 0;
> + watchdog_user_enabled = 0;
> return 1;
> }
> __setup("nosoftlockup", nosoftlockup_setup);
> @@ -158,7 +158,7 @@ void touch_all_softlockup_watchdogs(void)
> #ifdef CONFIG_HARDLOCKUP_DETECTOR
> void touch_nmi_watchdog(void)
> {
> - if (watchdog_enabled) {
> + if (watchdog_user_enabled) {
> unsigned cpu;
>
> for_each_present_cpu(cpu) {
> @@ -347,7 +347,7 @@ static void watchdog_enable(unsigned int cpu)
> hrtimer_init(hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> hrtimer->function = watchdog_timer_fn;
>
> - if (!watchdog_enabled) {
> + if (!watchdog_user_enabled) {
> kthread_park(current);
> return;
> }
> @@ -482,8 +482,8 @@ static void watchdog_enable_all_cpus(void)
> {
> unsigned int cpu;
>
> - if (watchdog_disabled) {
> - watchdog_disabled = 0;
> + if (!watchdog_running) {
> + watchdog_running = 1;
> for_each_online_cpu(cpu)
> kthread_unpark(per_cpu(softlockup_watchdog, cpu));
> }
> @@ -493,8 +493,8 @@ static void watchdog_disable_all_cpus(void)
> {
> unsigned int cpu;
>
> - if (!watchdog_disabled) {
> - watchdog_disabled = 1;
> + if (watchdog_running) {
> + watchdog_running = 0;
> for_each_online_cpu(cpu)
> kthread_park(per_cpu(softlockup_watchdog, cpu));
> }
> @@ -509,7 +509,7 @@ int proc_dowatchdog(struct ctl_table *table, int write,
> {
> int ret;
>
> - if (watchdog_disabled < 0)
> + if (watchdog_running < 0)
> return -ENODEV;
>
> ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
> @@ -522,7 +522,7 @@ int proc_dowatchdog(struct ctl_table *table, int write,
> * disabled. The 'watchdog_disabled' variable check in
> * watchdog_*_all_cpus() function takes care of this.
> */
> - if (watchdog_enabled && watchdog_thresh)
> + if (watchdog_user_enabled && watchdog_thresh)
> watchdog_enable_all_cpus();
> else
> watchdog_disable_all_cpus();
> @@ -544,14 +544,14 @@ static struct smp_hotplug_thread watchdog_threads = {
> void __init lockup_detector_init(void)
> {
> #ifdef CONFIG_NO_HZ_FULL
> - watchdog_enabled = 0;
> - watchdog_disabled = 1;
> + watchdog_user_enabled = 0;
> + watchdog_running = 0;
> pr_warning("Disabled lockup detectors by default because of full dynticks\n");
> pr_warning("You can overwrite that with 'sysctl -w kernel.watchdog=1'\n");
> #endif
> set_sample_period();
> if (smpboot_register_percpu_thread(&watchdog_threads)) {
> pr_err("Failed to create watchdog threads, disabled\n");
> - watchdog_disabled = -ENODEV;
> + watchdog_running = -ENODEV;
> }
> }
> --
> 1.7.5.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists