From owner-freebsd-hackers@freebsd.org Wed Oct 2 17:12:32 2019 Return-Path: Delivered-To: freebsd-hackers@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 E83F613471E for ; Wed, 2 Oct 2019 17:12:32 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id 46k2kl1X4sz3F5N for ; Wed, 2 Oct 2019 17:12:31 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 8C41643DE09; Thu, 3 Oct 2019 03:12:28 +1000 (AEST) Date: Thu, 3 Oct 2019 03:12:26 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Konstantin Belousov cc: Bruce Evans , Sebastian Huber , FreeBSD Subject: Re: Why is tc_get_timecount() called two times in tc_init()? In-Reply-To: <20191002163946.GE44691@kib.kiev.ua> Message-ID: <20191003030837.C2787@besplex.bde.org> References: <0e27fb3e-0f60-68e1-dbba-f17c3d91c332@embedded-brains.de> <20191002140040.GA44691@kib.kiev.ua> <20191003013314.O2151@besplex.bde.org> <20191002163946.GE44691@kib.kiev.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=P6RKvmIu c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=kj9zAlcOel0A:10 a=s1G7sxBSAAAA:20 a=tfnm91WlDkBHvcOSr7QA:9 a=CjuIK1q_8ugA:10 X-Rspamd-Queue-Id: 46k2kl1X4sz3F5N X-Spamd-Bar: - Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of brde@optusnet.com.au designates 211.29.132.246 as permitted sender) smtp.mailfrom=brde@optusnet.com.au X-Spamd-Result: default: False [-1.30 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; RCVD_IN_DNSWL_LOW(-0.10)[246.132.29.211.list.dnswl.org : 127.0.5.1]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; FREEMAIL_FROM(0.00)[optusnet.com.au]; R_SPF_ALLOW(-0.20)[+ip4:211.29.132.0/23:c]; MIME_GOOD(-0.10)[text/plain]; MIME_TRACE(0.00)[0:+]; DMARC_NA(0.00)[optusnet.com.au]; IP_SCORE_FREEMAIL(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; RWL_MAILSPIKE_POSSIBLE(0.00)[246.132.29.211.rep.mailspike.net : 127.0.0.17]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; SUBJECT_ENDS_QUESTION(1.00)[]; FREEMAIL_TO(0.00)[gmail.com]; RCVD_NO_TLS_LAST(0.10)[]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; FREEMAIL_ENVFROM(0.00)[optusnet.com.au]; ASN(0.00)[asn:4804, ipnet:211.28.0.0/14, country:AU]; FREEMAIL_CC(0.00)[optusnet.com.au]; IP_SCORE(0.00)[ip: (-7.03), ipnet: 211.28.0.0/14(-3.24), asn: 4804(-2.39), country: AU(0.01)]; RCVD_COUNT_TWO(0.00)[2] X-Mailman-Approved-At: Sat, 12 Oct 2019 23:39:36 +0000 X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 Oct 2019 17:12:33 -0000 On Wed, 2 Oct 2019, Konstantin Belousov wrote: > On Thu, Oct 03, 2019 at 02:25:46AM +1000, Bruce Evans wrote: >> On Wed, 2 Oct 2019, Konstantin Belousov wrote: >> >>> On Tue, Oct 01, 2019 at 01:11:18PM +0200, Sebastian Huber wrote: >>>> Hello, >>>> >>>> since this commit >>>> >>>> https://github.com/freebsd/freebsd/commit/307f787e5a7f >>>> >>> It is not very useful to pass github hashes around. >>> >>> I think that the addition of the second tc_get_timecount() was done >>> earlier, in r95530, and there it has semi-useful comment >>> + /* Warm up new timecounter. */ >>> + (void)newtc->tc_get_timecount(newtc); >>> + (void)newtc->tc_get_timecount(newtc); >>> >>> The commit message is not helpful at all. >> >> The comment was correct when I added it in r48887. Then it was only >> attached to the first tc_get_timecounter() call which is necessary to >> start up or sync the hardware timecounter in some cases, e.g., for the >> i8254. After the warmup, r48887 called the preexisting function >> tc_switch_timecount() switch the software state. This function was >> unused (ifdefed out) but required only minor modifications. >> >> r95530 removes tc_switch_timecount() and replaces it by a second call >> to tc_get_timecounter() and an assignment of the new timecounter pointer >> to the active timecounter pointer, and removes the blank line that >> separates the warmup from the application. The second call and the >> assignment are all that is left of the function after moving its >> initialization. >> >> The second warmup looks like nonsense in both versions, and it is >> unclear how activation by plain assignment can work in the second >> version (the first version was in FreeBSD-3 or 4 and was locked by >> splclock()). The assignment is sloppy about memory ordering. The >> second warmup may have helped by accidentally forcing the assignment >> to be after the first warmup. (Some timecounters that need at least >> 1 warmup, e.g., the i854, use atomic ops that accidentally give >> sufficient ordering.) Later work on ordering may have reduced the >> sloppiness. >> >>> I do not see a timecounter which would need two get_timecount() calls >>> to start working properly now, but I can imagine that at time it was. >> >> I think it never helped much. For the TSC, the 2 calls are ordered only >> relatively each other on a single CPU. They are not ordered relative to >> memory. For the i8254, 1 call is enough. The ACPI timer does hardware >> accesses so it is in between. > So the conclusion is that the second call can be removed, am I right ? Yes. All tc_get_timecount() functions should be checked for doing sufficient initialization in one call (so that deltas for subsequent calls are correct). Bruce