Message ID | e7a012d809a6f0cf8b33b923345ebe05b5d739e1.1551291557.git-series.mac@mcrowe.com |
---|---|
State | New |
Headers | show |
Series | Implement proposed POSIX _clockwait variants of existing _timedwait functions | expand |
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.
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); - 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) 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); - 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; 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) 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; + } +}