From owner-svn-src-head@freebsd.org Fri Oct 25 15:56:20 2019 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 54DBF174462 for ; Fri, 25 Oct 2019 15:56:20 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from outbound3d.ore.mailhop.org (outbound3d.ore.mailhop.org [54.186.57.195]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4707yD0fbwz3CN5 for ; Fri, 25 Oct 2019 15:56:19 +0000 (UTC) (envelope-from ian@freebsd.org) ARC-Seal: i=1; a=rsa-sha256; t=1572018977; cv=none; d=outbound.mailhop.org; s=arc-outbound20181012; b=h5ct80jdIctMuDRxbR/iMVwZkYn7Nkoie4Tv9MSVQD3ZPSwL2ajN6Rj4VFzR1nfIJ6yENdFWPiGBT MFXflWDCsOYn4RS74ruijMcNz7EMcFJru2t+WcS658xcT9vzvFLmiXE0FElAnpEL1yfpmSIcXhVb+T 4nehOJQ8pEGIsZ1X6PtQH5IpqxGvym9R2WGFQpM7rFuZgbVQXZq9/FYcF8pzaazt+laV0qtZ630sSX vIpCMUvM+DiKYLv9MGlRKSM8ZGGS1dbjiK+wFl7sok+Njs0+zQ+FjB2D+j1yYKWObao2krXFvjxKAB ekmvA6BmrhEaEcvOugdVEY0a2cpRRxQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=outbound.mailhop.org; s=arc-outbound20181012; h=content-transfer-encoding:mime-version:content-type:references:in-reply-to: date:to:from:subject:message-id:dkim-signature:from; bh=Q8RMfUnzVUfcbCTWzUmi3r1VqWc4wptLSpGU0mMHrt0=; b=ac6B0hWJsfqSuq1ScatdVqZYbbHOJdlgUYGchIvWpwTr5gsRzEhlLC1ZJvNCx6Pfazc1J6LXB0X7i 4POAFMFiuIgJYmAvSqlE5ph5i/KPiovOfClWwAeUnt2ZmvTOIiW08FOr7BQhWC4pK5dmGNKP/S6Hdb FAFfdZZtPoX27c22aMW1gwlIQ62g0MPQjAUjKRjvJ5PCuy7xvyL6D1NohffImrz+f9Z8WcjuiHCwhq Du5U4qfirk/CbZmeK2p7YdDkSOkhUAJMoviIdRaeMgy/f7Yb3yQmr2mcpITf57sT9uQhFAUrxSeN0Q dboKM/IYOgNlwWoi0Zt3FxjtUzy2S2A== ARC-Authentication-Results: i=1; outbound3.ore.mailhop.org; spf=softfail smtp.mailfrom=freebsd.org smtp.remote-ip=67.177.211.60; dmarc=none header.from=freebsd.org; arc=none header.oldest-pass=0; DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outbound.mailhop.org; s=dkim-high; h=content-transfer-encoding:mime-version:content-type:references:in-reply-to: date:to:from:subject:message-id:from; bh=Q8RMfUnzVUfcbCTWzUmi3r1VqWc4wptLSpGU0mMHrt0=; b=JDA7eg8WJkwNHSUGto2ECluBTSiaVtZGhs141VXOEz4lx6iRSWg5Gd6OVw5p9gZeHtRvV4rfiXG3q iyMmhZcNVFNscrGxnwWcaI/LrNiA7ZzMaltCm7GTNt8LUm90wFXQPm0bw/OBaX8o24tHAA3Ecy4kZ7 RuFQp6fgWE4d+IptMGwRfo2a2i3bL9DMUEgQ/Wh+GJnP0nB5ejfvKSUOw7tQIWC2kT30b9ZM8g/Dh6 RrS7q413cc7trZZ7Ovnz5RrlnImFNIZkJA8UGcrA/hW6yXKjJ15Q3jGzJZF27fB9awPxuRdS7W2q9L KGsI8v8rxJJ+XMmkeMtU6f7fSof7EzQ== X-MHO-RoutePath: aGlwcGll X-MHO-User: f972ee63-f73f-11e9-b80b-052b4a66b6b2 X-Report-Abuse-To: https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information X-Originating-IP: 67.177.211.60 X-Mail-Handler: DuoCircle Outbound SMTP Received: from ilsoft.org (unknown [67.177.211.60]) by outbound3.ore.mailhop.org (Halon) with ESMTPSA id f972ee63-f73f-11e9-b80b-052b4a66b6b2; Fri, 25 Oct 2019 15:56:16 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.15.2) with ESMTP id x9PFuEgQ049723; Fri, 25 Oct 2019 09:56:14 -0600 (MDT) (envelope-from ian@freebsd.org) Message-ID: <0877c65ce11b0509dcc9e3ee491a37cc2a488f93.camel@freebsd.org> Subject: Re: svn commit: r354076 - head/sys/dev/ow From: Ian Lepore To: Andriy Gapon , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Date: Fri, 25 Oct 2019 09:56:14 -0600 In-Reply-To: <2fde5b7b-6186-0969-08a5-83524b6aa274@FreeBSD.org> References: <201910251538.x9PFc9ii028313@repo.freebsd.org> <2fde5b7b-6186-0969-08a5-83524b6aa274@FreeBSD.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 FreeBSD GNOME Team Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 4707yD0fbwz3CN5 X-Spamd-Bar: - Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-1.89 / 15.00]; local_wl_from(0.00)[freebsd.org]; NEURAL_HAM_MEDIUM(-0.89)[-0.892,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; ASN(0.00)[asn:16509, ipnet:54.186.0.0/15, country:US] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 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: Fri, 25 Oct 2019 15:56:20 -0000 On Fri, 2019-10-25 at 18:51 +0300, Andriy Gapon wrote: > On 25/10/2019 18:46, Ian Lepore wrote: > > On Fri, 2019-10-25 at 15:38 +0000, Andriy Gapon wrote: > > > Author: avg > > > Date: Fri Oct 25 15:38:09 2019 > > > New Revision: 354076 > > > URL: https://svnweb.freebsd.org/changeset/base/354076 > > > > > > Log: > > > owc_gpiobus_read_data: compare times in sbintime_t units > > > > > > Previously the code used sbttous() before microseconds > > > comparison > > > in one > > > place, sbttons() and nanoseconds in another, division by > > > SBT_1US > > > and > > > microseconds in yet another. > > > > > > Now the code consistently uses multiplication by SBT_1US to > > > convert > > > microseconds to sbintime_t before comparing them with periods > > > between > > > calls to sbinuptime(). This is fast, this is precise enough > > > (below > > > 0.03%) and the periods defined by the protocol cannot overflow. > > > > > > Reviewed by: imp (D22108) > > > MFC after: 2 weeks > > > > > > Modified: > > > head/sys/dev/ow/owc_gpiobus.c > > > > > > Modified: head/sys/dev/ow/owc_gpiobus.c > > > ================================================================= > > > ==== > > > ========= > > > --- head/sys/dev/ow/owc_gpiobus.c Fri Oct 25 15:02:50 2019 ( > > > r354 > > > 075) > > > +++ head/sys/dev/ow/owc_gpiobus.c Fri Oct 25 15:38:09 2019 ( > > > r354 > > > 076) > > > @@ -296,10 +296,10 @@ owc_gpiobus_read_data(device_t dev, struct > > > ow_timing * > > > do { > > > now = sbinuptime(); > > > GETPIN(sc, &sample); > > > - } while (sbttous(now - then) < t->t_rdv + 2 && sample == 0); > > > + } while (now - then < (t->t_rdv + 2) * SBT_1US && sample == 0); > > > critical_exit(); > > > > > > - if (sbttons(now - then) < t->t_rdv * 1000) > > > + if (now - then < t->t_rdv * SBT_1US) > > > *bit = 1; > > > else > > > *bit = 0; > > > @@ -307,7 +307,7 @@ owc_gpiobus_read_data(device_t dev, struct > > > ow_timing * > > > /* Wait out the rest of t_slot */ > > > do { > > > now = sbinuptime(); > > > - } while ((now - then) / SBT_1US < t->t_slot); > > > + } while (now - then < t->t_slot * SBT_1US); > > > > > > RELBUS(sc); > > > > > > > Unit conversions with sbt times should be done using the macros > > that > > carefully avoid roundoff errors. I don't understand why you've > > changed > > the code that correctly did use those macros to inline math. > > I think that the commit message explains it: > This is fast, this is precise enough (below 0.03%) and the periods > defined by > the protocol cannot overflow. > > Do you disagree? > Could you please explain in which of the three lines changed the new > code is > worse and why? > I absolutely disagree (or I wouldn't have replied). Unit conversions using sbt times should use the predefined macros, NOT incline multiplication and division. I don't know how to say it more clearly than that. The conversion macros are idiomatic (at least, they would be if people stopped writing inline conversion expressions). -- Ian