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 06D6C1F454 for ; Thu, 7 Nov 2019 19:15:52 +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=PCSjxLI16eP9bFfX eYsXYkPhI0mx/yw6V3PDwKasD6JKnL1ppLnl8prZSg/ZlB49KQ+AV11XL+YXC5xY DS69T+vj23haZ/5QKcYeXedfOQbb0ZIJL5ChpaBYp7TLIfNdCdFgpBgQuSyM4zaA GkTxrA1x2jTcsVIdqND7z/MBCpQ= 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=Oqh5LoKh9dmGiV2dGQJL87 4Uj1Q=; b=xR9kGlBNEAOwfd29yDY3AgozW849S3r0l0hiDL2fAmQ9PgKxfTpoGF NF8HnJL/dDs6HIx7D/VPBraM1oiBMxSXUMpVAFJ1eqCX6I9izujLOR/l9CJuY+Qu 9jWEM+hner7IUbTZiFQkRaEeo7wha2GFW+UNpqY0yN27BF8U+ES2E= Received: (qmail 88744 invoked by alias); 7 Nov 2019 19:15:47 -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 88523 invoked by uid 89); 7 Nov 2019 19:15:27 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-qv1-f67.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=Uz73ceGKPCviSmgB5KOrX/LupxpIHUgebJs8WmiCs9Q=; b=OcjbJwocmj9WDeqN874H5EVRPuUVCBRPJ7Q/JCa1cBths0hiBEH6z3vlI/v1Arj6Xi gNjyCuegd1eY9y9TLp+M94RtyVCorHShedNvfwUcqYPHSGSYS7HObCAOHJB1OssCe1TI HOGmYu5IMPdOP+aKlfmWU8XkDChaVf4KIrzmwyb8rRUXMmcVsp/GHyoR35AeFncbc9Dn E5a5ycvbE6XVTlGVyXUwwdQ9okpm/8oikstd/mEDDejVDnUtyFtyHwMqo1IeS7VM9jPZ kNAaFe2vRVWOXCQUp1AE8D7623UbP+Mj3p1QhlgTAglidelE9nWFJzhylz4lpCQOa6ri yGvw== To: libc-alpha@sourceware.org References: <20191107181142.1048-1-alistair.francis@wdc.com> From: Adhemerval Zanella Openpgp: preference=signencrypt Subject: Re: [PATCH v5] sysdeps/clock_nanosleep: Use clock_nanosleep_time64 if avaliable Message-ID: Date: Thu, 7 Nov 2019 16:15:10 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20191107181142.1048-1-alistair.francis@wdc.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 07/11/2019 15:11, Alistair Francis wrote: > The clock_nanosleep syscall is not supported on newer 32-bit platforms (such > as RV32). To fix this issue let's use clock_nanosleep_time64 if it is > avaliable. > --- > This was patch was runtime tested with RV32 and RV64 > It was build tested using the ./scripts/build-many-glibcs.py script. > > I also ran: > $ make ; make install ; make check > tests on native ARM (32-bit) with the following three confiugrations: > - 4.19 Kernel and 4.19 Headers > - 5.2 Kernel and 4.19 Headers > - 5.2 Kernel and 5.2 Headers > and didn't see any regressions > > v5: > - Fix clock_nanosleep syscall > - Rebase on master > > v4: > - Rebase on master > - Use __clock_nanosleep to avoid duplicate implementations > - Fix the error handling when a syscall fails > v2: > - Explicitly include `#include ` > > include/time.h | 20 +++++++ > sysdeps/unix/sysv/linux/clock_nanosleep.c | 66 +++++++++++++++++++++-- > 2 files changed, 81 insertions(+), 5 deletions(-) > > diff --git a/include/time.h b/include/time.h > index b3e635395db..03389bda290 100644 > --- a/include/time.h > +++ b/include/time.h > @@ -209,6 +209,26 @@ libc_hidden_proto (__difftime64) > > extern double __difftime (time_t time1, time_t time0); > > +#if __TIMESIZE == 64 > +# define __thrd_sleep_time64 thrd_sleep I think it should be on its own patch. > +# define __clock_nanosleep_time64 __clock_nanosleep > +# define __nanosleep_time64 __nanosleep Same as before. > +# define __nanosleep_nocancel_time64 __nanosleep_nocancel There is no need to redirect this inexistent symbol. > +#else > +extern int __thrd_sleep_time64 (const struct __timespec64* time_point, > + struct __timespec64* remaining); As before. > +libc_hidden_proto (__thrd_sleep_time64) > +extern int __clock_nanosleep_time64 (clockid_t clock_id, > + int flags, const struct __timespec64 *req, > + struct __timespec64 *rem); > +libc_hidden_proto (__clock_nanosleep_time64) > +extern int __nanosleep_time64 (const struct __timespec64 *requested_time, > + struct __timespec64 *remaining); As before. > +libc_hidden_proto (__nanosleep_time64) > +extern int __nanosleep_nocancel_time64 (const struct __timespec64 *requested_time, > + struct __timespec64 *remaining); > +libc_hidden_proto (__nanosleep_nocancel_time64) As before. > +#endif > > /* Use in the clock_* functions. Size of the field representing the > actual clock ID. */ > diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c > index f3c6fd2d5f7..7212dcf9c6d 100644 > --- a/sysdeps/unix/sysv/linux/clock_nanosleep.c > +++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c > @@ -16,6 +16,7 @@ > . */ > > #include > +#include > #include > > #include > @@ -26,9 +27,11 @@ > /* We can simply use the syscall. The CPU clocks are not supported > with this function. */ > int > -__clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req, > - struct timespec *rem) > +__clock_nanosleep_time64 (clockid_t clock_id, int flags, const struct __timespec64 *req, > + struct __timespec64 *rem) > { > + int r; > + > if (clock_id == CLOCK_THREAD_CPUTIME_ID) > return EINVAL; > if (clock_id == CLOCK_PROCESS_CPUTIME_ID) Ok. > @@ -37,11 +40,64 @@ __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req, > /* If the call is interrupted by a signal handler or encounters an error, > it returns a positive value similar to errno. */ > INTERNAL_SYSCALL_DECL (err); > - int r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, clock_id, flags, > - req, rem); > + > +#ifdef __ASSUME_TIME64_SYSCALLS > +# ifndef __NR_clock_nanosleep_time64 > +# define __NR_clock_nanosleep_time64 __NR_clock_nanosleep > +# endif > + r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, clock_id, > + flags, req, rem); Ok. > +#else > +# ifdef __NR_clock_nanosleep_time64 > + r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, clock_id, > + flags, req, rem); > + > + if (r == 0 || errno != ENOSYS) > + { > + return (INTERNAL_SYSCALL_ERROR_P (r, err) > + ? INTERNAL_SYSCALL_ERRNO (r, err) : 0); > + } Ok. > +# endif /* __NR_clock_nanosleep_time64 */ > + struct timespec ts32, tr32; > + > + if (! in_time_t_range (req->tv_sec)) > + { > + __set_errno (EOVERFLOW); > + return -1; > + } > + > + ts32 = valid_timespec64_to_timespec (*req); > + r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, clock_id, flags, > + &ts32, &tr32); > + > + if ((r == 0 || errno != ENOSYS) && rem) > + *rem = valid_timespec_to_timespec64 (tr32); I think we can assume __NR_nanosleep here. > +#endif /* __ASSUME_TIME64_SYSCALLS */ > + > return (INTERNAL_SYSCALL_ERROR_P (r, err) > - ? INTERNAL_SYSCALL_ERRNO (r, err) : 0); > + ? INTERNAL_SYSCALL_ERRNO (r, err) : 0); > } This is just replacing a tab with spaces, I don't think it is really required. > + > +#if __TIMESIZE != 64 > +int > +__clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req, > + struct timespec *rem) > +{ > + int r; > + struct __timespec64 treq64, trem64; > + > + treq64 = valid_timespec_to_timespec64 (*req); > + r = __clock_nanosleep_time64 (clock_id, flags, &treq64, &trem64); > + > + if (r == 0 || errno != ENOSYS) > + { > + if (rem) > + *rem = valid_timespec64_to_timespec (trem64); Wouldn't we copy uninitialised data for EINVAL/ENOTSUPP/EFAULT? > + } > + > + return r; > +} > +#endif > libc_hidden_def (__clock_nanosleep) > versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17); > /* clock_nanosleep moved to libc in version 2.17; > Ok