Hello Wim,
On 09/28/2016 02:34 PM, Vladimir Zapolskiy wrote:
Hello Wim,
On 09/28/2016 11:49 AM, Wim Van Sebroeck wrote:
Hi All,
The change adds a simple watchdog pretimeout framework infrastructure,
its purpose is to allow users to select a desired handling of watchdog
pretimeout events, which may be generated by a watchdog driver.
The idea of adding this kind of a framework appeared after reviewing
several attempts to add hardcoded pretimeout event handling to some
watchdog driver and after a discussion with Guenter, see
https://lkml.org/lkml/2015/11/4/346
Watchdogs with WDIOF_PRETIMEOUT capability now may have three device
attributes in sysfs: read only pretimeout value attribute, read/write
pretimeout_governor attribute, read only pretimeout_available_governors
attribute.
To throw a pretimeout event for further processing a watchdog driver
should call exported watchdog_notify_pretimeout(wdd) interface.
In addition to the framework two simple watchdog pretimeout governors
are added for review: panic and noop.
I still have 2 questions about this series:
1) Patch 3:
+/* Use the following functions to report watchdog pretimeout event */
+#if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
+void watchdog_notify_pretimeout(struct watchdog_device *wdd);
+#else
+static inline void watchdog_notify_pretimeout(struct watchdog_device *wdd)
+{
+ panic("watchdog pretimeout event\n");
+}
+#endif
+
Why a panic here? Why not just a printk like in the noop pretimeout governor?
We now also don't do a panic, which means that changing this to a panic means that we change the default behaviour...
2) Why did we choose the panic pretimeout governer as the default and not the noop governor?
This is basically the same question/remark as my previous question. noop is more the old behaviour, panic is a change in regards to the old/current behaviour...
regarding a selection of panic as a default action I had a discussion with
Guenter about it right in the beginnig of the development (v1 of the series
introduced printk by default, v2 and all later series select panic), please
follow the link to it (ctrl-f panic):
http://www.spinics.net/lists/linux-watchdog/msg07955.html
I generally don't have a strong preference, because it is configurable both
during build time and runtime, and I'm flexible to accept any selection.
I noticed that the complete series is not on your watchdog/master branch,
for clarity do you request to do any changes to merge it into v4.9?
--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
