Remove unreliable parts of rt/tst-cpuclock2

Message ID xna6jn1d0k.fsf@greed.delorie.com
State New
Headers show
Series
  • Remove unreliable parts of rt/tst-cpuclock2
Related show

Commit Message

Carlos O'Donell via Libc-alpha Oct. 5, 2021, 7:34 p.m.
This is a follow-up to the tst-cpuclock1.c change here:
9a29f1a2ae3d4bb253ee368e0d71db0ca9494120

This test, like tst-cpuclock1, may fail on heavily loaded VM
servers (and has occasionally failed on the 32bit trybot),
so tests that rely on "wall time" have been removed.

Comments

Carlos O'Donell via Libc-alpha Oct. 7, 2021, 8:29 p.m. | #1
On 05/10/2021 16:34, DJ Delorie via Libc-alpha wrote:
> 

> This is a follow-up to the tst-cpuclock1.c change here:

> 9a29f1a2ae3d4bb253ee368e0d71db0ca9494120

> 

> This test, like tst-cpuclock1, may fail on heavily loaded VM

> servers (and has occasionally failed on the 32bit trybot),

> so tests that rely on "wall time" have been removed.

> 

> diff --git a/rt/tst-cpuclock2.c b/rt/tst-cpuclock2.c


Thanks, these pieces are too flaky and generated a lot of false
positives.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>


> index eebc3609d0a..32a1b75c2f3 100644

> --- a/rt/tst-cpuclock2.c

> +++ b/rt/tst-cpuclock2.c

> @@ -62,22 +62,9 @@ chew_cpu (void *arg)

>    return NULL;

>  }

>  

> -static unsigned long long int

> -tsdiff (const struct timespec *before, const struct timespec *after)

> -{

> -  struct timespec diff = { .tv_sec = after->tv_sec - before->tv_sec,

> -			   .tv_nsec = after->tv_nsec - before->tv_nsec };

> -  while (diff.tv_nsec < 0)

> -    {

> -      --diff.tv_sec;

> -      diff.tv_nsec += 1000000000;

> -    }

> -  return diff.tv_sec * 1000000000ULL + diff.tv_nsec;

> -}

> -

> -static unsigned long long int

> +static void

>  test_nanosleep (clockid_t clock, const char *which,

> -		const struct timespec *before, int *bad)

> +		int *bad)

