Why is tc_get_timecount() called two times in tc_init()?

[Konstantin Belousov]

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 ?

> 
> > 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