2020-09-29 12:22:58

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [PATCH] RFC: arm64: arch_timer: Fix timer inconsistency test for A64

Hello Roman,

On Tue, Sep 29, 2020 at 02:13:47PM +0300, Roman Stratiienko wrote:
> Fixes linux_kselftest:timers_inconsistency-check_arm_64
>
> Test logs without the fix:
> '''
> binary returned non-zero. Exit code: 1, stderr: , stdout:
> Consistent CLOCK_REALTIME
> 1601335525:467086804
> 1601335525:467087554
> 1601335525:467088345
> 1601335525:467089095
> 1601335525:467089887
> 1601335525:467090637
> 1601335525:467091429
> 1601335525:467092179
> 1601335525:467092929
> 1601335525:467093720
> 1601335525:467094470
> 1601335525:467095262
> 1601335525:467096012
> 1601335525:467096804
> --------------------
> 1601335525:467097554
> 1601335525:467077012
> --------------------
> 1601335525:467099095
> 1601335525:467099845
> 1601335525:467100637
> 1601335525:467101387
> 1601335525:467102179
> 1601335525:467102929
> '''

Can you reproduce the issue with a fixed CPU frequency. I suspect the root
cause is around CPU frequency scaling code on A64, and timer jumps happen when
the kernel is changing CPU frequency.

I fixed a similar issue on H3 SoC just by changing the CPU frequency scaling
code, without having to touch the timer readout code.

https://megous.com/git/linux/commit/?h=ths-5.9&id=51ff1a6d80126f678efca42555f93efa611f50c4

regards,
o.

> Signed-off-by: Roman Stratiienko <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> ---
> drivers/clocksource/arm_arch_timer.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 6c3e841801461..d50aa43cb654b 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -346,16 +346,17 @@ static u64 notrace arm64_858921_read_cntvct_el0(void)
> * number of CPU cycles in 3 consecutive 24 MHz counter periods.
> */
> #define __sun50i_a64_read_reg(reg) ({ \
> - u64 _val; \
> + u64 _val1, _val2; \
> int _retries = 150; \
> \
> do { \
> - _val = read_sysreg(reg); \
> + _val1 = read_sysreg(reg); \
> + _val2 = read_sysreg(reg); \
> _retries--; \
> - } while (((_val + 1) & GENMASK(9, 0)) <= 1 && _retries); \
> + } while (((_val2 - _val1) > 0x10) && _retries); \
> \
> WARN_ON_ONCE(!_retries); \
> - _val; \
> + _val2; \
> })
>
> static u64 notrace sun50i_a64_read_cntpct_el0(void)
> --
> 2.25.1
>