Re: [PATCH v8 clocksource 3/5] clocksource: Check per-CPU clock synchronization when marked unstable

[Paul E. McKenney]

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; :)
>