lkml.org 
[lkml]   [2013]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@redhat.com>

>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Don Zickus <dzickus@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.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
>


\
 
 \ /
  Last update: 2013-05-20 20:41    [W:0.228 / U:0.972 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site