From owner-svn-src-all@freebsd.org Tue Nov 20 07:11:24 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A89581102E41; Tue, 20 Nov 2018 07:11:24 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 50E087D617; Tue, 20 Nov 2018 07:11:24 +0000 (UTC) (envelope-from imp@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 mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 30C3F6116; Tue, 20 Nov 2018 07:11:24 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id wAK7BOGr092248; Tue, 20 Nov 2018 07:11:24 GMT (envelope-from imp@FreeBSD.org) Received: (from imp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id wAK7BOcX092247; Tue, 20 Nov 2018 07:11:24 GMT (envelope-from imp@FreeBSD.org) Message-Id: <201811200711.wAK7BOcX092247@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: imp set sender to imp@FreeBSD.org using -f From: Warner Losh Date: Tue, 20 Nov 2018 07:11:24 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r340664 - head/sys/sys X-SVN-Group: head X-SVN-Commit-Author: imp X-SVN-Commit-Paths: head/sys/sys X-SVN-Commit-Revision: 340664 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 50E087D617 X-Spamd-Result: default: False [0.50 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_SPAM_LONG(0.00)[0.001,0]; NEURAL_SPAM_MEDIUM(0.03)[0.027,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US]; NEURAL_SPAM_SHORT(0.48)[0.475,0] X-Rspamd-Server: mx1.freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Nov 2018 07:11:24 -0000 Author: imp Date: Tue Nov 20 07:11:23 2018 New Revision: 340664 URL: https://svnweb.freebsd.org/changeset/base/340664 Log: Ensure that all values of ns, us and ms work for {n,u,m}stosbt Integer overflows and wrong constants limited the accuracy of these functions and created situatiosn where sbttoXs(Xstosbt(Y)) != Y. This was especailly true in the ns case where we had millions of values that were wrong. Instead, used fixed constants because there's no way to say ceil(X) for integer math. Document what these crazy constants are. Also, use a shift one fewer left to avoid integer overflow causing incorrect results, and adjust the equasion accordingly. Document this. Allow times >= 1s to be well defined for these conversion functions (at least the Xstosbt). There's too many users in the tree that they work for >= 1s. This fixes a failure on boot to program firmware on the mlx4 NIC. There was a msleep(1000) in the code. Prior to my recent rounding changes, msleep(1000) worked, but msleep(1001) did not because the old code rounded to just below 2^64 and the new code rounds to just above it (overflowing, causing the msleep(1000) to really sleep 1ms). A test program to test all cases will be committed shortly. The test exaustively tries every value (thanks to bde for the test). Sponsored by: Netflix, Inc Differential Revision: https://reviews.freebsd.org/D18051 Modified: head/sys/sys/time.h Modified: head/sys/sys/time.h ============================================================================== --- head/sys/sys/time.h Tue Nov 20 01:59:57 2018 (r340663) +++ head/sys/sys/time.h Tue Nov 20 07:11:23 2018 (r340664) @@ -162,9 +162,24 @@ sbttobt(sbintime_t _sbt) * large roundoff errors which sbttons() and nstosbt() avoid. Millisecond and * microsecond functions are also provided for completeness. * - * These functions return the smallest sbt larger or equal to the number of - * seconds requested so that sbttoX(Xtosbt(y)) == y. The 1 << 32 - 1 term added - * transforms the >> 32 from floor() to ceil(). + * These functions return the smallest sbt larger or equal to the + * number of seconds requested so that sbttoX(Xtosbt(y)) == y. Unlike + * top of second computations below, which require that we tick at the + * top of second, these need to be rounded up so we do whatever for at + * least as long as requested. + * + * The naive computation we'd do is this + * ((unit * 2^64 / SIFACTOR) + 2^32-1) >> 32 + * However, that overflows. Instead, we compute + * ((unit * 2^63 / SIFACTOR) + 2^31-1) >> 32 + * and use pre-computed constants that are the ceil of the 2^63 / SIFACTOR + * term to ensure we are using exactly the right constant. We use the lesser + * evil of ull rather than a uint64_t cast to ensure we have well defined + * right shift semantics. With these changes, we get all the ns, us and ms + * conversions back and forth right. + * Note: This file is used for both kernel and userland includes, so we can't + * rely on KASSERT being defined, nor can we pollute the namespace by including + * assert.h. */ static __inline int64_t sbttons(sbintime_t _sbt) @@ -176,8 +191,18 @@ sbttons(sbintime_t _sbt) static __inline sbintime_t nstosbt(int64_t _ns) { + sbintime_t sb = 0; - return ((_ns * (((uint64_t)1 << 63) / 500000000) + (1ull << 32) - 1) >> 32); +#ifdef KASSERT + KASSERT(_ns >= 0, ("Negative values illegal for nstosbt: %jd", _ns)); +#endif + if (_ns >= SBT_1S) { + sb = (_ns / 1000000000) * SBT_1S; + _ns = _ns % 1000000000; + } + /* 9223372037 = ceil(2^63 / 1000000000) */ + sb += ((_ns * 9223372037ull) + 0x7fffffff) >> 31; + return (sb); } static __inline int64_t @@ -190,8 +215,18 @@ sbttous(sbintime_t _sbt) static __inline sbintime_t ustosbt(int64_t _us) { + sbintime_t sb = 0; - return ((_us * (((uint64_t)1 << 63) / 500000) + (1ull << 32) - 1) >> 32); +#ifdef KASSERT + KASSERT(_us >= 0, ("Negative values illegal for ustosbt: %jd", _us)); +#endif + if (_us >= SBT_1S) { + sb = (_us / 1000000) * SBT_1S; + _us = _us % 1000000; + } + /* 9223372036855 = ceil(2^63 / 1000000) */ + sb += ((_us * 9223372036855ull) + 0x7fffffff) >> 31; + return (sb); } static __inline int64_t @@ -204,8 +239,18 @@ sbttoms(sbintime_t _sbt) static __inline sbintime_t mstosbt(int64_t _ms) { + sbintime_t sb = 0; - return ((_ms * (((uint64_t)1 << 63) / 500) + (1ull << 32) - 1) >> 32); +#ifdef KASSERT + KASSERT(_ms >= 0, ("Negative values illegal for mstosbt: %jd", _ms)); +#endif + if (_ms >= SBT_1S) { + sb = (_ms / 1000) * SBT_1S; + _ms = _ms % 1000; + } + /* 9223372036854776 = ceil(2^63 / 1000) */ + sb += ((_ms * 9223372036854776ull) + 0x7fffffff) >> 31; + return (sb); } /*-