Applied, thank you
On Tue, Apr 6, 2021 at 10:15 AM Rasmus Villemoes <[email protected]> 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; > -- > 2.29.2 > _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
