Re: [PATCH v2 3/3] x86/time: don't move TSC backwards in time_calibration_tsc_rendezvous()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.02.2021 13:43, Jan Beulich wrote:
> As per the comment ahead of it, the original purpose of the function was
> to deal with TSCs halted in deep C states. While this probably explains
> why only forward moves were ever expected, I don't see how this could
> have been reliable in case CPU0 was deep-sleeping for a sufficiently
> long time. My only guess here is a hidden assumption of CPU0 never being
> idle for long enough.

Furthermore that comment looks to be contradicting the actual use of
the function: It gets installed when !RELIABLE_TSC, while the comment
would suggest !NONSTOP_TSC. I suppose the comment is simply misleading,
because RELIABLE_TSC implies NONSTOP_TSC according to all the places
where either of the two feature bits gets played with. Plus in the
!NONSTOP_TSC case we write the TSC explicitly anyway when coming back
out of a (deep; see below) C-state.

As an implication from the above mwait_idle_cpu_init() then looks to
pointlessly clear "reliable" when "nonstop" is clear.

It further looks odd that mwait_idle() (unlike acpi_processor_idle())
calls cstate_restore_tsc() independent of what C-state was active.

> @@ -1719,9 +1737,12 @@ static void time_calibration_tsc_rendezv
>              while ( atomic_read(&r->semaphore) > total_cpus )
>                  cpu_relax();
>          }
> +
> +        /* Just in case a read above ended up reading zero. */
> +        tsc += !tsc;
>      }
>  
> -    time_calibration_rendezvous_tail(r, r->master_tsc_stamp);
> +    time_calibration_rendezvous_tail(r, tsc, r->master_tsc_stamp);

This, in particular, wouldn't be valid when !NONSTOP_TSC without
cstate_restore_tsc(). We then wouldn't have a way to know whether
the observed gap is because of the TSC having been halted for a
while (as the comment ahead of the function - imo wrongly, as per
above - suggests), or whether - like in Claudemir's case - the
individual TSCs were offset against one another.

Jan