Re: [PATCH 2/2] Revert "x86/hpet: Reduce HPET counter read contention"

[Zhenzhong Duan]

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