Date: Thu, 3 Oct 2019 02:25:46 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Sebastian Huber <sebastian.huber@embedded-brains.de>, FreeBSD <freebsd-hackers@freebsd.org> Subject: Re: Why is tc_get_timecount() called two times in tc_init()? Message-ID: <20191003013314.O2151@besplex.bde.org> In-Reply-To: <20191002140040.GA44691@kib.kiev.ua> References: <0e27fb3e-0f60-68e1-dbba-f17c3d91c332@embedded-brains.de> <20191002140040.GA44691@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20191003013314.O2151>