Re: [PATCH v5 00/10] watchdog: add watchdog pretimeout framework

[Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>]

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