>  {

>    const struct timespec sleeptime = { .tv_nsec = 100000000 };

>    int e = clock_nanosleep (clock, 0, &sleeptime, NULL);

> @@ -85,13 +72,13 @@ test_nanosleep (clockid_t clock, const char *which,

>      {

>        printf ("clock_nanosleep not supported for %s CPU clock: %s\n",

>  	      which, strerror (e));

> -      return 0;

> +      return;

>      }

>    if (e != 0)

>      {

>        printf ("clock_nanosleep on %s CPU clock: %s\n", which, strerror (e));

>        *bad = 1;

> -      return 0;

> +      return;

>      }

>  

>    struct timespec after;

> @@ -100,16 +87,7 @@ test_nanosleep (clockid_t clock, const char *which,

>        printf ("clock_gettime on %s CPU clock %lx => %s\n",

>  	      which, (unsigned long int) clock, strerror (errno));

>        *bad = 1;

> -      return 0;

> -    }

> -

> -  unsigned long long int diff = tsdiff (before, &after);

> -  if (diff < sleeptime.tv_nsec || diff > sleeptime.tv_nsec * 2)

> -    {

> -      printf ("clock_nanosleep on %s slept %llu (outside reasonable range)\n",

> -	      which, diff);

> -      *bad = 1;

> -      return diff;

> +      return;

>      }

>  

>    struct timespec sleeptimeabs = sleeptime;

> @@ -126,7 +104,7 @@ test_nanosleep (clockid_t clock, const char *which,

>        printf ("absolute clock_nanosleep on %s CPU clock: %s\n",

>  	      which, strerror (e));

>        *bad = 1;

> -      return diff;

> +      return;

>      }

>  

>    struct timespec afterabs;

> @@ -135,28 +113,10 @@ test_nanosleep (clockid_t clock, const char *which,

>        printf ("clock_gettime on %s CPU clock %lx => %s\n",

>  	      which, (unsigned long int) clock, strerror (errno));

>        *bad = 1;

> -      return diff;

> -    }

> -

> -  unsigned long long int sleepdiff = tsdiff (&sleeptimeabs, &afterabs);

> -  if (sleepdiff > sleeptime.tv_nsec)

> -    {

> -      printf ("\

> -absolute clock_nanosleep on %s %llu past target (outside reasonable range)\n",

> -	      which, sleepdiff);

> -      *bad = 1;

> +      return;

>      }

>  

> -  unsigned long long int diffabs = tsdiff (&after, &afterabs);

> -  if (diffabs < sleeptime.tv_nsec || diffabs > sleeptime.tv_nsec * 2)

> -    {

> -      printf ("\

> -absolute clock_nanosleep on %s slept %llu (outside reasonable range)\n",

> -	      which, diffabs);

> -      *bad = 1;

> -    }

> -

> -  return diff + diffabs;

> +  return;

>  }

>  

>  

> @@ -290,37 +250,12 @@ do_test (void)

>    printf ("self thread after sleep => %ju.%.9ju\n",

>  	  (uintmax_t) me_after.tv_sec, (uintmax_t) me_after.tv_nsec);

>  

> -  unsigned long long int th_diff = tsdiff (&before, &after);

> -  unsigned long long int pdiff = tsdiff (&process_before, &process_after);

> -  unsigned long long int my_diff = tsdiff (&me_before, &me_after);

> -

> -  if (th_diff < 100000000 || th_diff > 600000000)

> -    {

> -      printf ("live thread before - after %llu outside reasonable range\n",

> -	      th_diff);

> -      result = 1;

> -    }

> -

> -  if (my_diff > 100000000)

> -    {

> -      printf ("self thread before - after %llu outside reasonable range\n",

> -	      my_diff);

> -      result = 1;

> -    }

> -

> -  if (pdiff < th_diff)

> -    {

> -      printf ("process before - after %llu outside reasonable range (%llu)\n",

> -	      pdiff, th_diff);

> -      result = 1;

> -    }

> -

> -  process_after.tv_nsec += test_nanosleep (th_clock, "live thread",

> -					   &after, &result);

> -  process_after.tv_nsec += test_nanosleep (process_clock, "process",

> -					   &process_after, &result);

> +  test_nanosleep (th_clock, "live thread",

> +		  &result);

> +  test_nanosleep (process_clock, "process",

> +		  &result);

>    test_nanosleep (CLOCK_PROCESS_CPUTIME_ID,

> -		  "PROCESS_CPUTIME_ID", &process_after, &result);

> +		  "PROCESS_CPUTIME_ID", &result);

>  

>    pthread_cancel (th);

>  

>
Carlos O'Donell via Libc-alpha Oct. 8, 2021, 2:40 a.m. | #2
Thanks!  Pushed.

Patch

diff --git a/rt/tst-cpuclock2.c b/rt/tst-cpuclock2.c
index eebc3609d0a..32a1b75c2f3 100644
--- a/rt/tst-cpuclock2.c
+++ b/rt/tst-cpuclock2.c
@@ -62,22 +62,9 @@  chew_cpu (void *arg)
   return NULL;
 }
 
