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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b878262a-4c98-91bd-eb88-456a869dc9e1@infradead.org>
Date:   Tue, 2 Feb 2021 17:31:55 -0800
From:   Randy Dunlap <rdunlap@...radead.org>
To:     paulmck@...nel.org
Cc:     linux-kernel@...r.kernel.org, kernel-team@...com,
        john.stultz@...aro.org, tglx@...utronix.de, sboyd@...nel.org,
        corbet@....net, Mark.Rutland@....com, maz@...nel.org,
        ak@...ux.intel.com, clm@...com
Subject: Re: [PATCH clocksource 4/5] clocksource: Provide a module parameter
 to fuzz per-CPU clock checking

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@...nel.org wrote:
>>> From: "Paul E. McKenney" <paulmck@...nel.org>
>>>
>>> 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@...aro.org>
>>> Cc: Thomas Gleixner <tglx@...utronix.de>
>>> Cc: Stephen Boyd <sboyd@...nel.org>
>>> Cc: Jonathan Corbet <corbet@....net>
>>> Cc: Mark Rutland <Mark.Rutland@....com>
>>> Cc: Marc Zyngier <maz@...nel.org>
>>> Cc: Andi Kleen <ak@...ux.intel.com>
>>> Reported-by: Chris Mason <clm@...com>
>>> Signed-off-by: Paul E. McKenney <paulmck@...nel.org>
>>> ---
>>>  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.

> 			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