This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 6/7] nptl/tst-rwlock: Use clock_gettime/timespec rather than gettimeofday/timeval
- From: Mike Crowe <mac at mcrowe dot com>
- To: libc-alpha at sourceware dot org
- Date: Wed, 20 Mar 2019 21:34:05 +0000
- Subject: Re: [PATCH 6/7] nptl/tst-rwlock: Use clock_gettime/timespec rather than gettimeofday/timeval
- References: <cover.b0c66849a87ca79889a49f2f1f2563b1a8a15d8b.1551291557.git-series.mac@mcrowe.com> <e7a012d809a6f0cf8b33b923345ebe05b5d739e1.1551291557.git-series.mac@mcrowe.com> <8bff34e7-d471-8dfa-0959-f7535a7cad57@linaro.org>
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.