-static unsigned long long int
-tsdiff (const struct timespec *before, const struct timespec *after)
-{
-  struct timespec diff = { .tv_sec = after->tv_sec - before->tv_sec,
-			   .tv_nsec = after->tv_nsec - before->tv_nsec };
-  while (diff.tv_nsec < 0)
-    {
-      --diff.tv_sec;
-      diff.tv_nsec += 1000000000;
-    }
-  return diff.tv_sec * 1000000000ULL + diff.tv_nsec;
-}
-
-static unsigned long long int
+static void
 test_nanosleep (clockid_t clock, const char *which,
-		const struct timespec *before, int *bad)
+		int *bad)
 {
   const struct timespec sleeptime = { .tv_nsec = 100000000 };
   int e = clock_nanosleep (clock, 0, &sleeptime, NULL);
@@ -85,13 +72,13 @@  test_nanosleep (clockid_t clock, const char *which,
     {
       printf ("clock_nanosleep not supported for %s CPU clock: %s\n",
 	      which, strerror (e));
-      return 0;
+      return;
     }
   if (e != 0)
     {
       printf ("clock_nanosleep on %s CPU clock: %s\n", which, strerror (e));
       *bad = 1;
-      return 0;
+      return;
     }
 
   struct timespec after;
@@ -100,16 +87,7 @@  test_nanosleep (clockid_t clock, const char *which,
       printf ("clock_gettime on %s CPU clock %lx => %s\n",
 	      which, (unsigned long int) clock, strerror (errno));
       *bad = 1;
-      return 0;
-    }
-
-  unsigned long long int diff = tsdiff (before, &after);
-  if (diff < sleeptime.tv_nsec || diff > sleeptime.tv_nsec * 2)
-    {
-      printf ("clock_nanosleep on %s slept %llu (outside reasonable range)\n",
-	      which, diff);
-      *bad = 1;
-      return diff;
+      return;
     }
 
   struct timespec sleeptimeabs = sleeptime;
@@ -126,7 +104,7 @@  test_nanosleep (clockid_t clock, const char *which,
       printf ("absolute clock_nanosleep on %s CPU clock: %s\n",
 	      which, strerror (e));
       *bad = 1;
-      return diff;
+      return;
     }
 
   struct timespec afterabs;
@@ -135,28 +113,10 @@  test_nanosleep (clockid_t clock, const char *which,
       printf ("clock_gettime on %s CPU clock %lx => %s\n",
 	      which, (unsigned long int) clock, strerror (errno));
       *bad = 1;
-      return diff;
-    }
-
-  unsigned long long int sleepdiff = tsdiff (&sleeptimeabs, &afterabs);
-  if (sleepdiff > sleeptime.tv_nsec)
-    {
-      printf ("\
-absolute clock_nanosleep on %s %llu past target (outside reasonable range)\n",
-	      which, sleepdiff);
-      *bad = 1;
+      return;
     }
 
-  unsigned long long int diffabs = tsdiff (&after, &afterabs);
-  if (diffabs < sleeptime.tv_nsec || diffabs > sleeptime.tv_nsec * 2)
-    {
-      printf ("\
-absolute clock_nanosleep on %s slept %llu (outside reasonable range)\n",
-	      which, diffabs);
-      *bad = 1;
-    }
-
-  return diff + diffabs;
+  return;
 }
 
 
@@ -290,37 +250,12 @@  do_test (void)
   printf ("self thread after sleep => %ju.%.9ju\n",
 	  (uintmax_t) me_after.tv_sec, (uintmax_t) me_after.tv_nsec);
 
-  unsigned long long int th_diff = tsdiff (&before, &after);
-  unsigned long long int pdiff = tsdiff (&process_before, &process_after);
-  unsigned long long int my_diff = tsdiff (&me_before, &me_after);
-
-  if (th_diff < 100000000 || th_diff > 600000000)
-    {
-      printf ("live thread before - after %llu outside reasonable range\n",
-	      th_diff);
-      result = 1;
-    }
-
-  if (my_diff > 100000000)
-    {
-      printf ("self thread before - after %llu outside reasonable range\n",
-	      my_diff);
-      result = 1;
-    }
-
-  if (pdiff < th_diff)
-    {
-      printf ("process before - after %llu outside reasonable range (%llu)\n",
-	      pdiff, th_diff);
-      result = 1;
-    }
-
-  process_after.tv_nsec += test_nanosleep (th_clock, "live thread",
-					   &after, &result);
-  process_after.tv_nsec += test_nanosleep (process_clock, "process",
-					   &process_after, &result);
+  test_nanosleep (th_clock, "live thread",
+		  &result);
+  test_nanosleep (process_clock, "process",
+		  &result);
   test_nanosleep (CLOCK_PROCESS_CPUTIME_ID,
-		  "PROCESS_CPUTIME_ID", &process_after, &result);
+		  "PROCESS_CPUTIME_ID", &result);
 
   pthread_cancel (th);