Hi Vladimir, > 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? I'll make the changes this weekend. I want to have the noop as default. So rest should go in this weekend so that we can still get it in for 4.9. Kind regards, Wim. -- 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
