[6/7] nptl/tst-rwlock: Use clock_gettime/timespec rather than gettimeofday/timeval
Commit Message
In preparation for adding pthread_rwlock_clockrdlock and
pthread_rwlock_clockwrlock, convert various tests to only use clock_gettime
and struct timespec.
* support/timespec.h: Create header to provide timespec helper functions
from sysdeps/pthread/posix-timer.h for tests to use.
* nptl/tst-rwlock6.c: Fix small bug in timeout-checking code that could
erroneously pass if the function incorrectly took more than a second.
* nptl/tst-rwlock6.c: Use clock_gettime(2) rather than gettimeofday(2) and
then converting to timespec in preparation for testing
pthread_rwlock_clockrdclock and pthread_rwlock_clockwrlock.
* nptl/tst-rwlock9.c, nptl/tst-rwlock7.c: Likewise.
---
nptl/tst-rwlock6.c | 47 ++++++++++++++++++-----------------------------
nptl/tst-rwlock7.c | 43 +++++++++++++++++--------------------------
nptl/tst-rwlock9.c | 8 ++------
support/timespec.h | 27 +++++++++++++++++++++++++++-
4 files changed, 64 insertions(+), 61 deletions(-)
create mode 100644 support/timespec.h
Comments
On 27/02/2019 15:23, Mike Crowe wrote:
> In preparation for adding pthread_rwlock_clockrdlock and
> pthread_rwlock_clockwrlock, convert various tests to only use clock_gettime
> and struct timespec.
>
> * support/timespec.h: Create header to provide timespec helper functions
> from sysdeps/pthread/posix-timer.h for tests to use.
>
> * nptl/tst-rwlock6.c: Fix small bug in timeout-checking code that could
> erroneously pass if the function incorrectly took more than a second.
>
> * nptl/tst-rwlock6.c: Use clock_gettime(2) rather than gettimeofday(2) and
> then converting to timespec in preparation for testing
> pthread_rwlock_clockrdclock and pthread_rwlock_clockwrlock.
>
> * nptl/tst-rwlock9.c, nptl/tst-rwlock7.c: Likewise.
I am seeing this issue sporadically on i686-linux-gnu with 6/7 patches
applied:
$ ./testrun.sh nptl/tst-rwlock7 --direct
0: got timedrdlock
child: timedwrlock failed with ETIMEDOUT
child: timedwrlock failed with EINVAL
1: got timedrdlock
child: timedwrlock failed with ETIMEDOUT
child: timedwrlock failed with EINVAL
2: got timedrdlock
child: timedwrlock failed with ETIMEDOUT
2nd timedwrlock did not return EINVAL
failure in round 2
> ---
> nptl/tst-rwlock6.c | 47 ++++++++++++++++++-----------------------------
> nptl/tst-rwlock7.c | 43 +++++++++++++++++--------------------------
> nptl/tst-rwlock9.c | 8 ++------
> support/timespec.h | 27 +++++++++++++++++++++++++++-
> 4 files changed, 64 insertions(+), 61 deletions(-)
> create mode 100644 support/timespec.h
>
> diff --git a/nptl/tst-rwlock6.c b/nptl/tst-rwlock6.c
> index 8d6c3dc..67119fa 100644
> --- a/nptl/tst-rwlock6.c
> +++ b/nptl/tst-rwlock6.c
> @@ -22,6 +22,7 @@
> #include <stdio.h>
> #include <string.h>
> #include <sys/time.h>
> +#include <support/timespec.h>
>
>
> static int kind[] =
> @@ -38,21 +39,15 @@ tf (void *arg)
> pthread_rwlock_t *r = arg;
>
> /* Timeout: 0.3 secs. */
> - struct timeval tv;
> - (void) gettimeofday (&tv, NULL);
> + struct timespec ts_start;
> + (void) clock_gettime(CLOCK_REALTIME, &ts_start);
Space after symbol. Also I think there is no need for (void) cast here.
>
> - struct timespec ts;
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> - ts.tv_nsec += 300000000;
> - if (ts.tv_nsec >= 1000000000)
> - {
> - ts.tv_nsec -= 1000000000;
> - ++ts.tv_sec;
> - }
> + struct timespec ts_timeout = {0, 300000000};
> + timespec_add(&ts_timeout, &ts_start, &ts_timeout);
Ditto.
>
> puts ("child calling timedrdlock");
>
> - int err = pthread_rwlock_timedrdlock (r, &ts);
> + int err = pthread_rwlock_timedrdlock (r, &ts_timeout);
> if (err == 0)
> {
> puts ("rwlock_timedrdlock returned");
> @@ -68,24 +63,24 @@ tf (void *arg)
>
> puts ("1st child timedrdlock done");
>
> - struct timeval tv2;
> - (void) gettimeofday (&tv2, NULL);
> + struct timespec ts_end;
> + (void) clock_gettime (CLOCK_REALTIME, &ts_end);
>
> - timersub (&tv2, &tv, &tv);
> + struct timespec ts_duration;
> + timespec_sub(&ts_duration, &ts_end, &ts_start);
Ditto.
>
> - if (tv.tv_usec < 200000)
> + if (ts_duration.tv_sec !=0 || ts_duration.tv_nsec < 200000000)
After before !=.
> {
> puts ("timeout too short");
> pthread_exit ((void *) 1l);
> }
>
> - (void) gettimeofday (&tv, NULL);
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> - ts.tv_sec += 10;
> + (void) clock_gettime (CLOCK_REALTIME, &ts_timeout);
> + ts_timeout.tv_sec += 10;
> /* Note that the following operation makes ts invalid. */
> - ts.tv_nsec += 1000000000;
> + ts_timeout.tv_nsec += 1000000000;
>
> - err = pthread_rwlock_timedrdlock (r, &ts);
> + err = pthread_rwlock_timedrdlock (r, &ts_timeout);
> if (err == 0)
> {
> puts ("2nd timedrdlock succeeded");
> @@ -136,12 +131,8 @@ do_test (void)
> exit (1);
> }
>
> - struct timeval tv;
> - (void) gettimeofday (&tv, NULL);
> -
> struct timespec ts;
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> -
> + (void) clock_gettime (CLOCK_REALTIME, &ts);
> ++ts.tv_sec;
>
> /* Get a write lock. */
> @@ -154,8 +145,7 @@ do_test (void)
>
> puts ("1st timedwrlock done");
>
> - (void) gettimeofday (&tv, NULL);
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> + (void) clock_gettime (CLOCK_REALTIME, &ts);
> ++ts.tv_sec;
> e = pthread_rwlock_timedrdlock (&r, &ts);
> if (e == 0)
> @@ -171,8 +161,7 @@ do_test (void)
>
> puts ("1st timedrdlock done");
>
> - (void) gettimeofday (&tv, NULL);
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> + (void) clock_gettime (CLOCK_REALTIME, &ts);
> ++ts.tv_sec;
> e = pthread_rwlock_timedwrlock (&r, &ts);
> if (e == 0)
Ok.
> diff --git a/nptl/tst-rwlock7.c b/nptl/tst-rwlock7.c
> index 4d6f561..812506c 100644
> --- a/nptl/tst-rwlock7.c
> +++ b/nptl/tst-rwlock7.c
> @@ -22,6 +22,7 @@
> #include <stdio.h>
> #include <string.h>
> #include <sys/time.h>
> +#include <support/timespec.h>
>
>
> static int kind[] =
> @@ -38,19 +39,12 @@ tf (void *arg)
> pthread_rwlock_t *r = arg;
>
> /* Timeout: 0.3 secs. */
> - struct timeval tv;
> - (void) gettimeofday (&tv, NULL);
> + struct timespec ts_start;
> + (void) clock_gettime (CLOCK_REALTIME, &ts_start);
> + struct timespec ts_timeout = {0, 300000000};
> + timespec_add(&ts_timeout, &ts_start, &ts_timeout);
Ditto.
>
> - struct timespec ts;
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> - ts.tv_nsec += 300000000;
> - if (ts.tv_nsec >= 1000000000)
> - {
> - ts.tv_nsec -= 1000000000;
> - ++ts.tv_sec;
> - }
> -
> - int err = pthread_rwlock_timedwrlock (r, &ts);
> + int err = pthread_rwlock_timedwrlock (r, &ts_timeout);
> if (err == 0)
> {
> puts ("rwlock_timedwrlock returned");
> @@ -65,24 +59,24 @@ tf (void *arg)
> }
> puts ("child: timedwrlock failed with ETIMEDOUT");
>
> - struct timeval tv2;
> - (void) gettimeofday (&tv2, NULL);
> -
> - timersub (&tv2, &tv, &tv);
> + struct timespec ts_end;
> + (void) clock_gettime (CLOCK_REALTIME, &ts_end);
> + struct timespec ts_diff;
> + timespec_sub (&ts_diff, &ts_end, &ts_start);
>
> - if (tv.tv_usec < 200000)
> + if (ts_diff.tv_sec != 0 || ts_diff.tv_nsec < 200000000)
> {
> puts ("timeout too short");
> pthread_exit ((void *) 1l);
> }
>
> - (void) gettimeofday (&tv, NULL);
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> - ts.tv_sec += 10;
> + struct timespec ts_invalid;
> + (void) clock_gettime (CLOCK_REALTIME, &ts_invalid);
> + ts_invalid.tv_sec += 10;
> /* Note that the following operation makes ts invalid. */
> - ts.tv_nsec += 1000000000;
> + ts_invalid.tv_nsec += 1000000000000;
>
> - err = pthread_rwlock_timedwrlock (r, &ts);
> + err = pthread_rwlock_timedwrlock (r, &ts_invalid);
> if (err == 0)
> {
> puts ("2nd timedwrlock succeeded");
> @@ -132,11 +126,8 @@ do_test (void)
> exit (1);
> }
>
> - struct timeval tv;
> - (void) gettimeofday (&tv, NULL);
> -
> struct timespec ts;
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> + (void) clock_gettime (CLOCK_REALTIME, &ts);
>
> ++ts.tv_sec;
>
Ok.
> diff --git a/nptl/tst-rwlock9.c b/nptl/tst-rwlock9.c
> index 34f2d04..ff15f90 100644
> --- a/nptl/tst-rwlock9.c
> +++ b/nptl/tst-rwlock9.c
> @@ -56,9 +56,7 @@ writer_thread (void *nr)
> int e;
> do
> {
> - struct timeval tv;
> - (void) gettimeofday (&tv, NULL);
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> + (void) clock_gettime (CLOCK_REALTIME, &ts);
>
> ts.tv_nsec += 2 * TIMEOUT;
> if (ts.tv_nsec >= 1000000000)
> @@ -110,9 +108,7 @@ reader_thread (void *nr)
> int e;
> do
> {
> - struct timeval tv;
> - (void) gettimeofday (&tv, NULL);
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> + (void) clock_gettime (CLOCK_REALTIME, &ts);
>
> ts.tv_nsec += TIMEOUT;
> if (ts.tv_nsec >= 1000000000)
Ok.
> diff --git a/support/timespec.h b/support/timespec.h
> new file mode 100644
> index 0000000..e5c89df
> --- /dev/null
> +++ b/support/timespec.h
> @@ -0,0 +1,27 @@
> +static inline void
> +timespec_sub (struct timespec *diff, const struct timespec *left,
> + const struct timespec *right)
> +{
> + diff->tv_sec = left->tv_sec - right->tv_sec;
> + diff->tv_nsec = left->tv_nsec - right->tv_nsec;
> +
> + if (diff->tv_nsec < 0)
> + {
> + --diff->tv_sec;
> + diff->tv_nsec += 1000000000;
> + }
> +}
> +
> +static inline void
> +timespec_add (struct timespec *sum, const struct timespec *left,
> + const struct timespec *right)
> +{
> + sum->tv_sec = left->tv_sec + right->tv_sec;
> + sum->tv_nsec = left->tv_nsec + right->tv_nsec;
> +
> + if (sum->tv_nsec >= 1000000000)
> + {
> + ++sum->tv_sec;
> + sum->tv_nsec -= 1000000000;
> + }
> +}
>
Ok.
Hi,
Le mercredi 27 février 2019 à 18:23 +0000, Mike Crowe a écrit :
> diff --git a/nptl/tst-rwlock9.c b/nptl/tst-rwlock9.c
> index 34f2d04..ff15f90 100644
> --- a/nptl/tst-rwlock9.c
> +++ b/nptl/tst-rwlock9.c
> @@ -56,9 +56,7 @@ writer_thread (void *nr)
> int e;
> do
> {
> - struct timeval tv;
> - (void) gettimeofday (&tv, NULL);
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> + (void) clock_gettime (CLOCK_REALTIME, &ts);
>
> ts.tv_nsec += 2 * TIMEOUT;
> if (ts.tv_nsec >= 1000000000)
Should timespec_add() be used here too ?
> @@ -110,9 +108,7 @@ reader_thread (void *nr)
> int e;
> do
> {
> - struct timeval tv;
> - (void) gettimeofday (&tv, NULL);
> - TIMEVAL_TO_TIMESPEC (&tv, &ts);
> + (void) clock_gettime (CLOCK_REALTIME, &ts);
>
> ts.tv_nsec += TIMEOUT;
> if (ts.tv_nsec >= 1000000000)
Same here
Regards.
On Tuesday 05 March 2019 at 15:02:21 -0300, Adhemerval Zanella wrote:
> On 27/02/2019 15:23, Mike Crowe wrote:
> > In preparation for adding pthread_rwlock_clockrdlock and
> > pthread_rwlock_clockwrlock, convert various tests to only use clock_gettime
> > and struct timespec.
> >
> > * support/timespec.h: Create header to provide timespec helper functions
> > from sysdeps/pthread/posix-timer.h for tests to use.
> >
> > * nptl/tst-rwlock6.c: Fix small bug in timeout-checking code that could
> > erroneously pass if the function incorrectly took more than a second.
> >
> > * nptl/tst-rwlock6.c: Use clock_gettime(2) rather than gettimeofday(2) and
> > then converting to timespec in preparation for testing
> > pthread_rwlock_clockrdclock and pthread_rwlock_clockwrlock.
> >
> > * nptl/tst-rwlock9.c, nptl/tst-rwlock7.c: Likewise.
>
> I am seeing this issue sporadically on i686-linux-gnu with 6/7 patches
> applied:
>
> $ ./testrun.sh nptl/tst-rwlock7 --direct
> 0: got timedrdlock
> child: timedwrlock failed with ETIMEDOUT
> child: timedwrlock failed with EINVAL
> 1: got timedrdlock
> child: timedwrlock failed with ETIMEDOUT
> child: timedwrlock failed with EINVAL
> 2: got timedrdlock
> child: timedwrlock failed with ETIMEDOUT
> 2nd timedwrlock did not return EINVAL
> failure in round 2
Sorry, I hadn't spotted this part of your email until today. Luckily it's
clear what I got wrong and why I didn't see it on AArch64 and x86_64:
> > + struct timespec ts_invalid;
> > + (void) clock_gettime (CLOCK_REALTIME, &ts_invalid);
> > + ts_invalid.tv_sec += 10;
> > /* Note that the following operation makes ts invalid. */
> > - ts.tv_nsec += 1000000000;
> > + ts_invalid.tv_nsec += 1000000000000;
I got so used to converting from µs to ns that I added three extra zeroes
unnecessarily here. It fits in a 64-bit long, but not a 32-bit one.
Thanks. I shall do more testing on 32-bit targets.
Mike.
@@ -22,6 +22,7 @@
#include <stdio.h>
#include <string.h>
#include <sys/time.h>
+#include <support/timespec.h>
static int kind[] =
@@ -38,21 +39,15 @@ tf (void *arg)
pthread_rwlock_t *r = arg;
/* Timeout: 0.3 secs. */
- struct timeval tv;
- (void) gettimeofday (&tv, NULL);
+ struct timespec ts_start;
+ (void) clock_gettime(CLOCK_REALTIME, &ts_start);
- struct timespec ts;
- TIMEVAL_TO_TIMESPEC (&tv, &ts);
- ts.tv_nsec += 300000000;
- if (ts.tv_nsec >= 1000000000)
- {
- ts.tv_nsec -= 1000000000;
- ++ts.tv_sec;
- }
+ struct timespec ts_timeout = {0, 300000000};
+ timespec_add(&ts_timeout, &ts_start, &ts_timeout);
puts ("child calling timedrdlock");
- int err = pthread_rwlock_timedrdlock (r, &ts);
+ int err = pthread_rwlock_timedrdlock (r, &ts_timeout);
if (err == 0)
{
puts ("rwlock_timedrdlock returned");
@@ -68,24 +63,24 @@ tf (void *arg)
puts ("1st child timedrdlock done");
- struct timeval tv2;
- (void) gettimeofday (&tv2, NULL);
+ struct timespec ts_end;
+ (void) clock_gettime (CLOCK_REALTIME, &ts_end);
- timersub (&tv2, &tv, &tv);
+ struct timespec ts_duration;
+ timespec_sub(&ts_duration, &ts_end, &ts_start);
- if (tv.tv_usec < 200000)
+ if (ts_duration.tv_sec !=0 || ts_duration.tv_nsec < 200000000)
{
puts ("timeout too short");
pthread_exit ((void *) 1l);
}
- (void) gettimeofday (&tv, NULL);
- TIMEVAL_TO_TIMESPEC (&tv, &ts);
- ts.tv_sec += 10;
+ (void) clock_gettime (CLOCK_REALTIME, &ts_timeout);
+ ts_timeout.tv_sec += 10;
/* Note that the following operation makes ts invalid. */
- ts.tv_nsec += 1000000000;
+ ts_timeout.tv_nsec += 1000000000;
- err = pthread_rwlock_timedrdlock (r, &ts);
+ err = pthread_rwlock_timedrdlock (r, &ts_timeout);
if (err == 0)
{
puts ("2nd timedrdlock succeeded");
@@ -136,12 +131,8 @@ do_test (void)
exit (1);
}
- struct timeval tv;
- (void) gettimeofday (&tv, NULL);
-
struct timespec ts;
- TIMEVAL_TO_TIMESPEC (&tv, &ts);
-
+ (void) clock_gettime (CLOCK_REALTIME, &ts);
++ts.tv_sec;
/* Get a write lock. */
@@ -154,8 +145,7 @@ do_test (void)
puts ("1st timedwrlock done");
- (void) gettimeofday (&tv, NULL);
- TIMEVAL_TO_TIMESPEC (&tv, &ts);
+ (void) clock_gettime (CLOCK_REALTIME, &ts);
++ts.tv_sec;
e = pthread_rwlock_timedrdlock (&r, &ts);
if (e == 0)
@@ -171,8 +161,7 @@ do_test (void)
puts ("1st timedrdlock done");
- (void) gettimeofday (&tv, NULL);
- TIMEVAL_TO_TIMESPEC (&tv, &ts);
+ (void) clock_gettime (CLOCK_REALTIME, &ts);
++ts.tv_sec;
e = pthread_rwlock_timedwrlock (&r, &ts);
if (e == 0)
@@ -22,6 +22,7 @@
#include <stdio.h>
#include <string.h>
#include <sys/time.h>
+#include <support/timespec.h>
static int kind[] =
@@ -38,19 +39,12 @@ tf (void *arg)
pthread_rwlock_t *r = arg;
/* Timeout: 0.3 secs. */
- struct timeval tv;
- (void) gettimeofday (&tv, NULL);
+ struct timespec ts_start;
+ (void) clock_gettime (CLOCK_REALTIME, &ts_start);
+ struct timespec ts_timeout = {0, 300000000};
+ timespec_add(&ts_timeout, &ts_start, &ts_timeout);
- struct timespec ts;
- TIMEVAL_TO_TIMESPEC (&tv, &ts);
- ts.tv_nsec += 300000000;
- if (ts.tv_nsec >= 1000000000)
- {
- ts.tv_nsec -= 1000000000;
- ++ts.tv_sec;
- }
-
- int err = pthread_rwlock_timedwrlock (r, &ts);
+ int err = pthread_rwlock_timedwrlock (r, &ts_timeout);
if (err == 0)
{
puts ("rwlock_timedwrlock returned");
@@ -65,24 +59,24 @@ tf (void *arg)
}
puts ("child: timedwrlock failed with ETIMEDOUT");
- struct timeval tv2;
- (void) gettimeofday (&tv2, NULL);
-
- timersub (&tv2, &tv, &tv);
+ struct timespec ts_end;
+ (void) clock_gettime (CLOCK_REALTIME, &ts_end);
+ struct timespec ts_diff;
+ timespec_sub (&ts_diff, &ts_end, &ts_start);
- if (tv.tv_usec < 200000)
+ if (ts_diff.tv_sec != 0 || ts_diff.tv_nsec < 200000000)
{
puts ("timeout too short");
pthread_exit ((void *) 1l);
}
- (void) gettimeofday (&tv, NULL);
- TIMEVAL_TO_TIMESPEC (&tv, &ts);
- ts.tv_sec += 10;
+ struct timespec ts_invalid;
+ (void) clock_gettime (CLOCK_REALTIME, &ts_invalid);
+ ts_invalid.tv_sec += 10;
/* Note that the following operation makes ts invalid. */
- ts.tv_nsec += 1000000000;
+ ts_invalid.tv_nsec += 1000000000000;
- err = pthread_rwlock_timedwrlock (r, &ts);
+ err = pthread_rwlock_timedwrlock (r, &ts_invalid);
if (err == 0)
{
puts ("2nd timedwrlock succeeded");
@@ -132,11 +126,8 @@ do_test (void)
exit (1);
}
- struct timeval tv;
- (void) gettimeofday (&tv, NULL);
-
struct timespec ts;
- TIMEVAL_TO_TIMESPEC (&tv, &ts);
+ (void) clock_gettime (CLOCK_REALTIME, &ts);
++ts.tv_sec;
@@ -56,9 +56,7 @@ writer_thread (void *nr)
int e;
do
{
- struct timeval tv;
- (void) gettimeofday (&tv, NULL);
- TIMEVAL_TO_TIMESPEC (&tv, &ts);
+ (void) clock_gettime (CLOCK_REALTIME, &ts);
ts.tv_nsec += 2 * TIMEOUT;
if (ts.tv_nsec >= 1000000000)
@@ -110,9 +108,7 @@ reader_thread (void *nr)
int e;
do
{
- struct timeval tv;
- (void) gettimeofday (&tv, NULL);
- TIMEVAL_TO_TIMESPEC (&tv, &ts);
+ (void) clock_gettime (CLOCK_REALTIME, &ts);
ts.tv_nsec += TIMEOUT;
if (ts.tv_nsec >= 1000000000)
new file mode 100644
@@ -0,0 +1,27 @@
+static inline void
+timespec_sub (struct timespec *diff, const struct timespec *left,
+ const struct timespec *right)
+{
+ diff->tv_sec = left->tv_sec - right->tv_sec;
+ diff->tv_nsec = left->tv_nsec - right->tv_nsec;
+
+ if (diff->tv_nsec < 0)
+ {
+ --diff->tv_sec;
+ diff->tv_nsec += 1000000000;
+ }
+}
+
+static inline void
+timespec_add (struct timespec *sum, const struct timespec *left,
+ const struct timespec *right)
+{
+ sum->tv_sec = left->tv_sec + right->tv_sec;
+ sum->tv_nsec = left->tv_nsec + right->tv_nsec;
+
+ if (sum->tv_nsec >= 1000000000)
+ {
+ ++sum->tv_sec;
+ sum->tv_nsec -= 1000000000;
+ }
+}