Date: Fri, 24 Feb 2017 22:50:19 +0800 From: Jia-Shiun Li <jiashiun@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: John Baldwin <jhb@freebsd.org>, freebsd-current <freebsd-current@freebsd.org>, Konstantin Belousov <kib@freebsd.org> Subject: Re: TSC as timecounter makes system lag Message-ID: <CAHNYxxMLX8kfJAuP-ua65d3UN1DH-BOU8i1vqRUdzAOVvgaH_w@mail.gmail.com> In-Reply-To: <CAHNYxxNiCP08dQ6bTJzHuAUmqwQUQJn%2B%2BCKMsCLiHQy2PxjP5A@mail.gmail.com> References: <20170113120534.GC2349@kib.kiev.ua> <20170223100829.GR2092@kib.kiev.ua> <CAHNYxxPDQtu5oJw2FEibYPaxZb==8CiSYyX6i2CXoi5DDB7PEw@mail.gmail.com> <2204246.QKzIRnxiUQ@ralph.baldwin.cx> <CAHNYxxNB2QT4_h6RtMz9-sAc5br_VBWj6-NafSXuf88W56BmBQ@mail.gmail.com> <20170224114541.GU2092@kib.kiev.ua> <CAHNYxxNiCP08dQ6bTJzHuAUmqwQUQJn%2B%2BCKMsCLiHQy2PxjP5A@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Fri, Feb 24, 2017 at 9:32 PM, Jia-Shiun Li <jiashiun@gmail.com> wrote:
> On Fri, Feb 24, 2017 at 7:45 PM, Konstantin Belousov <kostikbel@gmail.com>
> wrote:
>
>> On Fri, Feb 24, 2017 at 12:15:26PM +0800, Jia-Shiun Li wrote:
>> > Tested working on E7400 against r313909. And changing timecounter
>> from/to
>> > TSC
>> > correctly enables/disables C2.
>> >
>> > The latter part cpu_disable_c2_sleep++ is not needed. When
>> > init_TSC_tc() got called timecounter is not yet tsc_timecounter.
>> > inittimecounter() later will do the work calling tc_windup().
>> >
>>
>> You mean, just this
>> - if (cpu_deepest_sleep >= 2 && cpu_vendor_id == CPU_VENDOR_INTEL &&
>> + if (cpu_vendor_id == CPU_VENDOR_INTEL &&
>> is enough to fix the issue ? If yes, we can remove the cpu_deepest_sleep
>> variable. This is John' observation, I think he would prefer to prepare
>> the patch.
>>
>
> Correct. That's enough.
>
>
Since that's simple enough... patch attached.
Tested against r313909 too.
-Jia-Shiun.
[-- Attachment #2 --]
Index: sys/dev/acpica/acpi_cpu.c
===================================================================
--- sys/dev/acpica/acpi_cpu.c (revision 313909)
+++ sys/dev/acpica/acpi_cpu.c (working copy)
@@ -703,7 +703,6 @@
sc->cpu_non_c2 = sc->cpu_cx_count;
sc->cpu_non_c3 = sc->cpu_cx_count;
sc->cpu_cx_count++;
- cpu_deepest_sleep = 1;
/*
* The spec says P_BLK must be 6 bytes long. However, some systems
@@ -729,7 +728,6 @@
cx_ptr++;
sc->cpu_non_c3 = sc->cpu_cx_count;
sc->cpu_cx_count++;
- cpu_deepest_sleep = 2;
}
}
if (sc->cpu_p_blk_len < 6)
@@ -746,7 +744,6 @@
cx_ptr->trans_lat = AcpiGbl_FADT.C3Latency;
cx_ptr++;
sc->cpu_cx_count++;
- cpu_deepest_sleep = 3;
}
}
}
@@ -831,7 +828,6 @@
cx_ptr->type = ACPI_STATE_C0;
cx_ptr++;
sc->cpu_cx_count++;
- cpu_deepest_sleep = 1;
/* Set up all valid states. */
for (i = 0; i < count; i++) {
@@ -884,8 +880,6 @@
continue;
case ACPI_STATE_C2:
sc->cpu_non_c3 = sc->cpu_cx_count;
- if (cpu_deepest_sleep < 2)
- cpu_deepest_sleep = 2;
break;
case ACPI_STATE_C3:
default:
@@ -894,8 +888,7 @@
"acpi_cpu%d: C3[%d] not available.\n",
device_get_unit(sc->cpu_dev), i));
continue;
- } else
- cpu_deepest_sleep = 3;
+ }
break;
}
Index: sys/kern/kern_clocksource.c
===================================================================
--- sys/kern/kern_clocksource.c (revision 313909)
+++ sys/kern/kern_clocksource.c (working copy)
@@ -54,7 +54,6 @@
#include <machine/cpu.h>
#include <machine/smp.h>
-int cpu_deepest_sleep = 0; /* Deepest Cx state available. */
int cpu_disable_c2_sleep = 0; /* Timer dies in C2. */
int cpu_disable_c3_sleep = 0; /* Timer dies in C3. */
Index: sys/sys/systm.h
===================================================================
--- sys/sys/systm.h (revision 313909)
+++ sys/sys/systm.h (working copy)
@@ -321,7 +321,6 @@
void cpu_activeclock(void);
void cpu_new_callout(int cpu, sbintime_t bt, sbintime_t bt_opt);
void cpu_et_frequency(struct eventtimer *et, uint64_t newfreq);
-extern int cpu_deepest_sleep;
extern int cpu_disable_c2_sleep;
extern int cpu_disable_c3_sleep;
Index: sys/x86/x86/tsc.c
===================================================================
--- sys/x86/x86/tsc.c (revision 313909)
+++ sys/x86/x86/tsc.c (working copy)
@@ -542,7 +542,7 @@
* result incorrect runtimes for kernel idle threads (but not
* for any non-idle threads).
*/
- if (cpu_deepest_sleep >= 2 && cpu_vendor_id == CPU_VENDOR_INTEL &&
+ if (cpu_vendor_id == CPU_VENDOR_INTEL &&
(amd_pminfo & AMDPM_TSC_INVARIANT) == 0) {
tsc_timecounter.tc_flags |= TC_FLAGS_C2STOP;
if (bootverbose)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHNYxxMLX8kfJAuP-ua65d3UN1DH-BOU8i1vqRUdzAOVvgaH_w>