watchdog: make open-write-close-open functionality a config knob

The behaviour introduced by commit 31c765081d ("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 31c765081d 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 <mspinler@linux.vnet.ibm.com>
Cc: deweloper@wp.pl
Cc: tito <farmatito@tiscali.it>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
This commit is contained in:
Rasmus Villemoes 2021-04-06 10:14:53 +02:00 committed by Denys Vlasenko
parent 9c210f0efb
commit 50a37459ff

View File

@ -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 # this behavior was essentially a hack for a broken driver
//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))
@ -50,10 +65,6 @@
# define WDIOS_ENABLECARD 2
#endif
#define OPT_FOREGROUND (1 << 0)
#define OPT_STIMER (1 << 1)
#define OPT_HTIMER (1 << 2)
static void shutdown_watchdog(void)
{
static const char V = 'V';
@ -73,6 +84,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 +94,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;
@ -100,6 +113,9 @@ int watchdog_main(int argc UNUSED_PARAM, char **argv)
char *st_arg;
char *ht_arg;
#define OPT_FOREGROUND (1 << 0)
#define OPT_STIMER (1 << 1)
#define OPT_HTIMER (1 << 2)
opts = getopt32(argv, "^" "Ft:T:" "\0" "=1"/*must have exactly 1 arg*/,
&st_arg, &ht_arg
);
@ -132,7 +148,7 @@ int watchdog_main(int argc UNUSED_PARAM, char **argv)
#if 0
ioctl_or_warn(3, WDIOC_GETTIMEOUT, &htimer_duration);
printf("watchdog: SW timer is %dms, HW timer is %ds\n",
stimer_duration, htimer_duration * 1000);
stimer_duration, htimer_duration);
#endif
write_pidfile_std_path_and_ext("watchdog");