All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>,
	Ingo Molnar <mingo@kernel.org>, Don Zickus <dzickus@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>
Subject: [RFC PATCH 7/8] watchdog: Rename confusing state variable
Date: Mon, 20 May 2013 18:01:55 +0200	[thread overview]
Message-ID: <1369065716-22801-8-git-send-email-fweisbec@gmail.com> (raw)
In-Reply-To: <1369065716-22801-1-git-send-email-fweisbec@gmail.com>

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.

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


  parent reply	other threads:[~2013-05-20 16:02 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-20 16:01 [PATCH 0/8] nohz: Random fixes Frederic Weisbecker
2013-05-20 16:01 ` [PATCH 1/8] nohz: Warn if the machine can not perform nohz_full Frederic Weisbecker
2013-05-20 16:01 ` [PATCH 2/8] vtime: Use consistent clocks among nohz accounting Frederic Weisbecker
2013-06-03  9:47   ` Stefan Seyfried
2013-06-03 13:48     ` Steven Rostedt
2013-06-03 19:48     ` Frederic Weisbecker
2013-06-03 19:51       ` Stefan Seyfried
2013-06-03 20:12         ` Frederic Weisbecker
2013-05-20 16:01 ` [PATCH 3/8] watchdog: Boot-disable by default on full dynticks Frederic Weisbecker
2013-05-20 17:52   ` Don Zickus
2013-05-20 18:14     ` Frederic Weisbecker
2013-05-20 16:01 ` [PATCH 4/8] kvm: Move guest entry/exit APIs to context_tracking Frederic Weisbecker
2013-05-20 16:01 ` [PATCH 5/8] nohz: Fix notifier return val that enforce timekeeping Frederic Weisbecker
2013-05-20 16:01 ` [RFC PATCH 6/8] kthread: Enable parking requests from setup() and unpark() callbacks Frederic Weisbecker
2013-05-21  5:34   ` anish singh
2013-05-21  7:49     ` Srivatsa S. Bhat
2013-05-21  8:58       ` anish singh
2013-05-21  9:07         ` Srivatsa S. Bhat
2013-05-22 15:18         ` Frederic Weisbecker
2013-05-21  6:59   ` Srivatsa S. Bhat
2013-06-05 16:33     ` Frederic Weisbecker
2013-05-20 16:01 ` Frederic Weisbecker [this message]
2013-05-20 17:53   ` [RFC PATCH 7/8] watchdog: Rename confusing state variable Don Zickus
2013-05-20 16:01 ` [RFC PATCH 8/8] watchdog: Fix internal state with boot user disabled watchdog Frederic Weisbecker
2013-05-20 17:54   ` Don Zickus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1369065716-22801-8-git-send-email-fweisbec@gmail.com \
    --to=fweisbec@gmail.com \
    --cc=dzickus@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.