Re: TSC as timecounter makes system lag

On Thu, Feb 23, 2017 at 01:11:29AM +0800, Jia-Shiun Li wrote:
> I got the impression that TSC was not preferred timecounter
> if it is not C-state invariant. But this apparenly is not the case now.
> Dig a bit and found r277900 chose to prefer TSC over saving power
> by disabling C2 state when TSC is selected as timecounter.
> 
> But with EARLY_AP_STARTUP, and TSC as timecounter,
> CPU still enters C2 state.
> (Observed by sysctl dev.cpu.0.cx_usage_counters)
> With nooptions EARLY_AP_STARTUP, CPU correctly stays 100%
> in C1 and no lower.
> 
> I added a printf in tc_windup() to check. With EARLY_AP_STARTUP,
> cpu_disable_c2_sleep is never increased, so it can not prevent CPU
> from entering C2.
> 
> Guess there's some kind of race or init order issue, but it is
> beyond my understanding for now.

This is a useful analysis.

Yes, I think that there is an init ordering issue. Note that
cpu_disable_c2_sleep is only changed in tc_windup() when timecounter
is changed. If existing and already engadged timecounter suddenly gets
TC_FLAG_C2STOP set, tc_windup() ignores the flag.  And with the early
AP startup, tsc seems to be set as timecounter too early.

Just moving order of init_TSC_tc() would not help, since tsc checks smp
consistency, which requires started APs.  Try this for now, but might
be John has better idea how to handle the issue.  You might need to add
some extern declarations for the patch to compile.

diff --git a/sys/x86/x86/tsc.c b/sys/x86/x86/tsc.c
index 3f36fbd9f8a..f8e33069c70 100644
--- a/sys/x86/x86/tsc.c
+++ b/sys/x86/x86/tsc.c
_at__at_ -545,6 +545,8 _at__at_ init_TSC_tc(void)
 	if (cpu_deepest_sleep >= 2 && cpu_vendor_id == CPU_VENDOR_INTEL &&
 	    (amd_pminfo & AMDPM_TSC_INVARIANT) == 0) {
 		tsc_timecounter.tc_flags |= TC_FLAGS_C2STOP;
+		if (timecounter == &tsc_timecounter)
+			cpu_disable_c2_sleep++;
 		if (bootverbose)
 			printf("TSC timecounter disables C2 and C3.\n");
 	}