Messages in this thread | | | Subject | Re: [PATCH 2/2] Revert "x86/hpet: Reduce HPET counter read contention" | From | Waiman Long <> | Date | Mon, 18 Mar 2019 11:35:17 -0400 |
| |
On 03/18/2019 04:44 AM, Zhenzhong Duan wrote: > > 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 > I think what Thomas was asking is to provide a REALISTIC use case where TSC is wrecked and HPET is somehow not used and we have to fall back to use PM timer. If such use case exists, I am sure Thomas will be happy to take it.
Cheers, Longman
| |