Why is tc_get_timecount() called two times in tc_init()?
Bruce Evans
brde at optusnet.com.au
Wed Oct 2 17:12:33 UTC 2019
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
More information about the freebsd-hackers
mailing list