Messages in this thread | | | Date | Sat, 17 Apr 2021 16:42:57 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH v8 clocksource 3/5] clocksource: Check per-CPU clock synchronization when marked unstable |
| |
On Sat, Apr 17, 2021 at 02:28:22PM +0200, Thomas Gleixner wrote: > On Tue, Apr 13 2021 at 21:36, Paul E. McKenney wrote: > > diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c > > index 1fc0962c89c0..97eeaf164296 100644 > > --- a/arch/x86/kernel/kvmclock.c > > +++ b/arch/x86/kernel/kvmclock.c > > @@ -169,7 +169,7 @@ struct clocksource kvm_clock = { > > .read = kvm_clock_get_cycles, > > .rating = 400, > > .mask = CLOCKSOURCE_MASK(64), > > - .flags = CLOCK_SOURCE_IS_CONTINUOUS, > > + .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_VERIFY_PERCPU, > > kvm_clock is not marked with CLOCK_SOURCE_MUST_VERIFY, so what's the > point of adding this here? It's not subject to be monitored by the > watchdog muck.
Good point, removed.
> > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c > > index f70dffc2771f..56289170753c 100644 > > --- a/arch/x86/kernel/tsc.c > > +++ b/arch/x86/kernel/tsc.c > > @@ -1151,7 +1151,8 @@ static struct clocksource clocksource_tsc = { > > .mask = CLOCKSOURCE_MASK(64), > > .flags = CLOCK_SOURCE_IS_CONTINUOUS | > > CLOCK_SOURCE_VALID_FOR_HRES | > > - CLOCK_SOURCE_MUST_VERIFY, > > + CLOCK_SOURCE_MUST_VERIFY | > > + CLOCK_SOURCE_VERIFY_PERCPU, > > While this one is part of the horror show.
It probably would be good to decorate other arch's per-CPU clocks with CLOCK_SOURCE_VERIFY_PERCPU, but no takers thus far.
Thanx, Paul
> > +static u64 csnow_mid; > > +static cpumask_t cpus_ahead; > > +static cpumask_t cpus_behind; > > + > > +static void clocksource_verify_one_cpu(void *csin) > > +{ > > + struct clocksource *cs = (struct clocksource *)csin; > > + > > + csnow_mid = cs->read(cs); > > +} > > + > > +static void clocksource_verify_percpu(struct clocksource *cs) > > +{ > > + int64_t cs_nsec, cs_nsec_max, cs_nsec_min; > > + u64 csnow_begin, csnow_end; > > + bool firsttime = 1; > > + int testcpu; > > + s64 delta; > > + int cpu; > > int testcpu, cpu; :) >
| |