From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 2544B20248 for ; Tue, 5 Mar 2019 18:02:35 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=NVAKSaePMZCKhX17 lEWur5VtjtTUGJI09jwXkKx/sZL1EZNn2kkRUHnaCOM/pyQiuDB9dIqtgUHFnF/x 6piL3CPIql0Nokzg25Lw23jkzATROshwBwnH/O1f3/6USAw+yc/kg1WmR2hwAHAU 7cGh7sJDyrD6h/mNDHGb9FcrL0o= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=tn0gklAxsxeZcRZmGee/V2 DJ5A8=; b=KAiD/q0dLy3ttutN3b28+clhOmMErULu1LY5dPzmF3QCkNwRTeobMV RlDfbUu1UfAoD9ncsQpUlU2kMgmQkqOnkJzDg4jMOpo1fTyfMo1yZyfGzROQxtDj 5oJ0RX4s+kYu2lPqbNq1LmakMLz902GkR5tsjuCGpstoxtrINXXZ4= Received: (qmail 57263 invoked by alias); 5 Mar 2019 18:02:32 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 57149 invoked by uid 89); 5 Mar 2019 18:02:31 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-qt1-f196.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:references:from:openpgp:autocrypt:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=snzcurF084J8Yzixc5GZh1DPMFJt/nOnqiHHKkQkqV0=; b=FIGV+NWeSLiRtPQfpmx8mN+vz5x7rsyI77X21fwh48eKfWvu79nCkIGSERqSNHYODM xjEh05dyrFQrwDUEPIfjwHoB/0TbKXc/vPMKsxvbO90s/hpSHntZrJGu166yxH7pfSLT 607uzrLCEoL8jofpUaNF3HQsprt06AfYSQLTAbxUD8dcG1a/XjVg/Z9PtyqSMx9l3BBH rLlCM5yZVom2siUw5uDIyPOtfJUo/uCP9lAMARrlOpDQs51dlcJUUNmAa8nLQvAEZq9w 0rkVusLC+K+hOYtHSRyieDGAAJ0M0vOoRoKSMdD3RIHPQxrKg+eE6fMZmmKelnRZchOk 1s6A== To: libc-alpha@sourceware.org References: From: Adhemerval Zanella Openpgp: preference=signencrypt Subject: Re: [PATCH 6/7] nptl/tst-rwlock: Use clock_gettime/timespec rather than gettimeofday/timeval Message-ID: <8bff34e7-d471-8dfa-0959-f7535a7cad57@linaro.org> Date: Tue, 5 Mar 2019 15:02:21 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit 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 > #include > #include > +#include > > > 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 > #include > #include > +#include > > > 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.