svn commit: r253719 - in head: sys/conf sys/dev/watchdog sys/libkern sys/sys usr.sbin/watchdogd
Glen Barber
gjb at FreeBSD.org
Tue Jul 30 11:22:17 UTC 2013
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20130730/8b11abbe/attachment.sig>
More information about the svn-src-head
mailing list