Re: [PATCH v2 0/4] clocksource: Avoid incorrect hpet fallback

From: Waiman Long
Date: Wed Nov 17 2021 - 13:52:04 EST

On 11/17/21 11:54, Paul E. McKenney wrote:
On Tue, Nov 16, 2021 at 06:44:22PM -0500, Waiman Long wrote:
A few questions:

1. Once you have all the patches in place, is the increase in
WATCHDOG_MAX_SKEW from 50us to 100us necessary?

I think so. Using Feng's reproducer, I was able to cause a hpet-hpet delay of more than 90us on a 1-socket system. With a default 50us WATCHDOG_MAX_SKEW, the chance of a warning showing up will be much higher. Trying to minimize the chance that a warning may appear is my primary reason to increase WATCHDOG_MAX_SKEW.

2. The reason for having cs->uncertainty_margin set to
2*WATCHDOG_MAX_SKEW was to allow for worst-case skew from both
the previous and the current reading. Are you sure that
dropping back to WATCHDOG_MAX_SKEW avoids false positives?
I can remove the hunk of changing cs->uncertainty_margin. It is critical for this patch.

3. In patch 3/4, shouldn't clock_skew_skip be a field in the
clocksource structure rather than a global? If a system had
multiple clocks being checked, wouldn't having this as a field
make things more predictable? Or am I missing something subtle

Yes, you are right. I should have put it into clocksource structure. I will make the change in v3.

4. These are intended to replace this commit in -rcu, correct?

9d5739316f36 ("clocksource: Forgive repeated long-latency watchdog clocksource reads")

But not this commit, correct?

5444fb39fd49 ("torture: Test splatting for delay-ridden clocksources")
Yes, that is my intention.
And would you like me to queue these, or would you rather send them
separately? (Either way works for me, just please let me know.)

I don't have a preference either way. If you are willing to queue these, it will be great too.