From owner-svn-src-head@freebsd.org Sat Jul 29 17:00:24 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id BD82DDC6FBA; Sat, 29 Jul 2017 17:00:24 +0000 (UTC) (envelope-from ian@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 7FECD71324; Sat, 29 Jul 2017 17:00:24 +0000 (UTC) (envelope-from ian@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v6TH0NZ7056638; Sat, 29 Jul 2017 17:00:23 GMT (envelope-from ian@FreeBSD.org) Received: (from ian@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v6TH0Nx0056634; Sat, 29 Jul 2017 17:00:23 GMT (envelope-from ian@FreeBSD.org) Message-Id: <201707291700.v6TH0Nx0056634@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: ian set sender to ian@FreeBSD.org using -f From: Ian Lepore Date: Sat, 29 Jul 2017 17:00:23 +0000 (UTC) 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 X-SVN-Group: head X-SVN-Commit-Author: ian X-SVN-Commit-Paths: in head/sys: arm/arm compat/linuxkpi/common/src dev/ow sys X-SVN-Commit-Revision: 321686 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 29 Jul 2017 17:00:24 -0000 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 */