Messages in this thread | | | Date | Sun, 5 Apr 2015 12:46:47 -0400 (EDT) | From | Ulrich Obergfell <> | Subject | Re: [PATCH v4 2/2] watchdog: add watchdog_exclude sysctl to assist nohz |
| |
Chris,
I'd like to comment on the following proposed change:
+int proc_dowatchdog_exclude(struct ctl_table *table, int write, + void __user *buffer, size_t *lenp, loff_t *ppos) +{ + int err; + + mutex_lock(&watchdog_proc_mutex); + err = proc_do_large_bitmap(table, write, buffer, lenp, ppos); + if (!err && write && watchdog_user_enabled) { + watchdog_disable_all_cpus(); + watchdog_enable_all_cpus(false); + } + mutex_unlock(&watchdog_proc_mutex); + return err; +}
The watchdog mechanism is enabled if watchdog_user_enabled and watchdog_thresh are both non-zero. Hence, I think the if-statement in the above snippet of code should look like this:
if (!err && write && watchdog_user_enabled && watchdog_thresh)
Please see proc_dowatchdog() which checks the content of both variables before it calls watchdog_enable_all_cpus():
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/kernel/watchdog.c?id=refs/tags/v4.0-rc6#n682
For completeness, I'd also like to point out that if the patch series at https://lkml.org/lkml/2015/2/5/626 gets accepted upstream, the if-statement will have to be adjusted. I think it should then look like this:
if (!err && write && watchdog_enabled && watchdog_thresh) { watchdog_disable_all_cpus(); watchdog_enable_all_cpus(); }
Please see proc_watchdog_update() here which is similar to the above.
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/kernel/watchdog.c?id=refs/tags/next-20150402#n710
Regards,
Uli
| |