Re: [PATCH clocksource 4/5] clocksource: Provide a module parameter to fuzz per-CPU clock checking

[Paul E. McKenney]

On Tue, Feb 02, 2021 at 05:31:55PM -0800, Randy Dunlap wrote:
> On 2/2/21 4:50 PM, Paul E. McKenney wrote:
> > On Tue, Feb 02, 2021 at 11:51:02AM -0800, Randy Dunlap wrote:
> >> On 2/2/21 9:06 AM, paulmck@xxxxxxxxxx wrote:
> >>> From: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
> >>>
> >>> Code that checks for clock desynchronization must itself be tested, so
> >>> this commit creates a new clocksource.inject_delay_shift_percpu= kernel
> >>> boot parameter that adds or subtracts a large value from the check read,
> >>> using the specified bit of the CPU ID to determine whether to add or
> >>> to subtract.
> >>>
> >>> Cc: John Stultz <john.stultz@xxxxxxxxxx>
> >>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> >>> Cc: Stephen Boyd <sboyd@xxxxxxxxxx>
> >>> Cc: Jonathan Corbet <corbet@xxxxxxx>
> >>> Cc: Mark Rutland <Mark.Rutland@xxxxxxx>
> >>> Cc: Marc Zyngier <maz@xxxxxxxxxx>
> >>> Cc: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> >>> Reported-by: Chris Mason <clm@xxxxxx>
> >>> Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> >>> ---
> >>>  Documentation/admin-guide/kernel-parameters.txt |  9 +++++++++
> >>>  kernel/time/clocksource.c                       | 10 +++++++++-
> >>>  2 files changed, 18 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> >>> index 9965266..f561e94 100644
> >>> --- a/Documentation/admin-guide/kernel-parameters.txt
> >>> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >>> @@ -593,6 +593,15 @@
> >>>  			times the value specified for inject_delay_freq
> >>>  			of consecutive non-delays.
> >>>  
> >>> +	clocksource.inject_delay_shift_percpu= [KNL]
> >>> +			Shift count to obtain bit from CPU number to
> >>> +			determine whether to shift the time of the per-CPU
> >>> +			clock under test ahead or behind.  For example,
> >>
> >> It's a good think that you give an example -- it helps a little bit.
> >> That sentence above needs to be rewritten...
> > 
> > That is a bit obscure, now that you mention it.
> > 
> >>> +			setting this to the value four will result in
> >>> +			alternating groups of 16 CPUs shifting ahead and
> >>> +			the rest of the CPUs shifting behind.  The default
> >>> +			value of -1 disable this type of error injection.
> >>
> >> 			            disables
> > 
> > Good eyes!
> > 
> > So how about like this?
> 
> Much better, thanks.
> 
> > 	clocksource.inject_delay_shift_percpu= [KNL]
> > 			Clocksource delay injection partitions the CPUs
> > 			into two sets, one whose clocks are moved ahead
> > 			and the other whose clocks are moved behind.
> > 			This kernel parameter selects the CPU-number
> > 			bit that determines which of these two sets the
> > 			corresponding CPU is placed into.  For example,
> > 			setting this parameter to the value four will
> 
> I know that in "writing," "four" should be written out as you have it,
> but IMO using "4" here would be much better.  FWIW.

As long as it is "four" and not "fore"!  Updated as requested.  ;-)

						Thanx, Paul

> > 			result in the first set containing alternating
> > 			groups of 16 CPUs whose clocks are moved ahead,
> > 			while the second set will contain the rest of
> > 			the CPUs, whose clocks are moved behind.
> > 
> > 			The default value of -1 disables this type of
> > 			error injection.
> > 
> > 							Thanx, Paul
> > 
> >>> +
> >>>  	clocksource.max_read_retries= [KNL]
> >>>  			Number of clocksource_watchdog() retries due to
> >>>  			external delays before the clock will be marked
> 
> 
> thanks!
> -- 
> ~Randy
>