From owner-freebsd-current@freebsd.org Fri Feb 24 11:45:52 2017 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 58733CEA9B1 for ; Fri, 24 Feb 2017 11:45:52 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id E79B11D98; Fri, 24 Feb 2017 11:45:51 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id v1OBjfeY013181 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 24 Feb 2017 13:45:41 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua v1OBjfeY013181 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id v1OBjfgS013179; Fri, 24 Feb 2017 13:45:41 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 24 Feb 2017 13:45:41 +0200 From: Konstantin Belousov To: Jia-Shiun Li Cc: John Baldwin , freebsd-current , Konstantin Belousov Subject: Re: TSC as timecounter makes system lag Message-ID: <20170224114541.GU2092@kib.kiev.ua> References: <20170113120534.GC2349@kib.kiev.ua> <20170223100829.GR2092@kib.kiev.ua> <2204246.QKzIRnxiUQ@ralph.baldwin.cx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.2 (2016-11-26) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Feb 2017 11:45:52 -0000 On Fri, Feb 24, 2017 at 12:15:26PM +0800, Jia-Shiun Li wrote: > On Fri, Feb 24, 2017 at 12:55 AM, John Baldwin wrote: > > > On Thursday, February 23, 2017 11:04:58 PM Jia-Shiun Li wrote: > > > > > > > > > This does not work. > > > > > > I added a printf before the outer if clause, and it says > > > > > > init_TSC_tc:546: deepest 00000000 vendor 00008086 amd_pminfo 00000000 > > > > > > full boot dmesg attached. Looks init_TSC_tc() is called too early before > > > acpi_cpu_attach() initializing cpu_deepest_sleep. Maybe it should be put > > > after > > > driver initialization, since it depends on probed ACPI C states? > > > > We don't actually need cpu_deepest_sleep. We could just set C2STOP always. > > It doesn't hurt to have the flag set if the system only supports C1 except > > that you get the printf in a verbose boot. > > > > Try this slight variation of Konstantin's patch. If this works we can > > remove > > cpu_deepest_sleep entirely as a followup since it will no longer be used: > > > > Index: tsc.c > > =================================================================== > > --- tsc.c (revision 314113) > > +++ tsc.c (working copy) > > @@ -542,9 +542,11 @@ init_TSC_tc(void) > > * 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 (timecounter == &tsc_timecounter) > > + cpu_disable_c2_sleep++; > > if (bootverbose) > > printf("TSC timecounter disables C2 and C3.\n"); > > } > > > > 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.