Messages in this thread | | | Date | Wed, 17 Nov 2021 13:25:47 -0800 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH v2 0/4] clocksource: Avoid incorrect hpet fallback |
| |
On Wed, Nov 17, 2021 at 01:51:51PM -0500, Waiman Long wrote: > 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.
Should we downgrade the "had to retry read" complain to pr_info(), and make the only real warning be the case where a large number of consecutive read attempts fail? I believe that Heiner Kallweit was looking for something like this.
> > 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.
Assuming "not critical", good!
> > 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 > > here? > > Yes, you are right. I should have put it into clocksource structure. I will > make the change in v3.
Sounds good! Looking forward to 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.
Very good, thank you!
> > 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.
Happy to do so!
Thanx, Paul
| |