From owner-svn-src-all@freebsd.org Fri Oct 25 16:03:54 2019 Return-Path: Delivered-To: svn-src-all@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 A766717476A; Fri, 25 Oct 2019 16:03:54 +0000 (UTC) (envelope-from agapon@gmail.com) Received: from mail-lj1-f196.google.com (mail-lj1-f196.google.com [209.85.208.196]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 47086x1QXtz3CnP; Fri, 25 Oct 2019 16:03:52 +0000 (UTC) (envelope-from agapon@gmail.com) Received: by mail-lj1-f196.google.com with SMTP id 139so3119961ljf.1; Fri, 25 Oct 2019 09:03:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:openpgp:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=GYqBfrWc5dSH51xNwAjbZr3KIG2z9Qh3PYDnrdKAJYk=; b=UrW8S/Hyg/j3cPrzDn4R06MsAl3EATAXUA3Ghb3y2ukxOw+zqb3TQRp0WUXOlqyQZP ckzKghSsPiZ87fYoLz2F5Mqo1/U2a8ly7G+3+hFKz+1ykz3702V7CcptMAKx7Ck/Ckl2 cH7m5T7JWj3O+uIlWTJLFHftBv0wNBuAK090iDlAAtvQr33WBuQYB3ov1gOi3rz5e8zB 6pX9XQkpy34o7gDXzJkmA7et/WFywlXI3fVsPXPchCeTehhI2T7K78rT31XCnETiKRRK GcAXcBSKSDFnWEzGEeCjpaXngYFtpkA7B0U2pJtqwFB09VJfkO/eJ4EY3R6N6dHP5w/Q g3Kw== X-Gm-Message-State: APjAAAVNDVwymr2bo6wnZF7v8TvxR8FdESRJFrdtRNNIMJ7El2ZsbM4B dHTABMdhsgx9EHVEUJ8gEvj86GGRlOA= X-Google-Smtp-Source: APXvYqxbmQgF3ODMKNMtv51MhZOYxzOeCb23nyRo+rWojdi8YE1S2AP6jLcpkS9SIBrRp51eXxVaMw== X-Received: by 2002:a2e:3617:: with SMTP id d23mr3040338lja.169.1572019429392; Fri, 25 Oct 2019 09:03:49 -0700 (PDT) Received: from [192.168.0.88] (east.meadow.volia.net. [93.72.151.96]) by smtp.googlemail.com with ESMTPSA id b2sm641883ljk.64.2019.10.25.09.03.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 25 Oct 2019 09:03:48 -0700 (PDT) Subject: Re: svn commit: r354076 - head/sys/dev/ow To: Ian Lepore , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201910251538.x9PFc9ii028313@repo.freebsd.org> <2fde5b7b-6186-0969-08a5-83524b6aa274@FreeBSD.org> <0877c65ce11b0509dcc9e3ee491a37cc2a488f93.camel@freebsd.org> From: Andriy Gapon Openpgp: preference=signencrypt Autocrypt: addr=avg@FreeBSD.org; prefer-encrypt=mutual; keydata= mQINBFm4LIgBEADNB/3lT7f15UKeQ52xCFQx/GqHkSxEdVyLFZTmY3KyNPQGBtyvVyBfprJ7 mAeXZWfhat6cKNRAGZcL5EmewdQuUfQfBdYmKjbw3a9GFDsDNuhDA2QwFt8BmkiVMRYyvI7l N0eVzszWCUgdc3qqM6qqcgBaqsVmJluwpvwp4ZBXmch5BgDDDb1MPO8AZ2QZfIQmplkj8Y6Z AiNMknkmgaekIINSJX8IzRzKD5WwMsin70psE8dpL/iBsA2cpJGzWMObVTtCxeDKlBCNqM1i gTXta1ukdUT7JgLEFZk9ceYQQMJJtUwzWu1UHfZn0Fs29HTqawfWPSZVbulbrnu5q55R4PlQ /xURkWQUTyDpqUvb4JK371zhepXiXDwrrpnyyZABm3SFLkk2bHlheeKU6Yql4pcmSVym1AS4 dV8y0oHAfdlSCF6tpOPf2+K9nW1CFA8b/tw4oJBTtfZ1kxXOMdyZU5fiG7xb1qDgpQKgHUX8 7Rd2T1UVLVeuhYlXNw2F+a2ucY+cMoqz3LtpksUiBppJhw099gEXehcN2JbUZ2TueJdt1FdS ztnZmsHUXLxrRBtGwqnFL7GSd6snpGIKuuL305iaOGODbb9c7ne1JqBbkw1wh8ci6vvwGlzx rexzimRaBzJxlkjNfMx8WpCvYebGMydNoeEtkWldtjTNVsUAtQARAQABtB5BbmRyaXkgR2Fw b24gPGF2Z0BGcmVlQlNELm9yZz6JAlQEEwEIAD4WIQS+LEO7ngQnXA4Bjr538m7TUc1yjwUC WbgsiAIbIwUJBaOagAULCQgHAgYVCAkKCwIEFgIDAQIeAQIXgAAKCRB38m7TUc1yj+JAEACV l9AK/nOWAt/9cufV2fRj0hdOqB1aCshtSrwHk/exXsDa4/FkmegxXQGY+3GWX3deIyesbVRL rYdtdK0dqJyT1SBqXK1h3/at9rxr9GQA6KWOxTjUFURsU7ok/6SIlm8uLRPNKO+yq0GDjgaO LzN+xykuBA0FlhQAXJnpZLcVfPJdWv7sSHGedL5ln8P8rxR+XnmsA5TUaaPcbhTB+mG+iKFj GghASDSfGqLWFPBlX/fpXikBDZ1gvOr8nyMY9nXhgfXpq3B6QCRYKPy58ChrZ5weeJZ29b7/ QdEO8NFNWHjSD9meiLdWQaqo9Y7uUxN3wySc/YUZxtS0bhAd8zJdNPsJYG8sXgKjeBQMVGuT eCAJFEYJqbwWvIXMfVWop4+O4xB+z2YE3jAbG/9tB/GSnQdVSj3G8MS80iLS58frnt+RSEw/ psahrfh0dh6SFHttE049xYiC+cM8J27Aaf0i9RflyITq57NuJm+AHJoU9SQUkIF0nc6lfA+o JRiyRlHZHKoRQkIg4aiKaZSWjQYRl5Txl0IZUP1dSWMX4s3XTMurC/pnja45dge/4ESOtJ9R 8XuIWg45Oq6MeIWdjKddGhRj3OohsltKgkEU3eLKYtB6qRTQypHHUawCXz88uYt5e3w4V16H lCpSTZV/EVHnNe45FVBlvK7k7HFfDDkryLkCDQRZuCyIARAAlq0slcsVboY/+IUJdcbEiJRW be9HKVz4SUchq0z9MZPX/0dcnvz/gkyYA+OuM78dNS7Mbby5dTvOqfpLJfCuhaNYOhlE0wY+ 1T6Tf1f4c/uA3U/YiadukQ3+6TJuYGAdRZD5EqYFIkreARTVWg87N9g0fT9BEqLw9lJtEGDY EWUE7L++B8o4uu3LQFEYxcrb4K/WKmgtmFcm77s0IKDrfcX4doV92QTIpLiRxcOmCC/OCYuO jB1oaaqXQzZrCutXRK0L5XN1Y1PYjIrEzHMIXmCDlLYnpFkK+itlXwlE2ZQxkfMruCWdQXye syl2fynAe8hvp7Mms9qU2r2K9EcJiR5N1t1C2/kTKNUhcRv7Yd/vwusK7BqJbhlng5ZgRx0m WxdntU/JLEntz3QBsBsWM9Y9wf2V4tLv6/DuDBta781RsCB/UrU2zNuOEkSixlUiHxw1dccI 6CVlaWkkJBxmHX22GdDFrcjvwMNIbbyfQLuBq6IOh8nvu9vuItup7qemDG3Ms6TVwA7BD3j+ 3fGprtyW8Fd/RR2bW2+LWkMrqHffAr6Y6V3h5kd2G9Q8ZWpEJk+LG6Mk3fhZhmCnHhDu6CwN MeUvxXDVO+fqc3JjFm5OxhmfVeJKrbCEUJyM8ESWLoNHLqjywdZga4Q7P12g8DUQ1mRxYg/L HgZY3zfKOqcAEQEAAYkCPAQYAQgAJhYhBL4sQ7ueBCdcDgGOvnfybtNRzXKPBQJZuCyIAhsM BQkFo5qAAAoJEHfybtNRzXKPBVwQAKfFy9P7N3OsLDMB56A4Kf+ZT+d5cIx0Yiaf4n6w7m3i ImHHHk9FIetI4Xe54a2IXh4Bq5UkAGY0667eIs+Z1Ea6I2i27Sdo7DxGwq09Qnm/Y65ADvXs 3aBvokCcm7FsM1wky395m8xUos1681oV5oxgqeRI8/76qy0hD9WR65UW+HQgZRIcIjSel9vR XDaD2HLGPTTGr7u4v00UeTMs6qvPsa2PJagogrKY8RXdFtXvweQFz78NbXhluwix2Tb9ETPk LIpDrtzV73CaE2aqBG/KrboXT2C67BgFtnk7T7Y7iKq4/XvEdDWscz2wws91BOXuMMd4c/c4 OmGW9m3RBLufFrOag1q5yUS9QbFfyqL6dftJP3Zq/xe+mr7sbWbhPVCQFrH3r26mpmy841ym dwQnNcsbIGiBASBSKksOvIDYKa2Wy8htPmWFTEOPRpFXdGQ27awcjjnB42nngyCK5ukZDHi6 w0qK5DNQQCkiweevCIC6wc3p67jl1EMFY5+z+zdTPb3h7LeVnGqW0qBQl99vVFgzLxchKcl0 R/paSFgwqXCZhAKMuUHncJuynDOP7z5LirUeFI8qsBAJi1rXpQoLJTVcW72swZ42IdPiboqx NbTMiNOiE36GqMcTPfKylCbF45JNX4nF9ElM0E+Y8gi4cizJYBRr2FBJgay0b9Cp Message-ID: <01c62f31-1b10-6856-eb33-9238dcb0d9c6@FreeBSD.org> Date: Fri, 25 Oct 2019 19:03:47 +0300 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <0877c65ce11b0509dcc9e3ee491a37cc2a488f93.camel@freebsd.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 47086x1QXtz3CnP X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of agapon@gmail.com designates 209.85.208.196 as permitted sender) smtp.mailfrom=agapon@gmail.com X-Spamd-Result: default: False [-3.13 / 15.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; RCVD_TLS_ALL(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; R_SPF_ALLOW(-0.20)[+ip4:209.85.128.0/17:c]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; MIME_TRACE(0.00)[0:+]; DMARC_NA(0.00)[FreeBSD.org]; TO_DN_SOME(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; RCVD_COUNT_THREE(0.00)[3]; IP_SCORE(-1.13)[ip: (-0.34), ipnet: 209.85.128.0/17(-3.22), asn: 15169(-2.05), country: US(-0.05)]; RCVD_IN_DNSWL_NONE(0.00)[196.208.85.209.list.dnswl.org : 127.0.5.0]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; FORGED_SENDER(0.30)[avg@FreeBSD.org,agapon@gmail.com]; RWL_MAILSPIKE_POSSIBLE(0.00)[196.208.85.209.rep.mailspike.net : 127.0.0.17]; R_DKIM_NA(0.00)[]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; FROM_NEQ_ENVFROM(0.00)[avg@FreeBSD.org,agapon@gmail.com]; MID_RHS_MATCH_FROM(0.00)[]; RECEIVED_SPAMHAUS_PBL(0.00)[96.151.72.93.khpj7ygk5idzvmvt5x4ziurxhy.zen.dq.spamhaus.net : 127.0.0.10] 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: Fri, 25 Oct 2019 16:03:54 -0000 On 25/10/2019 18:56, Ian Lepore wrote: > 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). I can agree that I should have used ustosbt() instead of multiplication by SBT_1US, but I don't agree with your original message that I changed the code that correctly used the macros. But again, I know the times being converted, they are fixed by the protocol and I do not see why I have to use this: static __inline sbintime_t ustosbt(int64_t _us) { sbintime_t sb = 0; #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); } instead of this x * (((sbintime_t)1 << 32) / 1000000) The times: static struct ow_timing timing_regular = { .t_slot = 60, /* 60 to 120 */ .t_low0 = 60, /* really 60 to 120 */ .t_low1 = 1, /* really 1 to 15 */ .t_release = 45, /* <= 45us */ .t_rec = 1, /* at least 1us */ .t_rdv = 15, /* 15us */ .t_rstl = 480, /* 480us or more */ .t_rsth = 480, /* 480us or more */ .t_pdl = 60, /* 60us to 240us */ .t_pdh = 60, /* 15us to 60us */ .t_lowr = 1, /* 1us */ }; -- Andriy Gapon