Re: [LTP] [PATCH] clock_gettime04: set threshold based on the clock tick rate

[Cyril Hrubis <chrubis@suse.cz>]

On Fri, Mar 25, 2022 at 12:00:57PM +0800, Li Wang wrote:
> This is to get rid of the intermittent failures in clock_gettime04,
> which are likely caused by different clock tick rates on platforms.
> Here set the threshold no less than each clock tick in millisecond:
> 
> 	delta = 1000(ms)/ticks_number_per_sec + 5;
> 
> Error log:
>   clock_gettime04.c:163: TFAIL: CLOCK_REALTIME_COARSE(syscall with old kernel spec):
>         Difference between successive readings greater than 5 ms (1): 10
>   clock_gettime04.c:163: TFAIL: CLOCK_MONOTONIC_COARSE(vDSO with old kernel spec):
> 	Difference between successive readings greater than 5 ms (2): 10
> 
> From Waiman Long:
>   That failure happens for CLOCK_REALTIME_COARSE which is a faster but less
>   precise version of CLOCK_REALTIME. The time resolution is actually a clock
>   tick. Since arm64 has a HZ rate of 100. That means each tick is 10ms. So a
>   CLOCK_REALTIME_COARSE threshold of 5ms is probably not enough. I would say
>   in the case of CLOCK_REALTIME_COARSE, we have to increase the threshold based
>   on the clock tick rate of the system. This is more a test failure than is
>   an inherent problem in the kernel.
> 
> Fixes #898
> 
> Reported-by: Eirik Fuller <efuller@redhat.com>
> Signed-off-by: Li Wang <liwang@redhat.com>
> Cc: Waiman Long <llong@redhat.com>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  testcases/kernel/syscalls/clock_gettime/clock_gettime04.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/clock_gettime/clock_gettime04.c b/testcases/kernel/syscalls/clock_gettime/clock_gettime04.c
> index a8d2c5b38..cccbc9383 100644
> --- a/testcases/kernel/syscalls/clock_gettime/clock_gettime04.c
> +++ b/testcases/kernel/syscalls/clock_gettime/clock_gettime04.c
> @@ -35,7 +35,7 @@ clockid_t clks[] = {
>  };
>  
>  static gettime_t ptr_vdso_gettime, ptr_vdso_gettime64;
> -static long long delta = 5;
> +static long long delta;
>  
>  static inline int do_vdso_gettime(gettime_t vdso, clockid_t clk_id, void *ts)
>  {
> @@ -92,6 +92,7 @@ static struct time64_variants variants[] = {
>  
>  static void setup(void)
>  {
> +	delta = 1000/sysconf(_SC_CLK_TCK) + 5;

This does not look correct to me. The sysconf(_SC_CLK_TCK) returns 100
on systems where the test was working fine with 5 second delta. I think
that the difference is that _SC_CLK_TCK returns how fast are the jiffies
incremented which does not really matter for most of the modern hardware
that uses high resolution harware for timers.

I think that we should really use whatever is returned by the
clock_getres(CLOCK_REALTIME_COARSE, &res), as long as high resolution
timers are not available this call will return resolution in miliseconds
and with high resolution timers available the reported resolution will
be in nanosecond range, so it should probably be something as:

	clock_getres(CLOCK_REALTIME_COARSE, &res);

	delta = 5 + (res.tv_nsec / 1000000) * 5;

>  	if (tst_is_virt(VIRT_ANY)) {
>  		tst_res(TINFO, "Running in a virtual machine, multiply the delta by 10.");
>  		delta *= 10;
> -- 
> 2.31.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp