Messages in this thread | | | Subject | Re: [PATCH 2/2] Revert "x86/hpet: Reduce HPET counter read contention" | From | Zhenzhong Duan <> | Date | Mon, 18 Mar 2019 16:44:47 +0800 |
| |
On 2019/3/15 22:17, Waiman Long wrote: > On 03/15/2019 05:25 AM, Peter Zijlstra wrote: >> On Thu, Mar 14, 2019 at 04:42:12PM +0800, Zhenzhong Duan wrote: >>> This reverts commit f99fd22e4d4bc84880a8a3117311bbf0e3a6a9dc. >>> >>> It's unnecessory after commit "acpi_pm: Fix bootup softlockup due to PMTMR >>> counter read contention", the simple HPET access code could be restored. >>> >>> On a general system with good TSC, TSC is the final default clocksource. >>> So the potential performce loss is only at bootup stage before TSC >>> replacing HPET, we didn't observe obvious delay of bootup. >> The timeline here is: >> >> - Len took out SKX from native_calibrate_tsc >> b51120309348 ("x86/tsc: Fix erroneous TSC rate on Skylake Xeon") >> >> This causes the TSC to run through the calibration code, which >> completes _after_ SMP bringup. >> >> - This then caused HPET to be used during SMP bringup, which resulted >> in Waiman doing the patch you now propose removing. >> >> Because large (multi-socket) SKX machines would barely boot. >> >> f99fd22e4d4b ("x86/hpet: Reduce HPET counter read contention") >> >> - Now, I figured that was all crazy to begin with, and introduced >> clocksource_tsc_early, such that we can run at the guestimate TSC >> frequency until we've completed calibration and then swap to the real >> TSC clocksource. >> >> aa83c45762a2 ("x86/tsc: Introduce early tsc clocksource") >> (and assorted fixes) >> >> This means that we now only use HPET for a very short time in early >> boot, _IFF_ TSC is stable. >> >> Now, given the amount of wreckage we still see with TSC, I'm very >> reluctant to revert this patch. Because the moment TSC goes out the >> window, we're back on HPET, and this patch does make a huge difference. >> >> Yes, its sad, gross and nasty... but the same is true for TSC still being >> a trainwreck. >> >> So NAK. > I concur. In the uncontended case, the overhead is mostly just the > additional cmpxchg instruction for acquiring the spinlock. Even then, it > isn't significant when compared with the time needed to actually read > from the HPET. Without that code, any fallback to HPET for whatever > reason will likely see degradation in performance especially on systems > with large number of CPUs.
Thank Peter and Waiman for reply.
I see, we still care the performance on a system with wreckage TSC.
So now we come back to the old question, do we care the softlockup
and the performance when pmtmr is chosed for whatever reason?
For which I had provide two different fixes:
https://lkml.org/lkml/2019/1/22/1172
and
https://lkml.org/lkml/2019/3/15/101
-- Thanks Zhenzhong
| |