From: Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org>
To: Lukasz Majewski <lukma@denx.de>, Joseph Myers <joseph@codesourcery.com>
Cc: Florian Weimer <fweimer@redhat.com>,
GNU C Library <libc-alpha@sourceware.org>,
Alistair Francis <alistair.francis@wdc.com>,
Andreas Schwab <schwab@suse.de>
Subject: Re: [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation
Date: Tue, 19 May 2020 16:07:36 -0300 [thread overview]
Message-ID: <4450aefc-079f-d73d-5c70-ce8ed54b777b@linaro.org> (raw)
In-Reply-To: <20200508145640.16336-2-lukma@denx.de>
On 08/05/2020 11:56, Lukasz Majewski wrote:
> This patch replaces auto generated wrapper (as described in
> sysdeps/unix/sysv/linux/syscalls.list) for clock_adjtime with one which adds
> extra support for reading 64 bit time values on machines with __TIMESIZE != 64.
>
> To achieve this goal new __clock_adjtime64 explicit 64 bit function for
> adjusting Linux clock has been added.
> Moreover, a 32 bit version - __clock_adjtime has been refactored to internally
> use __clock_adjtime64.
>
> The __clock_adjtime is now supposed to be used on systems still supporting 32
> bit time (__TIMESIZE != 64) - hence the necessary conversions between 64 bit
> struct __timespec64 and struct timespec.
>
> The new __clock_adjtime64 syscall available from Linux 5.1+ has been used, when
> applicable.
> Up till v5.4 in the Linux kernel there was a bug preventing this call from
> obtaining correct struct's timex time.tv_sec time after time_t overflow
> (i.e. not being Y2038 safe).
>
> Build tests:
> - ./src/scripts/build-many-glibcs.py glibcs
>
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
> https://github.com/lmajewski/meta-y2038 and run tests:
> https://github.com/lmajewski/y2038-tests/commits/master
>
> Linux kernel, headers and minimal kernel version for glibc build test matrix:
> - Linux v5.1 (with clock_adjtime64) and glibc build with v5.1 as
> minimal kernel version (--enable-kernel="5.1.0")
> The __ASSUME_TIME64_SYSCALLS flag defined.
>
> - Linux v5.1 and default minimal kernel version
> The __ASSUME_TIME64_SYSCALLS not defined, but kernel supports clock_adjtime64
> syscall.
>
> - Linux v4.19 (no clock_adjtime64 support) with default minimal kernel version
> for contemporary glibc (3.2.0)
> This kernel doesn't support clock_adjtime64 syscall, so the fallback to
> clock_adjtime is tested.
>
> Above tests were performed with Y2038 redirection applied as well as without
> (so the __TIMESIZE != 64 execution path is checked as well).
>
> No regressions were observed.
>
> Changes for v2:
> - Fix wrong indentation
> - Remove the check for (ret >= TIME_OK && ret <= TIME_ERROR) in the
> clock_adjtime64 call - just check if syscall returned ENOSYS
> - Add 64 bit time overflow for ADJ_SETOFFSET set in modes
Ok with the changes below.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/unix/sysv/linux/Makefile | 2 +-
> sysdeps/unix/sysv/linux/clock_adjtime.c | 71 +++++++++++++++++++++
> sysdeps/unix/sysv/linux/include/sys/timex.h | 3 +
> sysdeps/unix/sysv/linux/syscalls.list | 1 -
> 4 files changed, 75 insertions(+), 2 deletions(-)
> create mode 100644 sysdeps/unix/sysv/linux/clock_adjtime.c
>
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 0326f92c40..2f2fd5d4d0 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -60,7 +60,7 @@ sysdep_routines += adjtimex clone umount umount2 readahead sysctl \
> personality epoll_wait tee vmsplice splice \
> open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get \
> timerfd_gettime timerfd_settime prctl \
> - process_vm_readv process_vm_writev
> + process_vm_readv process_vm_writev clock_adjtime
>
> CFLAGS-gethostid.c = -fexceptions
> CFLAGS-tee.c = -fexceptions -fasynchronous-unwind-tables
Ok.
> diff --git a/sysdeps/unix/sysv/linux/clock_adjtime.c b/sysdeps/unix/sysv/linux/clock_adjtime.c
> new file mode 100644
> index 0000000000..5c02031bb5
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/clock_adjtime.c
> @@ -0,0 +1,71 @@
> +/* clock_adjtime -- tune kernel clock
I think we usually add a final period in one line description.
> + Copyright (C) 2020 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; see the file COPYING.LIB. If
> + not, see <https://www.gnu.org/licenses/>. */
> +
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <time.h>
> +#include <sysdep.h>
> +#include <sys/timex.h>
> +#include <kernel-features.h>
> +
> +int
> +__clock_adjtime64 (const clockid_t clock_id, struct __timex64 *tx64)
> +{
> +#ifdef __ASSUME_TIME64_SYSCALLS
> +# ifndef __NR_clock_adjtime64
> +# define __NR_clock_adjtime64 __NR_clock_adjtime
> +# endif
> + return INLINE_SYSCALL_CALL (clock_adjtime64, clock_id, tx64);
> +#else
> + int ret = INLINE_SYSCALL_CALL (clock_adjtime64, clock_id, tx64);
> + if (errno != ENOSYS)
> + return ret;
> +
> + if (tx64->modes & ADJ_SETOFFSET
> + && ! in_time_t_range (tx64->time.tv_sec))
> + {
> + __set_errno (EOVERFLOW);
> + return -1;
> + }
> +
> + struct timex tx32 = valid_timex64_to_timex (*tx64);
> + int retval = INLINE_SYSCALL_CALL (clock_adjtime, clock_id, &tx32);
> + *tx64 = valid_timex_to_timex64 (tx32);> +
> + return retval;
The function might still return EINVAL for invalid clocks, ENODEV for
invalid dynamic ones, EOPNOTSUPP if the clock does not support adjustment,
or EPERM if buf.modes is neither 0 nor ADJ_OFFSET_SS_READ and the caller
does not have sufficient privilege.
So we still need to check the syscall return value prior setting the
tx64 value:
int retval = INLINE_SYSCALL_CALL (clock_adjtime, clock_id, &tx32);
if (retval >= 0)
*tx64 = valid_timex_to_timex64 (tx32);
return retval;
Ok with this change.
> +#endif
> +}
> +
> +#if __TIMESIZE != 64
> +libc_hidden_def (__clock_adjtime64)
> +
> +int
> +__clock_adjtime (const clockid_t clock_id, struct timex *tx)
> +{
> + struct __timex64 tx64;
> + int retval;
> +
> + tx64 = valid_timex_to_timex64 (*tx);
> + retval = __clock_adjtime64 (clock_id, &tx64);
Same as before, we need to check its return value prior setting TX:
retval = __clock_adjtime64 (clock_id, &tx64);
if (retval >= 0)
*tx = valid_timex64_to_timex (tx64);
return retval;
Ok with this change.
> + *tx = valid_timex64_to_timex (tx64);
> +
> + return retval;
> +}
> +#endif
> +libc_hidden_def (__clock_adjtime);
> +strong_alias (__clock_adjtime, clock_adjtime)
Ok.
> diff --git a/sysdeps/unix/sysv/linux/include/sys/timex.h b/sysdeps/unix/sysv/linux/include/sys/timex.h
> index 3efe7cd306..2848c5cf76 100644
> --- a/sysdeps/unix/sysv/linux/include/sys/timex.h
> +++ b/sysdeps/unix/sysv/linux/include/sys/timex.h
> @@ -30,6 +30,7 @@ libc_hidden_proto (__adjtimex)
> /* Local definition of 64 bit time supporting timex struct */
> # if __TIMESIZE == 64
> # define __timex64 timex
> +# define __clock_adjtime64 __clock_adjtime
> # else
>
> struct __timex64
> @@ -71,6 +72,8 @@ struct __timex64
> int :32;
> int :32;
> };
> +extern int __clock_adjtime64 (const clockid_t clock_id, struct __timex64 *tx64);
> +libc_hidden_proto (__clock_adjtime64);
> # endif
>
> /* Convert a known valid struct timex into a struct __timex64. */
Ok.
> diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
> index 1d8893d1b8..01ec2bfa95 100644
> --- a/sysdeps/unix/sysv/linux/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/syscalls.list
> @@ -4,7 +4,6 @@ alarm - alarm i:i alarm
> bdflush EXTRA bdflush i:ii __compat_bdflush bdflush@GLIBC_2.0:GLIBC_2.23
> capget EXTRA capget i:pp capget
> capset EXTRA capset i:pp capset
> -clock_adjtime EXTRA clock_adjtime i:ip __clock_adjtime clock_adjtime
> create_module EXTRA create_module 3 __compat_create_module create_module@GLIBC_2.0:GLIBC_2.23
> delete_module EXTRA delete_module 3 delete_module
> epoll_create EXTRA epoll_create i:i epoll_create
>
Ok.
next prev parent reply other threads:[~2020-05-19 19:07 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-08 14:56 [PATCH v2 0/7] y2038: Convert clock_adjtime related syscalls to support 64 bit time Lukasz Majewski
2020-05-08 14:56 ` [PATCH v2 1/7] y2038: linux: Provide __clock_adjtime64 implementation Lukasz Majewski
2020-05-15 10:03 ` Lukasz Majewski
2020-05-19 19:07 ` Adhemerval Zanella via Libc-alpha [this message]
2020-05-19 19:58 ` Lukasz Majewski
2020-05-08 14:56 ` [PATCH v2 2/7] y2038: linux: Provide ___adjtimex64 implementation Lukasz Majewski
2020-05-08 14:56 ` [PATCH v2 3/7] y2038: linux: Provide __adjtime64 implementation Lukasz Majewski
2020-05-08 14:56 ` [PATCH v2 4/7] y2038: Introduce struct __ntptimeval64 - new internal glibc type Lukasz Majewski
2020-05-19 19:10 ` Adhemerval Zanella via Libc-alpha
2020-05-08 14:56 ` [PATCH v2 5/7] y2038: Provide conversion helpers for struct __ntptimeval64 Lukasz Majewski
2020-05-19 19:12 ` Adhemerval Zanella via Libc-alpha
2020-05-08 14:56 ` [PATCH v2 6/7] y2038: linux: Provide __ntp_gettime64 implementation Lukasz Majewski
2020-05-19 19:18 ` Adhemerval Zanella via Libc-alpha
2020-05-19 20:20 ` Lukasz Majewski
2020-05-19 20:25 ` Adhemerval Zanella via Libc-alpha
2020-05-20 15:23 ` Joseph Myers
2020-05-20 17:08 ` Lukasz Majewski
2020-05-20 17:21 ` Joseph Myers
2020-05-21 10:31 ` Lukasz Majewski
2020-05-08 14:56 ` [PATCH v2 7/7] y2038: linux: Provide __ntp_gettimex64 implementation Lukasz Majewski
2020-05-19 19:19 ` Adhemerval Zanella via Libc-alpha
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=4450aefc-079f-d73d-5c70-ce8ed54b777b@linaro.org \
--to=libc-alpha@sourceware.org \
--cc=adhemerval.zanella@linaro.org \
--cc=alistair.francis@wdc.com \
--cc=fweimer@redhat.com \
--cc=joseph@codesourcery.com \
--cc=lukma@denx.de \
--cc=schwab@suse.de \
/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).