From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: Wilco Dijkstra <Wilco.Dijkstra@arm.com>,
'GNU C Library' <libc-alpha@sourceware.org>
Cc: nd <nd@arm.com>
Subject: Re: [PATCHv2 1/4] Cleanup clock_*time includes
Date: Mon, 28 Jan 2019 15:08:07 -0200 [thread overview]
Message-ID: <73eefefe-cf64-fb0a-9505-0f5eb62f468d@linaro.org> (raw)
In-Reply-To: <DB5PR08MB10300F4B507034FBE758D011839B0@DB5PR08MB1030.eurprd08.prod.outlook.com>
On 25/01/2019 14:15, Wilco Dijkstra wrote:
> (resend patch as part of hp-timing cleanup)
>
> Clock_gettime, settime and getres implementations are unnecessarily
> complex due to using many defines and C file inclusion. Simplify the
> code by replacing the redundant defines and removing the inclusion,
> making it much easier to understand. No functional changes.
>
> OK for commit (for 2.30)?
https://sourceware.org/ml/libc-alpha/2019-01/msg00364.html
>
> ChangeLog:
> 2019-01-14 Wilco Dijkstra <wdijkstr@arm.com>
>
> * sysdeps/posix/clock_getres.c (__clock_getres): Cleanup.
> * sysdeps/unix/clock_gettime.c (__clock_gettime): Cleanup.
> * sysdeps/unix/clock_settime.c (__clock_settime): Cleanup.
> * sysdeps/unix/sysv/linux/clock_getres.c (__clock_getres): Cleanup.
> * sysdeps/unix/sysv/linux/clock_gettime.c (__clock_gettime): Cleanup.
> * sysdeps/unix/sysv/linux/clock_settime.c (__clock_settime): Cleanup.
>
> --
> diff --git a/sysdeps/posix/clock_getres.c b/sysdeps/posix/clock_getres.c
> index f47715ed6b48a54248fb375fbbeb55008b995104..dac4761fcc2c86cb0d53425cab4e88d587696540 100644
> --- a/sysdeps/posix/clock_getres.c
> +++ b/sysdeps/posix/clock_getres.c
> @@ -82,20 +82,11 @@ __clock_getres (clockid_t clock_id, struct timespec *res)
>
> switch (clock_id)
> {
> -#ifdef SYSDEP_GETRES
> - SYSDEP_GETRES;
> -#endif
> -
> -#ifndef HANDLED_REALTIME
> case CLOCK_REALTIME:
> retval = realtime_getres (res);
> break;
> -#endif /* handled REALTIME */
>
> default:
> -#ifdef SYSDEP_GETRES_CPU
> - SYSDEP_GETRES_CPU;
> -#endif
> #if HP_TIMING_AVAIL
> if ((clock_id & ((1 << CLOCK_IDFIELD_SIZE) - 1))
> == CLOCK_THREAD_CPUTIME_ID)
> @@ -105,7 +96,7 @@ __clock_getres (clockid_t clock_id, struct timespec *res)
> __set_errno (EINVAL);
> break;
>
> -#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
> +#if HP_TIMING_AVAIL
> case CLOCK_PROCESS_CPUTIME_ID:
> case CLOCK_THREAD_CPUTIME_ID:
> retval = hp_timing_getres (res);
> diff --git a/sysdeps/unix/clock_gettime.c b/sysdeps/unix/clock_gettime.c
> index 138991d1bc081cea004e94defe361580dfb022dc..33a1f3335c269eec2b4f01fb927e3836a191b5eb 100644
> --- a/sysdeps/unix/clock_gettime.c
> +++ b/sysdeps/unix/clock_gettime.c
> @@ -95,11 +95,6 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
>
> switch (clock_id)
> {
> -#ifdef SYSDEP_GETTIME
> - SYSDEP_GETTIME;
> -#endif
> -
> -#ifndef HANDLED_REALTIME
> case CLOCK_REALTIME:
> {
> struct timeval tv;
> @@ -108,12 +103,8 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
> TIMEVAL_TO_TIMESPEC (&tv, tp);
> }
> break;
> -#endif
>
> default:
> -#ifdef SYSDEP_GETTIME_CPU
> - SYSDEP_GETTIME_CPU (clock_id, tp);
> -#endif
> #if HP_TIMING_AVAIL
> if ((clock_id & ((1 << CLOCK_IDFIELD_SIZE) - 1))
> == CLOCK_THREAD_CPUTIME_ID)
> @@ -123,7 +114,7 @@ __clock_gettime (clockid_t clock_id, struct timespec *tp)
> __set_errno (EINVAL);
> break;
>
> -#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
> +#if HP_TIMING_AVAIL
> case CLOCK_PROCESS_CPUTIME_ID:
> retval = hp_timing_gettime (clock_id, tp);
> break;
> diff --git a/sysdeps/unix/clock_settime.c b/sysdeps/unix/clock_settime.c
> index b589e875cf2dfb5fc083550e01e774a6f4d0cae6..24d73b048c1b891906dacdf0c199544b85d924d2 100644
> --- a/sysdeps/unix/clock_settime.c
> +++ b/sysdeps/unix/clock_settime.c
> @@ -21,7 +21,7 @@
> #include <ldsodefs.h>
>
>
> -#if HP_TIMING_AVAIL && !defined HANDLED_CPUTIME
> +#if HP_TIMING_AVAIL
> /* Clock frequency of the processor. We make it a 64-bit variable
> because some jokers are already playing with processors with more
> than 4GHz. */
> @@ -84,29 +84,15 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp)
>
> switch (clock_id)
> {
> -#define HANDLE_REALTIME \
> - do { \
> - struct timeval tv; \
> - TIMESPEC_TO_TIMEVAL (&tv, tp); \
> - \
> - retval = __settimeofday (&tv, NULL); \
> - } while (0)
> -
> -#ifdef SYSDEP_SETTIME
> - SYSDEP_SETTIME;
> -#endif
> -
> -#ifndef HANDLED_REALTIME
> case CLOCK_REALTIME:
> - HANDLE_REALTIME;
> + do {
> + struct timeval tv;
> + TIMESPEC_TO_TIMEVAL (&tv, tp);
> + retval = __settimeofday (&tv, NULL);
> + } while (0);
> break;
> -#endif
>
> default:
> -#ifdef SYSDEP_SETTIME_CPU
> - SYSDEP_SETTIME_CPU;
> -#endif
> -#ifndef HANDLED_CPUTIME
> # if HP_TIMING_AVAIL
> if (CPUCLOCK_WHICH (clock_id) == CLOCK_PROCESS_CPUTIME_ID
> || CPUCLOCK_WHICH (clock_id) == CLOCK_THREAD_CPUTIME_ID)
> @@ -117,7 +103,6 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp)
> __set_errno (EINVAL);
> retval = -1;
> }
> -#endif
> break;
> }
>
> diff --git a/sysdeps/unix/sysv/linux/clock_getres.c b/sysdeps/unix/sysv/linux/clock_getres.c
> index a24532888fd3f3e256f34fb5a7636cee8b0b7904..bb2ee109bd66f916cc790899fbc205c8890098ae 100644
> --- a/sysdeps/unix/sysv/linux/clock_getres.c
> +++ b/sysdeps/unix/sysv/linux/clock_getres.c
> @@ -26,26 +26,11 @@
> #endif
> #include <sysdep-vdso.h>
>
> -#define SYSCALL_GETRES \
> - retval = INLINE_VSYSCALL (clock_getres, 2, clock_id, res); \
> - break
> +/* Get resolution of clock. */
> +int
> +__clock_getres (clockid_t clock_id, struct timespec *res)
> +{
> + return INLINE_VSYSCALL (clock_getres, 2, clock_id, res);
> +}
> +weak_alias (__clock_getres, clock_getres)
>
> -/* The REALTIME and MONOTONIC clock are definitely supported in the
> - kernel. */
> -#define SYSDEP_GETRES \
> - SYSDEP_GETRES_CPUTIME \
> - case CLOCK_REALTIME: \
> - case CLOCK_MONOTONIC: \
> - case CLOCK_MONOTONIC_RAW: \
> - case CLOCK_REALTIME_COARSE: \
> - case CLOCK_MONOTONIC_COARSE: \
> - SYSCALL_GETRES
> -
> -/* We handled the REALTIME clock here. */
> -#define HANDLED_REALTIME 1
> -#define HANDLED_CPUTIME 1
> -
> -#define SYSDEP_GETRES_CPU SYSCALL_GETRES
> -#define SYSDEP_GETRES_CPUTIME /* Default catches them too. */
> -
> -#include <sysdeps/posix/clock_getres.c>
> diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c b/sysdeps/unix/sysv/linux/clock_gettime.c
> index 9af806f76ef34ad711a9f94de77599d6e17fe421..2ad7846a4d811635c40daab62bc0eae9d7d3b7ac 100644
> --- a/sysdeps/unix/sysv/linux/clock_gettime.c
> +++ b/sysdeps/unix/sysv/linux/clock_gettime.c
> @@ -26,22 +26,12 @@
> #endif
> #include <sysdep-vdso.h>
>
> -/* The REALTIME and MONOTONIC clock are definitely supported in the
> - kernel. */
> -#define SYSDEP_GETTIME \
> - SYSDEP_GETTIME_CPUTIME; \
> - case CLOCK_REALTIME: \
> - case CLOCK_MONOTONIC: \
> - retval = INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp); \
> - break
> +/* Get current value of CLOCK and store it in TP. */
> +int
> +__clock_gettime (clockid_t clock_id, struct timespec *tp)
> +{
> + return INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);
> +}
> +weak_alias (__clock_gettime, clock_gettime)
> +libc_hidden_def (__clock_gettime)
>
> -/* We handled the REALTIME clock here. */
> -#define HANDLED_REALTIME 1
> -#define HANDLED_CPUTIME 1
> -
> -#define SYSDEP_GETTIME_CPU(clock_id, tp) \
> - retval = INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp); \
> - break
> -#define SYSDEP_GETTIME_CPUTIME /* Default catches them too. */
> -
> -#include <sysdeps/unix/clock_gettime.c>
> diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c
> index 7e2432b244a40ac620c39010a7e0a556f73a9c79..c5668012e0819059f9bfdd684616e0c9add2b115 100644
> --- a/sysdeps/unix/sysv/linux/clock_settime.c
> +++ b/sysdeps/unix/sysv/linux/clock_settime.c
> @@ -21,18 +21,18 @@
>
> #include "kernel-posix-cpu-timers.h"
>
> +/* Set CLOCK to value TP. */
> +int
> +__clock_settime (clockid_t clock_id, const struct timespec *tp)
> +{
> + /* Make sure the time cvalue is OK. */
> + if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000)
> + {
> + __set_errno (EINVAL);
> + return -1;
> + }
> +
> + return INLINE_SYSCALL (clock_settime, 2, clock_id, tp);
> +}
> +weak_alias (__clock_settime, clock_settime)
>
> -/* The REALTIME clock is definitely supported in the kernel. */
> -#define SYSDEP_SETTIME \
> - case CLOCK_REALTIME: \
> - retval = INLINE_SYSCALL (clock_settime, 2, clock_id, tp); \
> - break
> -
> -/* We handled the REALTIME clock here. */
> -#define HANDLED_REALTIME 1
> -
> -#define HANDLED_CPUTIME 1
> -#define SYSDEP_SETTIME_CPU \
> - retval = INLINE_SYSCALL (clock_settime, 2, clock_id, tp)
> -
> -#include <sysdeps/unix/clock_settime.c>
>
next prev parent reply other threads:[~2019-01-28 17:08 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-25 16:15 [PATCHv2 1/4] Cleanup clock_*time includes Wilco Dijkstra
2019-01-28 17:08 ` Adhemerval Zanella [this message]
2019-01-29 14:07 ` Wilco Dijkstra
2019-01-29 19:11 ` Adhemerval Zanella
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/libc/involved.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=73eefefe-cf64-fb0a-9505-0f5eb62f468d@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=Wilco.Dijkstra@arm.com \
--cc=libc-alpha@sourceware.org \
--cc=nd@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).