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

[Li Wang]

Cyril Hrubis <chrubis@suse.cz> wrote:


> > --- 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.
>

You're right, I checked some documents and confirmed this.
There I used the system tick rate (in the worst situations) that
seemed a bit loose to other clockids.


> 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;
>

Sounds reasonable.

But I don't understand why you multiply 5 for the resolution
(in milliseconds) here. Or, a wiser choice is to get the real
resolution for each clockid? i.e.

--- a/testcases/kernel/syscalls/clock_gettime/clock_gettime04.c
+++ b/testcases/kernel/syscalls/clock_gettime/clock_gettime04.c
@@ -92,23 +92,27 @@ static struct time64_variants variants[] = {

 static void setup(void)
 {
-       delta = 1000/sysconf(_SC_CLK_TCK) + 5;
-       if (tst_is_virt(VIRT_ANY)) {
-               tst_res(TINFO, "Running in a virtual machine, multiply the
delta by 10.");
-               delta *= 10;
-       }
-
        find_clock_gettime_vdso(&ptr_vdso_gettime, &ptr_vdso_gettime64);
 }

 static void run(unsigned int i)
 {
        struct tst_ts ts;
+       struct timespec res;
        long long start, end = 0, diff, slack;
        struct time64_variants *tv;
        int count = 10000, ret;
        unsigned int j;

+       clock_getres(clks[i], &res);
+       tst_res(TINFO, "%s: resolution is %ldns", tst_clock_name(clks[i]),
res.tv_nsec);
+
+       delta = 5 + res.tv_nsec/1000000;
+       if (tst_is_virt(VIRT_ANY)) {
+               delta *= 10;
+               tst_res(TINFO, "Multiply the delta by 10 in virtual
machine: %lld", delta);
+       }
+
        do {
                for (j = 0; j < ARRAY_SIZE(variants); j++) {
                        /* Refresh time in start */

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20220326/da22b2cf/attachment.htm>