Date: Thu, 3 Oct 2019 03:12:26 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Bruce Evans <brde@optusnet.com.au>, 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: <20191003030837.C2787@besplex.bde.org> In-Reply-To: <20191002163946.GE44691@kib.kiev.ua> References: <0e27fb3e-0f60-68e1-dbba-f17c3d91c332@embedded-brains.de> <20191002140040.GA44691@kib.kiev.ua> <20191003013314.O2151@besplex.bde.org> <20191002163946.GE44691@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20191003030837.C2787>