Date: Sat, 29 Jul 2017 17:00:23 +0000 (UTC) From: Ian Lepore <ian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r321686 - in head/sys: arm/arm compat/linuxkpi/common/src dev/ow sys Message-ID: <201707291700.v6TH0Nx0056634@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ian Date: Sat Jul 29 17:00:23 2017 New Revision: 321686 URL: https://svnweb.freebsd.org/changeset/base/321686 Log: Add inline functions to convert between sbintime_t and decimal time units. Use them in some existing code that is vulnerable to roundoff errors. The existing constant SBT_1NS is a honeypot, luring unsuspecting folks into writing code such as long_timeout_ns*SBT_1NS to generate the argument for a sleep call. The actual value of 1ns in sbt units is ~4.3, leading to a large roundoff error giving a shorter sleep than expected when multiplying by the trucated value of 4 in SBT_1NS. (The evil honeypot aspect becomes clear after you waste a whole day figuring out why your sleeps return early.) Modified: head/sys/arm/arm/mpcore_timer.c head/sys/compat/linuxkpi/common/src/linux_hrtimer.c head/sys/dev/ow/owc_gpiobus.c head/sys/sys/time.h Modified: head/sys/arm/arm/mpcore_timer.c ============================================================================== --- head/sys/arm/arm/mpcore_timer.c Sat Jul 29 13:54:28 2017 (r321685) +++ head/sys/arm/arm/mpcore_timer.c Sat Jul 29 17:00:23 2017 (r321686) @@ -351,7 +351,7 @@ attach_et(struct arm_tmr_softc *sc) sc->et.et_flags = ET_FLAGS_PERIODIC | ET_FLAGS_ONESHOT | ET_FLAGS_PERCPU; sc->et.et_quality = 1000; sc->et.et_frequency = sc->clkfreq; - sc->et.et_min_period = 20 * SBT_1NS; + sc->et.et_min_period = nstosbt(20); sc->et.et_max_period = 2 * SBT_1S; sc->et.et_start = arm_tmr_start; sc->et.et_stop = arm_tmr_stop; Modified: head/sys/compat/linuxkpi/common/src/linux_hrtimer.c ============================================================================== --- head/sys/compat/linuxkpi/common/src/linux_hrtimer.c Sat Jul 29 13:54:28 2017 (r321685) +++ head/sys/compat/linuxkpi/common/src/linux_hrtimer.c Sat Jul 29 17:00:23 2017 (r321686) @@ -101,8 +101,8 @@ linux_hrtimer_start_range_ns(struct hrtimer *hrtimer, { mtx_lock(&hrtimer->mtx); - callout_reset_sbt(&hrtimer->callout, time.tv64 * SBT_1NS, - nsec * SBT_1NS, hrtimer_call_handler, hrtimer, 0); + callout_reset_sbt(&hrtimer->callout, nstosbt(time.tv64), nstosbt(nsec), + hrtimer_call_handler, hrtimer, 0); hrtimer->flags |= HRTIMER_ACTIVE; mtx_unlock(&hrtimer->mtx); } Modified: head/sys/dev/ow/owc_gpiobus.c ============================================================================== --- head/sys/dev/ow/owc_gpiobus.c Sat Jul 29 13:54:28 2017 (r321685) +++ head/sys/dev/ow/owc_gpiobus.c Sat Jul 29 17:00:23 2017 (r321686) @@ -295,10 +295,10 @@ owc_gpiobus_read_data(device_t dev, struct ow_timing * do { now = sbinuptime(); GETPIN(sc, &sample); - } while ((now - then) / SBT_1US < t->t_rdv + 2 && sample == 0); + } while (sbttous(now - then) < t->t_rdv + 2 && sample == 0); critical_exit(); - if ((now - then) / SBT_1NS < t->t_rdv * 1000) + if (sbttons(now - then) < t->t_rdv * 1000) *bit = 1; else *bit = 0; Modified: head/sys/sys/time.h ============================================================================== --- head/sys/sys/time.h Sat Jul 29 13:54:28 2017 (r321685) +++ head/sys/sys/time.h Sat Jul 29 17:00:23 2017 (r321686) @@ -128,7 +128,7 @@ bintime_shift(struct bintime *_bt, int _exp) #define SBT_1M (SBT_1S * 60) #define SBT_1MS (SBT_1S / 1000) #define SBT_1US (SBT_1S / 1000000) -#define SBT_1NS (SBT_1S / 1000000000) +#define SBT_1NS (SBT_1S / 1000000000) /* beware rounding, see nstosbt() */ #define SBT_MAX 0x7fffffffffffffffLL static __inline int @@ -155,6 +155,53 @@ sbttobt(sbintime_t _sbt) return (_bt); } +/* + * Decimal<->sbt conversions. Multiplying or dividing by SBT_1NS results in + * large roundoff errors which sbttons() and nstosbt() avoid. Millisecond and + * microsecond functions are also provided for completeness. + */ +static __inline int64_t +sbttons(sbintime_t _sbt) +{ + + return ((1000000000 * _sbt) >> 32); +} + +static __inline sbintime_t +nstosbt(int64_t _ns) +{ + + return ((_ns * (((uint64_t)1 << 63) / 500000000) >> 32)); +} + +static __inline int64_t +sbttous(sbintime_t _sbt) +{ + + return ((1000000 * _sbt) >> 32); +} + +static __inline sbintime_t +ustosbt(int64_t _us) +{ + + return ((_us * (((uint64_t)1 << 63) / 500000) >> 32)); +} + +static __inline int64_t +sbttoms(sbintime_t _sbt) +{ + + return ((1000 * _sbt) >> 32); +} + +static __inline sbintime_t +mstosbt(int64_t _ms) +{ + + return ((_ms * (((uint64_t)1 << 63) / 500) >> 32)); +} + /*- * Background information: * @@ -210,7 +257,7 @@ sbttots(sbintime_t _sbt) struct timespec _ts; _ts.tv_sec = _sbt >> 32; - _ts.tv_nsec = ((uint64_t)1000000000 * (uint32_t)_sbt) >> 32; + _ts.tv_nsec = sbttons((uint32_t)_sbt); return (_ts); } @@ -218,8 +265,7 @@ static __inline sbintime_t tstosbt(struct timespec _ts) { - return (((sbintime_t)_ts.tv_sec << 32) + - (_ts.tv_nsec * (((uint64_t)1 << 63) / 500000000) >> 32)); + return (((sbintime_t)_ts.tv_sec << 32) + nstosbt(_ts.tv_nsec)); } static __inline struct timeval @@ -228,7 +274,7 @@ sbttotv(sbintime_t _sbt) struct timeval _tv; _tv.tv_sec = _sbt >> 32; - _tv.tv_usec = ((uint64_t)1000000 * (uint32_t)_sbt) >> 32; + _tv.tv_usec = sbttous((uint32_t)_sbt); return (_tv); } @@ -236,8 +282,7 @@ static __inline sbintime_t tvtosbt(struct timeval _tv) { - return (((sbintime_t)_tv.tv_sec << 32) + - (_tv.tv_usec * (((uint64_t)1 << 63) / 500000) >> 32)); + return (((sbintime_t)_tv.tv_sec << 32) + ustosbt(_tv.tv_usec)); } #endif /* __BSD_VISIBLE */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201707291700.v6TH0Nx0056634>