lkml.org 
[lkml]   [2016]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC 3/3] watchdog: introduce CONFIG_WATCHDOG_OPEN_DEADLINE
From
Date
On 2016-07-14 16:42, Guenter Roeck wrote:
> On 07/14/2016 02:16 AM, Rasmus Villemoes wrote:
>>
>> +config WATCHDOG_OPEN_DEADLINE
>> + bool "Allow deadline for opening watchdog device"
>> + help
>> + If a watchdog driver indicates that to the framework that
>> + the hardware watchdog is running, the framework takes care
>> + of pinging the watchdog until userspace opens
>> + /dev/watchdogN. By selecting this option, you can set a
>> + maximum time for which the kernel will do this after the
>> + device has been registered.
>> +
>> +config WATCHDOG_OPEN_TIMEOUT
>> + int "Timeout value for opening watchdog device"
>> + depends on WATCHDOG_OPEN_DEADLINE
>> + default 120000
>> + help
>> + The maximum time, in milliseconds, for which the watchdog
>> + framework takes care of pinging a watchdog device. A value
>> + of 0 means infinite. The value set here can be overridden by
>> + the commandline parameter "watchdog.open_timeout" or through
>> + sysfs.
>> +
>
> I like the basic idea, and we always thought about implementing it,
> though as "initial timeout" (I personally preferred that term).

I also used WATCHDOG_INIT_TIMEOUT in my first few drafts, and my helper
watchdog_set_open_deadline was called watchdog_set_init_timeout. But
then I stumbled on watchdog_init_timeout in watchdog_core.c, and thought
that might end up being quite confusing. I think having 'open' part of
the name is quite natural, but I don't really have strong feelings about
the naming of this thing.

> However, implementing it as configuration option diminishes its
> value substantially, since it means that using it in multi-platform
> images (such as multi_v7_defconfig) becomes impossible.

If one wants to allow this feature in an existing _defconfig, one can
set OPEN_DEADLINE=y and OPEN_TIMEOUT=0; we could change the default for
the latter to that. (I thought about just having that single config
option with a default of 0, but it wasn't much more code to allow this
thing to be compiled out completely.) Platforms without a running
watchdog won't be affected, and for those with, having a non-zero
deadline requires opt-in anyway.

> The initial timeout should be specified as module option or as
> devicetree parameter, and there should be no additional configuration
> option.

I was under the impression that device tree was exclusively for
describing hardware, and this certainly is not that. I also wanted to
avoid having to modify each driver, which would seem to be necessary if
it was module parameter/DT - the only thing required of a driver now is
that it correctly reports WDOG_HW_RUNNING.

Regardless of implementation, it's not something that any "distro"
kernel is going to enable (with non-zero deadline), so to use it will
require some customization - which could be a .config tweak, passing a
command line parameter, a custom dtb, and probably other options. ISTM
that the first two are the most generic and require the least repeated
work across platforms.

> Where does the module parameter in watchdog_dev.c end up ?

# cat /sys/module/watchdog/parameters/open_timeout
120000

Rasmus

\
 
 \ /
  Last update: 2016-07-15 10:21    [W:1.840 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site