Re: [PATCH v14 clocksource 4/6] clocksource: Reduce clocksource-skew threshold for TSC

[Paul E. McKenney]

On Wed, May 12, 2021 at 09:18:15PM +0800, Feng Tang wrote:
> Hi Paul,
> 
> On Tue, May 11, 2021 at 08:51:56PM -0700, Paul E. McKenney wrote:
> > On Wed, May 12, 2021 at 10:18:49AM +0800, Feng Tang wrote:
> > > Hi Paul,
> > > 
> > > On Tue, May 11, 2021 at 04:34:53PM -0700, Paul E. McKenney wrote:
> 
> [SNIP]
> > > >  	 * Ensure clocksources that have large 'mult' values don't overflow
> > > >  	 * when adjusted.
> > > > diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
> > > > index a492e4da69ba..b3f608c2b936 100644
> > > > --- a/kernel/time/jiffies.c
> > > > +++ b/kernel/time/jiffies.c
> > > > @@ -49,13 +49,14 @@ static u64 jiffies_read(struct clocksource *cs)
> > > >   * for "tick-less" systems.
> > > >   */
> > > >  static struct clocksource clocksource_jiffies = {
> > > > -	.name		= "jiffies",
> > > > -	.rating		= 1, /* lowest valid rating*/
> > > > -	.read		= jiffies_read,
> > > > -	.mask		= CLOCKSOURCE_MASK(32),
> > > > -	.mult		= TICK_NSEC << JIFFIES_SHIFT, /* details above */
> > > > -	.shift		= JIFFIES_SHIFT,
> > > > -	.max_cycles	= 10,
> > > > +	.name			= "jiffies",
> > > > +	.rating			= 1, /* lowest valid rating*/
> > > > +	.uncertainty_margin	= TICK_NSEC,
> > > 
> > > 'jiffies' is known to be very bad as a watchdog ("worse bandaid" in
> > > Thomas' words :)), and TICK_NSEC just turns to 1ms for HZ=1000 case. 
> > > Maybe we should give it a bigger margin, like the 32ms margin for 
> > > 'tsc-early'?
> > > 
> > > Other than this, it looks good to me, thanks!
> > 
> > As in the fixup diff below?
> 
> Yep, thanks,
> 
> > Would you be willing to provide an ack or tested-by for the rest
> > of the series?  (I have your ack on 1/6.)
> 
> I haven't figured out a way to check 5/6 patch yet, but feel free
> to add my ack for the first 4 patches (1/6 ~ 4/6) 
> 
> 	Acked-by: Feng Tang <feng.tang@xxxxxxxxx>

Thank you, and I will apply this.

On the unlikely off-chance that this works in your environtment, here
is how I test it:

	tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 45s --configs TREE03 --kconfig "CONFIG_TEST_CLOCKSOURCE_WATCHDOG=y" --bootargs "clocksource.max_cswd_read_retries=1" --trust-make > /tmp/kvm.sh.out 2>&1

	tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 45s --configs TREE03 --kconfig "CONFIG_TEST_CLOCKSOURCE_WATCHDOG=y" --trust-make > /tmp/kvm.sh.out 2>&1

	tools/testing/selftests/rcutorture/bin/kvm.sh --allcpus --duration 45s --configs TREE03 --trust-make > /tmp/kvm.sh.out 2>&1

If there are no splats, the test passed.  The third is tests running the
kernel without the module loaded, so it is probably redundant with your
other testing.  The test completes in a few minutes, give or take your
kernel build time compared to mine.

These create VMs running rcutorture, and test the clocksource watchdog
as a side effect.

							Thanx, Paul