From patchwork Wed Mar 20 14:13:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 31912 Received: (qmail 70337 invoked by alias); 20 Mar 2019 14:13:30 -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 Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 70328 invoked by uid 89); 20 Mar 2019 14:13:29 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-ua1-f68.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:cc:references:from:openpgp:autocrypt:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=TDO6i9i9nsScdTf7S0JpRJpQn1zza+ntcFXu340F3XI=; b=d/StQUgbef3VSOyN85QbGg9BqxMYuM1VfnTXlaGVcrEGKVfV/8sB7x7K9C8OmZy2Xs vrPZj0gzFjixcEnhQYJo37Wtqagq9VSctP0BLl3YI5IA+mqp8c4xqVRjNb3UvkW5aWw1 3vYTlMezU0bXXDg1a0u/S+UP4pZ5nTiuL+u1gBzjAQSXojy2u7oHQAofEtSzxNNBEQwL UNYb7OA78e2z/A4R03wtoiEkMBsnpgtKhVTeU+02w/18uGL9RjBglXVBlePyMXoHmgrL PeCvhAtcUZG7c74tyPOUl6pQXh8vEzu5mQwbnpuTuV1sgov1bd/HmwHSWN+AaJAChq1i Q50A== Return-Path: To: Andreas Schwab Cc: libc-alpha@sourceware.org References: <20190218211128.1869-1-adhemerval.zanella@linaro.org> <20190218211128.1869-2-adhemerval.zanella@linaro.org> From: Adhemerval Zanella Openpgp: preference=signencrypt Subject: Re: [PATCH v2 2/6] linux: Assume clock_getres CLOCK_{PROCESS,THREAD}_CPUTIME_ID Message-ID: <871634e9-1c01-a573-12ee-de423a889572@linaro.org> Date: Wed, 20 Mar 2019 11:13:19 -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: On 20/03/2019 08:49, Andreas Schwab wrote: > On Feb 18 2019, Adhemerval Zanella wrote: > >> This patch assumes that clock_getres syscall always support >> CLOCK_PROCESS_CPUTIME_ID and CLOCK_THREAD_CPUTIME_ID, so there is no need >> to fallback to hp-timing support for _SC_MONOTONIC_CLOCK. This allows >> simplify the sysconf support to always use the syscall. > > Under which condition can clock_getres return an error for these clocks? Good question, checking kernel code kernel/posix-cpu-timers.c both thread_cpu_clock_getres and process_cpu_clock_getres calls posix_cpu_clock_getres. And it fails on check_clock only if an invalid clock is used (not the case) or if we pass an invalid the pid/tid in 29 msb of clock_id (not the case either). I will just remove the check_clock_getres call. > >> diff --git a/sysdeps/unix/sysv/linux/sysconf.c b/sysdeps/unix/sysv/linux/sysconf.c >> index 4b297ba35f..2027444488 100644 >> --- a/sysdeps/unix/sysv/linux/sysconf.c >> +++ b/sysdeps/unix/sysv/linux/sysconf.c >> @@ -35,33 +35,18 @@ >> static long int posix_sysconf (int name); >> >> >> -#ifndef HAS_CPUCLOCK >> static long int >> -has_cpuclock (int name) >> +check_clock_getres (clockid_t clk_id) >> { >> -# if defined __NR_clock_getres || HP_TIMING_AVAIL >> - /* If we have HP_TIMING, we will fall back on that if the system >> - call does not work, so we support it either way. */ >> -# if !HP_TIMING_AVAIL >> - /* Check using the clock_getres system call. */ >> struct timespec ts; >> INTERNAL_SYSCALL_DECL (err); >> - int r = INTERNAL_SYSCALL (clock_getres, err, 2, >> - (name == _SC_CPUTIME >> - ? CLOCK_PROCESS_CPUTIME_ID >> - : CLOCK_THREAD_CPUTIME_ID), >> - &ts); >> + /* Avoid setting errno to we can check whether the kernel supports > > s/to/so/ > >> + the CLK_ID. */ >> + int r = INTERNAL_SYSCALL_CALL (clock_getres, err, clk_id, &ts); >> if (INTERNAL_SYSCALL_ERROR_P (r, err)) >> return -1; >> -# endif >> return _POSIX_VERSION; >> -# else >> - return -1; >> -# endif >> } >> -# define HAS_CPUCLOCK(name) has_cpuclock (name) >> -#endif >> - >> >> /* Get the value of the system variable NAME. */ >> long int >> @@ -71,29 +56,21 @@ __sysconf (int name) >> >> switch (name) >> { >> - struct rlimit rlimit; >> -#ifdef __NR_clock_getres >> case _SC_MONOTONIC_CLOCK: >> - /* Check using the clock_getres system call. */ >> - { >> - struct timespec ts; >> - INTERNAL_SYSCALL_DECL (err); >> - int r; >> - r = INTERNAL_SYSCALL (clock_getres, err, 2, CLOCK_MONOTONIC, &ts); >> - return INTERNAL_SYSCALL_ERROR_P (r, err) ? -1 : _POSIX_VERSION; >> - } >> -#endif >> - >> + return check_clock_getres (CLOCK_MONOTONIC); >> case _SC_CPUTIME: >> + return check_clock_getres (CLOCK_PROCESS_CPUTIME_ID); >> case _SC_THREAD_CPUTIME: >> - return HAS_CPUCLOCK (name); >> + return check_clock_getres (CLOCK_THREAD_CPUTIME_ID); >> >> - case _SC_ARG_MAX: >> + case _SC_ARG_MAX: { > > Brace on next line. Ack. > >> + struct rlimit rlimit; >> /* Use getrlimit to get the stack limit. */ >> if (__getrlimit (RLIMIT_STACK, &rlimit) == 0) >> return MAX (legacy_ARG_MAX, rlimit.rlim_cur / 4); >> >> return legacy_ARG_MAX; >> + } break; > > No break needed. Ack. > >> >> case _SC_NGROUPS_MAX: >> /* Try to read the information from the /proc/sys/kernel/ngroups_max >> @@ -101,13 +78,14 @@ __sysconf (int name) >> procfname = "/proc/sys/kernel/ngroups_max"; >> break; >> >> - case _SC_SIGQUEUE_MAX: >> + case _SC_SIGQUEUE_MAX: { > > Brace on next line. Ack. > >> + struct rlimit rlimit; >> if (__getrlimit (RLIMIT_SIGPENDING, &rlimit) == 0) >> return rlimit.rlim_cur; >> >> /* The /proc/sys/kernel/rtsig-max file contains the answer. */ >> procfname = "/proc/sys/kernel/rtsig-max"; >> - break; >> + } break; > > Line break after brace. Ack. > > Andreas. > I changed to: --- For Linux 3.2, the kernel code for clock_getres (kernel/posix-cpu-timers.c) issued for clock_getres CLOCK_PROCESS_CPUTIME_ID (process_cpu_clock_getres) and CLOCK_THREAD_CPUTIME_ID (thread_cpu_clock_getres) calls posix_cpu_clock_getres. And it fails on check_clock only if an invalid clock is used (not the case) or if we pass an invalid the pid/tid in 29 msb of clock_id (not the case either). This patch assumes that clock_getres syscall always support CLOCK_PROCESS_CPUTIME_ID and CLOCK_THREAD_CPUTIME_ID, so there is no need to fallback to hp-timing support for _SC_MONOTONIC_CLOCK neither to issue the syscall to certify the clock_id is supported bt the kernel. This allows simplify the sysconf support to always use the syscall. it also removes ia64 itc drift check and assume kernel handles it correctly. Checked on aarch64-linux-gnu, x86_64-linux-gnu, and i686-linux-gnu. * sysdeps/unix/sysv/linux/ia64/has_cpuclock.c: Remove file. * sysdeps/unix/sysv/linux/ia64/sysconf.c: Likewise. * sysdeps/unix/sysv/linux/sysconf.c (has_cpuclock): Remove function. (__sysconf): Assume kernel support for _SC_MONOTONIC_CLOCK, _SC_CPUTIME, and _SC_THREAD_CPUTIME. --- sysdeps/unix/sysv/linux/ia64/has_cpuclock.c | 51 ---------------- sysdeps/unix/sysv/linux/ia64/sysconf.c | 30 ---------- sysdeps/unix/sysv/linux/sysconf.c | 64 +++++---------------- 3 files changed, 15 insertions(+), 130 deletions(-) delete mode 100644 sysdeps/unix/sysv/linux/ia64/has_cpuclock.c delete mode 100644 sysdeps/unix/sysv/linux/ia64/sysconf.c diff --git a/sysdeps/unix/sysv/linux/ia64/has_cpuclock.c b/sysdeps/unix/sysv/linux/ia64/has_cpuclock.c deleted file mode 100644 index b3afb37f8b..0000000000 --- a/sysdeps/unix/sysv/linux/ia64/has_cpuclock.c +++ /dev/null @@ -1,51 +0,0 @@ -/* Copyright (C) 2000-2019 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -#include -#include -#include -#include -#include -#include - -static int itc_usable; - -static int -has_cpuclock (void) -{ - if (__builtin_expect (itc_usable == 0, 0)) - { - int newval = 1; - int fd = __open_nocancel ("/proc/sal/itc_drift", O_RDONLY); - if (__builtin_expect (fd != -1, 1)) - { - char buf[16]; - /* We expect the file to contain a single digit followed by - a newline. If the format changes we better not rely on - the file content. */ - if (__read_nocancel (fd, buf, sizeof buf) != 2 - || buf[0] != '0' || buf[1] != '\n') - newval = -1; - - __close_nocancel_nostatus (fd); - } - - itc_usable = newval; - } - - return itc_usable; -} diff --git a/sysdeps/unix/sysv/linux/ia64/sysconf.c b/sysdeps/unix/sysv/linux/ia64/sysconf.c deleted file mode 100644 index ef75322f1f..0000000000 --- a/sysdeps/unix/sysv/linux/ia64/sysconf.c +++ /dev/null @@ -1,30 +0,0 @@ -/* Get file-specific information about a file. Linux/ia64 version. - Copyright (C) 2003-2019 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -#include -#include -#include -#include - - -#include "has_cpuclock.c" -#define HAS_CPUCLOCK(name) (has_cpuclock () ? _POSIX_VERSION : -1) - - -/* Now the generic Linux version. */ -#include diff --git a/sysdeps/unix/sysv/linux/sysconf.c b/sysdeps/unix/sysv/linux/sysconf.c index 4b297ba35f..2492d7a291 100644 --- a/sysdeps/unix/sysv/linux/sysconf.c +++ b/sysdeps/unix/sysv/linux/sysconf.c @@ -35,34 +35,6 @@ static long int posix_sysconf (int name); -#ifndef HAS_CPUCLOCK -static long int -has_cpuclock (int name) -{ -# if defined __NR_clock_getres || HP_TIMING_AVAIL - /* If we have HP_TIMING, we will fall back on that if the system - call does not work, so we support it either way. */ -# if !HP_TIMING_AVAIL - /* Check using the clock_getres system call. */ - struct timespec ts; - INTERNAL_SYSCALL_DECL (err); - int r = INTERNAL_SYSCALL (clock_getres, err, 2, - (name == _SC_CPUTIME - ? CLOCK_PROCESS_CPUTIME_ID - : CLOCK_THREAD_CPUTIME_ID), - &ts); - if (INTERNAL_SYSCALL_ERROR_P (r, err)) - return -1; -# endif - return _POSIX_VERSION; -# else - return -1; -# endif -} -# define HAS_CPUCLOCK(name) has_cpuclock (name) -#endif - - /* Get the value of the system variable NAME. */ long int __sysconf (int name) @@ -71,29 +43,20 @@ __sysconf (int name) switch (name) { - struct rlimit rlimit; -#ifdef __NR_clock_getres case _SC_MONOTONIC_CLOCK: - /* Check using the clock_getres system call. */ - { - struct timespec ts; - INTERNAL_SYSCALL_DECL (err); - int r; - r = INTERNAL_SYSCALL (clock_getres, err, 2, CLOCK_MONOTONIC, &ts); - return INTERNAL_SYSCALL_ERROR_P (r, err) ? -1 : _POSIX_VERSION; - } -#endif - case _SC_CPUTIME: case _SC_THREAD_CPUTIME: - return HAS_CPUCLOCK (name); + return _POSIX_VERSION; case _SC_ARG_MAX: - /* Use getrlimit to get the stack limit. */ - if (__getrlimit (RLIMIT_STACK, &rlimit) == 0) - return MAX (legacy_ARG_MAX, rlimit.rlim_cur / 4); + { + struct rlimit rlimit; + /* Use getrlimit to get the stack limit. */ + if (__getrlimit (RLIMIT_STACK, &rlimit) == 0) + return MAX (legacy_ARG_MAX, rlimit.rlim_cur / 4); - return legacy_ARG_MAX; + return legacy_ARG_MAX; + } case _SC_NGROUPS_MAX: /* Try to read the information from the /proc/sys/kernel/ngroups_max @@ -102,11 +65,14 @@ __sysconf (int name) break; case _SC_SIGQUEUE_MAX: - if (__getrlimit (RLIMIT_SIGPENDING, &rlimit) == 0) - return rlimit.rlim_cur; + { + struct rlimit rlimit; + if (__getrlimit (RLIMIT_SIGPENDING, &rlimit) == 0) + return rlimit.rlim_cur; - /* The /proc/sys/kernel/rtsig-max file contains the answer. */ - procfname = "/proc/sys/kernel/rtsig-max"; + /* The /proc/sys/kernel/rtsig-max file contains the answer. */ + procfname = "/proc/sys/kernel/rtsig-max"; + } break; default: