Ping.
Denys, do you see any problem with default-disabling this, given the
rationale in the commit log and the feedback from others?
On 06/04/2021 10.14, Rasmus Villemoes wrote:
> The behaviour introduced by commit 31c765081dc4 ("watchdog: stop
> watchdog first on startup") causes warnings in the kernel log when the
> nowayout feature is enabled:
>
> [ 16.212184] watchdog: watchdog0: nowayout prevents watchdog being stopped!
> [ 16.212196] watchdog: watchdog0: watchdog did not stop!
>
> The latter may also appear by itself in case the watchdog is of the
> type that cannot be stopped once started (e.g. the common
> always-running gpio_wdt kind).
>
> These warnings can be somewhat ominous and distracting, so allow
> configuring whether to use this open-write-close-open sequence rather
> than just open. Also saves a bit of .text when disabled:
>
> function old new delta
> shutdown_on_signal 31 58 +27
> watchdog_main 339 306 -33
> shutdown_watchdog 34 - -34
> ------------------------------------------------------------------------------
> (add/remove: 0/1 grow/shrink: 1/1 up/down: 27/-67) Total: -40 bytes
>
> Make it default n:
>
> - It's a workaround for one specific type of watchdog (and
> that seems to be a defect in the kernel driver)
> - Even when not enabled in busybox config, it can easily be
> implemented outside busybox
> - Code size
> - Commit 31c765081dc4 should be considered a regression for all the
> boards that now end up with KERN_CRIT warnings in dmesg.
> - The author of that commit said "This use case is evidently rare, so
> if it is indeed causing problems for other people I'd OK then I
> understand whatever needs to be done." in the v1 thread.
>
> Cc: Matt Spinler <[email protected]>
> Cc: [email protected]
> Cc: tito <[email protected]>
> Signed-off-by: Rasmus Villemoes <[email protected]>
> ---
> v3: typo fix
> v2: change default to n, reword help text and commit log accordingly.
>
> miscutils/watchdog.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/miscutils/watchdog.c b/miscutils/watchdog.c
> index 0ed10bcf1..959e4995d 100644
> --- a/miscutils/watchdog.c
> +++ b/miscutils/watchdog.c
> @@ -18,6 +18,21 @@
> //config: watchdog applet ever fails to write the magic character within a
> //config: certain amount of time, the watchdog device assumes the system
> has
> //config: hung, and will cause the hardware to reboot.
> +//config:
> +//config:config FEATURE_WATCHDOG_OPEN_TWICE
> +//config: bool "Open watchdog device twice, closing it gracefully in
> between"
> +//config: depends on WATCHDOG
> +//config: default n
> +//config: help
> +//config: When enabled, the watchdog device is opened and then immediately
> +//config: magic-closed, before being opened a second time. This may be
> necessary
> +//config: for some watchdog devices, but can cause spurious warnings in
> the
> +//config: kernel log if the nowayout feature is enabled. Also, if this
> workaround
> +//config: is really needed for you machine to work properly, consider
> whether
> +//config: it should be fixed in the kernel driver instead. Even when
> disabled,
> +//config: the behaviour is easily emulated with a "printf 'V' >
> /dev/watchdog"
> +//config: immediately before starting the busybox watchdog daemon. Say n
> unless
> +//config: you know that you absolutely need this.
>
> //applet:IF_WATCHDOG(APPLET(watchdog, BB_DIR_SBIN, BB_SUID_DROP))
>
> @@ -73,6 +88,7 @@ static void watchdog_open(const char* device)
> /* Use known fd # - avoid needing global 'int fd' */
> xmove_fd(xopen(device, O_WRONLY), 3);
>
> +#if ENABLE_FEATURE_WATCHDOG_OPEN_TWICE
> /* If the watchdog driver can do something other than cause a reboot
> * on a timeout, then it's possible this program may be starting from
> * a state when the watchdog hadn't been previously stopped with
> @@ -82,6 +98,7 @@ static void watchdog_open(const char* device)
> shutdown_watchdog();
>
> xmove_fd(xopen(device, O_WRONLY), 3);
> +#endif
> }
>
> int watchdog_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
>
--
Rasmus Villemoes
Software Developer
Prevas A/S
Hedeager 3
DK-8200 Aarhus N
+45 51210274
[email protected]
www.prevas.dk
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
