From owner-freebsd-hackers@freebsd.org Wed Oct 2 16:25:55 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 516721330D8 for ; Wed, 2 Oct 2019 16:25:55 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id 46k1hx5Mf1z4f44 for ; Wed, 2 Oct 2019 16:25:52 +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 mail105.syd.optusnet.com.au (Postfix) with ESMTPS id EB5753628E2; Thu, 3 Oct 2019 02:25:46 +1000 (AEST) Date: Thu, 3 Oct 2019 02:25:46 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Konstantin Belousov cc: Sebastian Huber , FreeBSD Subject: Re: Why is tc_get_timecount() called two times in tc_init()? In-Reply-To: <20191002140040.GA44691@kib.kiev.ua> Message-ID: <20191003013314.O2151@besplex.bde.org> References: <0e27fb3e-0f60-68e1-dbba-f17c3d91c332@embedded-brains.de> <20191002140040.GA44691@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=3O9r_qV0sLByUYXe2nsA:9 a=CjuIK1q_8ugA:10 X-Rspamd-Queue-Id: 46k1hx5Mf1z4f44 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.249 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.995,0]; RCVD_IN_DNSWL_LOW(-0.10)[249.132.29.211.list.dnswl.org : 127.0.5.1]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; 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]; SUBJECT_ENDS_QUESTION(1.00)[]; 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)[249.132.29.211.rep.mailspike.net : 127.0.0.17]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; IP_SCORE(0.00)[ip: (-5.13), ipnet: 211.28.0.0/14(-3.25), asn: 4804(-2.40), country: AU(0.01)]; 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]; MIME_TRACE(0.00)[0:+]; 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 16:25:55 -0000 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. > I added Bruce to Cc: to may be get more context and explanation. > >> tc_get_timecount() is called two times in tc_init(). >> >> /* >> * Initialize a new timecounter and possibly use it. >> */ >> void >> tc_init(struct timecounter *tc) >> { >> [...] >> if (tc->tc_quality == timecounter->tc_quality && >> tc->tc_frequency < timecounter->tc_frequency) >> return; >> (void)tc->tc_get_timecount(tc); >> (void)tc->tc_get_timecount(tc); >> timecounter = tc; >> } >> >> What is the reason for this procedure? Bruce