On Sat, Jul 27, 2013 at 08:47:02PM +0000, Alfred Perlstein wrote: > Author: alfred > Date: Sat Jul 27 20:47:01 2013 > New Revision: 253719 > URL: http://svnweb.freebsd.org/changeset/base/253719 > > Log: > Fix watchdog pretimeout. > > The original API calls for pow2ns, however the new APIs from > Linux call for seconds. > > We need to be able to convert to/from 2^Nns to seconds in both > userland and kernel to fix this and properly compare units. > > Added: > head/sys/libkern/flsll.c (contents, props changed) > Modified: > head/sys/conf/files > head/sys/dev/watchdog/watchdog.c > head/sys/sys/libkern.h > head/usr.sbin/watchdogd/watchdogd.c > > Modified: head/sys/conf/files > ============================================================================== > --- head/sys/conf/files Sat Jul 27 20:15:18 2013 (r253718) > +++ head/sys/conf/files Sat Jul 27 20:47:01 2013 (r253719) > @@ -2976,6 +2976,7 @@ libkern/arc4random.c standard > libkern/bcd.c standard > libkern/bsearch.c standard > libkern/crc32.c standard > +libkern/flsll.c standard > libkern/fnmatch.c standard > libkern/iconv.c optional libiconv > libkern/iconv_converter_if.m optional libiconv > > Modified: head/sys/dev/watchdog/watchdog.c > ============================================================================== > --- head/sys/dev/watchdog/watchdog.c Sat Jul 27 20:15:18 2013 > (r253718) > +++ head/sys/dev/watchdog/watchdog.c Sat Jul 27 20:47:01 2013 > (r253719) > @@ -39,6 +39,7 @@ __FBSDID("$FreeBSD$"); > #include <sys/kernel.h> > #include <sys/malloc.h> > #include <sys/module.h> > +#include <sys/sysctl.h> > #include <sys/syslog.h> > #include <sys/watchdog.h> > #include <sys/bus.h> > @@ -60,10 +61,56 @@ static int wd_softtimeout_act = WD_SOFT_ > > static struct cdev *wd_dev; > static volatile u_int wd_last_u; /* last timeout value set by kern_do_pat > */ > +static u_int wd_last_u_sysctl; /* last timeout value set by kern_do_pat */ > +static u_int wd_last_u_sysctl_secs; /* wd_last_u in seconds */ > + > +SYSCTL_NODE(_hw, OID_AUTO, watchdog, CTLFLAG_RD, 0, "Main watchdog device"); > +SYSCTL_UINT(_hw_watchdog, OID_AUTO, wd_last_u, CTLFLAG_RD, > + &wd_last_u_sysctl, 0, "Watchdog last update time"); > +SYSCTL_UINT(_hw_watchdog, OID_AUTO, wd_last_u_secs, CTLFLAG_RD, > + &wd_last_u_sysctl_secs, 0, "Watchdog last update time"); > > static int wd_lastpat_valid = 0; > static time_t wd_lastpat = 0; /* when the watchdog was last patted */ > > +static void > +pow2ns_to_ts(int pow2ns, struct timespec *ts) > +{ > + uint64_t ns; > + > + ns = 1ULL << pow2ns; > + ts->tv_sec = ns / 1000000000ULL; > + ts->tv_nsec = ns % 1000000000ULL; > +} > + > +static int > +pow2ns_to_ticks(int pow2ns) > +{ > + struct timeval tv; > + struct timespec ts; > + > + pow2ns_to_ts(pow2ns, &ts); > + TIMESPEC_TO_TIMEVAL(&tv, &ts); > + return (tvtohz(&tv)); > +} > + > +static int > +seconds_to_pow2ns(int seconds) > +{ > + uint64_t power; > + uint64_t ns; > + uint64_t shifted; > + > + ns = ((uint64_t)seconds) * 1000000000ULL; > + power = flsll(ns); > + shifted = 1ULL << power; > + if (shifted <= ns) { > + power++; > + } > + return (power); > +} > + > + > int > wdog_kern_pat(u_int utim) > { > @@ -86,6 +133,8 @@ wdog_kern_pat(u_int utim) > * This can be zero (to disable the watchdog) > */ > wd_last_u = (utim & WD_INTERVAL); > + wd_last_u_sysctl = wd_last_u; > + wd_last_u_sysctl_secs = pow2ns_to_ticks(wd_last_u) / hz; > } > if ((utim & WD_INTERVAL) == WD_TO_NEVER) { > utim = 0; > @@ -101,7 +150,7 @@ wdog_kern_pat(u_int utim) > callout_stop(&wd_softtimeo_handle); > } else { > (void) callout_reset(&wd_softtimeo_handle, > - hz*utim, wd_timeout_cb, "soft"); > + pow2ns_to_ticks(utim), wd_timeout_cb, "soft"); > } > error = 0; > } else { > @@ -201,10 +250,13 @@ static int > wd_set_pretimeout(int newtimeout, int disableiftoolong) > { > u_int utime; > + struct timespec utime_ts; > + int timeout_ticks; > > utime = wdog_kern_last_timeout(); > + pow2ns_to_ts(utime, &utime_ts); > /* do not permit a pre-timeout >= than the timeout. */ > - if (newtimeout >= utime) { > + if (newtimeout >= utime_ts.tv_sec) { > /* > * If 'disableiftoolong' then just fall through > * so as to disable the pre-watchdog > @@ -222,8 +274,22 @@ wd_set_pretimeout(int newtimeout, int di > return 0; > } > > + timeout_ticks = pow2ns_to_ticks(utime) - (hz*newtimeout); > +#if 0 > + printf("wd_set_pretimeout: " > + "newtimeout: %d, " > + "utime: %d -> utime_ticks: %d, " > + "hz*newtimeout: %d, " > + "timeout_ticks: %d -> sec: %d\n", > + newtimeout, > + utime, pow2ns_to_ticks(utime), > + hz*newtimeout, > + timeout_ticks, timeout_ticks / hz); > +#endif > + > /* We determined the value is sane, so reset the callout */ > - (void) callout_reset(&wd_pretimeo_handle, hz*(utime - newtimeout), > + (void) callout_reset(&wd_pretimeo_handle, > + timeout_ticks, > wd_timeout_cb, "pre-timeout"); > wd_pretimeout = newtimeout; > return 0; > @@ -282,7 +348,7 @@ wd_ioctl(struct cdev *dev __unused, u_lo > break; > case WDIOC_SETTIMEOUT: > u = *(u_int *)data; > - error = wdog_kern_pat(u); > + error = wdog_kern_pat(seconds_to_pow2ns(u)); > break; > case WDIOC_GETTIMEOUT: > u = wdog_kern_last_timeout(); > > Added: head/sys/libkern/flsll.c > ============================================================================== > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ head/sys/libkern/flsll.c Sat Jul 27 20:47:01 2013 (r253719) > @@ -0,0 +1,47 @@ > +/*- > + * Copyright (c) 1990, 1993 > + * The Regents of the University of California. All rights reserved. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in the > + * documentation and/or other materials provided with the distribution. > + * 4. Neither the name of the University nor the names of its contributors > + * may be used to endorse or promote products derived from this software > + * without specific prior written permission. > + * > + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE > + * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. > + */ > + > +#include <sys/cdefs.h> > +#include <sys/libkern.h> > +__FBSDID("$FreeBSD$"); > + > +/* > + * Find Last Set bit > + */ > +int > +flsll(long long mask) > +{ > + int bit; > + > + if (mask == 0) > + return (0); > + for (bit = 1; mask != 1; bit++) > + mask = (unsigned long long)mask >> 1; > + return (bit); > +} > > Modified: head/sys/sys/libkern.h > ============================================================================== > --- head/sys/sys/libkern.h Sat Jul 27 20:15:18 2013 (r253718) > +++ head/sys/sys/libkern.h Sat Jul 27 20:47:01 2013 (r253719) > @@ -94,6 +94,10 @@ int fls(int); > #ifndef HAVE_INLINE_FLSL > int flsl(long); > #endif > +#ifndef HAVE_INLINE_FLSLL > +int flsll(long long); > +#endif > + > int fnmatch(const char *, const char *, int); > int locc(int, char *, u_int); > void *memchr(const void *s, int c, size_t n); > > Modified: head/usr.sbin/watchdogd/watchdogd.c > ============================================================================== > --- head/usr.sbin/watchdogd/watchdogd.c Sat Jul 27 20:15:18 2013 > (r253718) > +++ head/usr.sbin/watchdogd/watchdogd.c Sat Jul 27 20:47:01 2013 > (r253719) > @@ -39,6 +39,7 @@ __FBSDID("$FreeBSD$"); > #include <sys/rtprio.h> > #include <sys/stat.h> > #include <sys/time.h> > +#include <sys/sysctl.h> > #include <sys/watchdog.h> > > #include <err.h> > @@ -58,19 +59,24 @@ __FBSDID("$FreeBSD$"); > > #include <getopt.h> > > +static long fetchtimeout(int opt, const char *longopt, const char > *myoptarg); > static void parseargs(int, char *[]); > +static int seconds_to_pow2ns(int); > static void sighandler(int); > static void watchdog_loop(void); > static int watchdog_init(void); > static int watchdog_onoff(int onoff); > static int watchdog_patpat(u_int timeout); > static void usage(void); > +static int tstotv(struct timeval *tv, struct timespec *ts); > +static int tvtohz(struct timeval *tv); > > static int debugging = 0; > static int end_program = 0; > static const char *pidfile = _PATH_VARRUN "watchdogd.pid"; > static u_int timeout = WD_TO_128SEC; > static u_int pretimeout = 0; > +static u_int timeout_sec; > static u_int passive = 0; > static int is_daemon = 0; > static int is_dry_run = 0; /* do not arm the watchdog, only > @@ -183,6 +189,59 @@ main(int argc, char *argv[]) > } > } > > +static void > +pow2ns_to_ts(int pow2ns, struct timespec *ts) > +{ > + uint64_t ns; > + > + ns = 1ULL << pow2ns; > + ts->tv_sec = ns / 1000000000ULL; > + ts->tv_nsec = ns % 1000000000ULL; > +} > + > +/* > + * Convert a timeout in seconds to N where 2^N nanoseconds is close to > + * "seconds". > + * > + * The kernel expects the timeouts for watchdogs in "2^N nanosecond format". > + */ > +static u_int > +parse_timeout_to_pow2ns(char opt, const char *longopt, const char *myoptarg) > +{ > + double a; > + u_int rv; > + struct timespec ts; > + struct timeval tv; > + int ticks; > + char shortopt[] = "- "; > + > + if (!longopt) > + shortopt[1] = opt; > + > + a = fetchtimeout(opt, longopt, myoptarg); > + > + if (a == 0) > + rv = WD_TO_NEVER; > + else > + rv = seconds_to_pow2ns(a); > + pow2ns_to_ts(rv, &ts); > + tstotv(&tv, &ts); > + ticks = tvtohz(&tv); > + if (debugging) { > + printf("Timeout for %s%s " > + "is 2^%d nanoseconds " > + "(in: %s sec -> out: %ld sec %ld ns -> %d ticks)\n", > + longopt ? "-" : "", longopt ? longopt : shortopt, > + rv, > + myoptarg, ts.tv_sec, ts.tv_nsec, ticks); > + } > + if (ticks <= 0) { > + errx(1, "Timeout for %s%s is too small, please choose a higher > timeout.", longopt ? "-" : "", longopt ? longopt : shortopt); > + } > + > + return (rv); > +} > + > /* > * Catch signals and begin shutdown process. > */ > @@ -513,6 +572,110 @@ timeout_act_str2int(const char *lopt, co > return rv; > } > > +int > +tstotv(struct timeval *tv, struct timespec *ts) > +{ > + > + tv->tv_sec = ts->tv_sec; > + tv->tv_usec = ts->tv_nsec / 1000; > + return 0; > +} > + > +/* > + * Convert a timeval to a number of ticks. > + * Mostly copied from the kernel. > + */ > +int > +tvtohz(struct timeval *tv) > +{ > + register unsigned long ticks; > + register long sec, usec; > + int hz; > + size_t hzsize; > + int error; > + int tick; > + > + hzsize = sizeof(hz); > + > + error = sysctlbyname("kern.hz", &hz, &hzsize, NULL, 0); > + if (error) > + err(1, "sysctlbyname kern.hz"); > + > + tick = 1000000 / hz; > + > + /* > + * If the number of usecs in the whole seconds part of the time > + * difference fits in a long, then the total number of usecs will > + * fit in an unsigned long. Compute the total and convert it to > + * ticks, rounding up and adding 1 to allow for the current tick > + * to expire. Rounding also depends on unsigned long arithmetic > + * to avoid overflow. > + * > + * Otherwise, if the number of ticks in the whole seconds part of > + * the time difference fits in a long, then convert the parts to > + * ticks separately and add, using similar rounding methods and > + * overflow avoidance. This method would work in the previous > + * case but it is slightly slower and assumes that hz is integral. > + * > + * Otherwise, round the time difference down to the maximum > + * representable value. > + * > + * If ints have 32 bits, then the maximum value for any timeout in > + * 10ms ticks is 248 days. > + */ > + sec = tv->tv_sec; > + usec = tv->tv_usec; > + if (usec < 0) { > + sec--; > + usec += 1000000; > + } > + if (sec < 0) { > +#ifdef DIAGNOSTIC > + if (usec > 0) { > + sec++; > + usec -= 1000000; > + } > + printf("tvotohz: negative time difference %ld sec %ld usec\n", > + sec, usec); > +#endif > + ticks = 1; > + } else if (sec <= LONG_MAX / 1000000) > + ticks = (sec * 1000000 + (unsigned long)usec + (tick - 1)) > + / tick + 1; > + else if (sec <= LONG_MAX / hz) > + ticks = sec * hz > + + ((unsigned long)usec + (tick - 1)) / tick + 1; > + else > + ticks = LONG_MAX; > + if (ticks > INT_MAX) > + ticks = INT_MAX; > + return ((int)ticks); > +} > + > +static int > +seconds_to_pow2ns(int seconds) > +{ > + uint64_t power; > + uint64_t ns; > + uint64_t shifted; > + > + if (seconds <= 0) > + errx(1, "seconds %d < 0", seconds); > + ns = ((uint64_t)seconds) * 1000000000ULL; > + power = flsll(ns); > + shifted = 1ULL << power; > + if (shifted <= ns) { > + power++; > + } > + if (debugging) { > + printf("shifted %lld\n", (long long)shifted); > + printf("seconds_to_pow2ns: seconds: %d, ns %lld, power %d\n", > + seconds, (long long)ns, (int)power); > + } > + return (power); > +} > + > + > /* > * Handle the few command line arguments supported. > */ > @@ -521,9 +684,7 @@ parseargs(int argc, char *argv[]) > { > int longindex; > int c; > - char *p; > const char *lopt; > - double a; > > /* > * if we end with a 'd' aka 'watchdogd' then we are the daemon program, > @@ -565,21 +726,11 @@ parseargs(int argc, char *argv[]) > do_syslog = 0; > break; > case 't': > - p = NULL; > - errno = 0; > - a = strtod(optarg, &p); > - if ((p != NULL && *p != '\0') || errno != 0) > - errx(EX_USAGE, "-t argument is not a number"); > - if (a < 0) > - errx(EX_USAGE, "-t argument must be positive"); > - > - if (a == 0) > - timeout = WD_TO_NEVER; > - else > - timeout = flsll(a * 1e9); > - if (debugging) > - printf("Timeout is 2^%d nanoseconds\n", > - timeout); > + timeout_sec = atoi(optarg); > + timeout = parse_timeout_to_pow2ns(c, NULL, optarg); > + if (debugging) > + printf("Timeout is 2^%d nanoseconds\n", > + timeout); > break; > case 'T': > carp_thresh_seconds = fetchtimeout(c, "NULL", optarg); > @@ -618,4 +769,15 @@ parseargs(int argc, char *argv[]) > errx(EX_USAGE, "extra arguments."); > if (is_daemon && timeout < WD_TO_1SEC) > errx(EX_USAGE, "-t argument is less than one second."); > + if (pretimeout_set) { > + struct timespec ts; > + > + pow2ns_to_ts(timeout, &ts); > + if (pretimeout >= ts.tv_sec) { > + errx(EX_USAGE, > + "pretimeout (%d) >= timeout (%d -> %ld)\n" > + "see manual section TIMEOUT RESOLUTION", > + pretimeout, timeout_sec, (long)ts.tv_sec); > + } > + }
Come on. It has been 3 days that tinderbox complains about this.
===> usr.sbin/watchdogd (all)
cc -O2 -pipe -std=gnu99 -Qunused-arguments -fstack-protector
-Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter
-Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wreturn-type
-Wcast-qual -Wwrite-strings -Wswitch -Wshadow -Wunused-parameter
-Wcast-align -Wchar-subscripts -Winline -Wnested-externs
-Wredundant-decls -Wold-style-definition -Wmissing-variable-declarations
-Wno-pointer-sign -Wno-empty-body -Wno-string-plus-int -c
/src/usr.sbin/watchdogd/watchdogd.c
/src/usr.sbin/watchdogd/watchdogd.c:777:18: error: comparison of
integers of different signs: 'u_int' (aka 'unsigned int') and 'time_t'
(aka 'int') [-Werror,-Wsign-compare]
if (pretimeout >= ts.tv_sec) {
~~~~~~~~~~ ^ ~~~~~~~~~
Glen
pgpVcYTBaSJ0O.pgp
Description: PGP signature
