Message ID | 20190507131848.30980-3-lukma@denx.de |
---|---|
State | Superseded |
Headers |
Received: (qmail 124536 invoked by alias); 7 May 2019 13:19:15 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 124473 invoked by uid 89); 7 May 2019 13:19:15 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_NUMSUBJECT, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 spammy= X-HELO: mail-out.m-online.net From: Lukasz Majewski <lukma@denx.de> To: libc-alpha@sourceware.org Cc: Stepan Golosunov <stepan@golosunov.pp.ru>, Arnd Bergmann <arnd@arndb.de>, Paul Eggert <eggert@cs.ucla.edu>, Joseph Myers <joseph@codesourcery.com>, Lukasz Majewski <lukma@denx.de> Subject: [PATCH v3 2/5] y2038: Introduce internal for glibc struct __timespec64 Date: Tue, 7 May 2019 15:18:39 +0200 Message-Id: <20190507131848.30980-3-lukma@denx.de> In-Reply-To: <20190507131848.30980-1-lukma@denx.de> References: <20190414220841.20243-1-lukma@denx.de> <20190507131848.30980-1-lukma@denx.de> |
Commit Message
Lukasz Majewski
May 7, 2019, 1:18 p.m. UTC
This type is a glibc's type similar to struct timespec but whose tv_sec field is a __time64_t rather than a time_t, which makes it Y2038-proof and usable to pass between user code and Y2038-proof kernel syscalls (e.g. clock_gettime()). To support passing this structure to the kernel - the tv_pad, 32 bit padding field has been introduced. The placement of it depends on endiannes of the SoC. Tested on x86_64 and ARM. * include/time.h: Add struct __timespec64 definition --- Changes for v3: - Replace __TIMESIZE with __WORDSIZE (as architectures with __TIMESIZE==64 will need to use this struct with 32 bit tv_nsec field). - Update in-code comment Changes for v2: - None --- include/time.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
Comments
On Mai 07 2019, Lukasz Majewski <lukma@denx.de> wrote: > +struct __timespec64 > +{ > + __time64_t tv_sec; /* Seconds */ > +# if BYTE_ORDER == BIG_ENDIAN > + int tv_pad: 32; /* Padding named for checking/setting */ > + __int32_t tv_nsec; /* Nanoseconds */ > +# else > + __int32_t tv_nsec; /* Nanoseconds */ > + int tv_pad: 32; /* Padding named for checking/setting */ > +# endif No need to use a bitfield, since the padding is not fractional. Andreas.
Hi Andreas, > On Mai 07 2019, Lukasz Majewski <lukma@denx.de> wrote: > > > +struct __timespec64 > > +{ > > + __time64_t tv_sec; /* Seconds */ > > +# if BYTE_ORDER == BIG_ENDIAN > > + int tv_pad: 32; /* Padding named for checking/setting > > */ > > + __int32_t tv_nsec; /* Nanoseconds */ > > +# else > > + __int32_t tv_nsec; /* Nanoseconds */ > > + int tv_pad: 32; /* Padding named for checking/setting > > */ +# endif > > No need to use a bitfield, since the padding is not fractional. That bitfield is for following reasons: 1. Have a backup plan in the case if we need to copy and clear the padding anyway before passing this structure to the kernel in the future (as recently we discovered that for example x32 has a kernel bug with clearing it). 2. Kernel syscalls (e.g. clock_settime64) expects on those systems two values - each 8 bytes. To avoid any nasty surprises the explicit padding was added. > > Andreas. > Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Mai 07 2019, Lukasz Majewski <lukma@denx.de> wrote: > Hi Andreas, > >> On Mai 07 2019, Lukasz Majewski <lukma@denx.de> wrote: >> >> > +struct __timespec64 >> > +{ >> > + __time64_t tv_sec; /* Seconds */ >> > +# if BYTE_ORDER == BIG_ENDIAN >> > + int tv_pad: 32; /* Padding named for checking/setting >> > */ >> > + __int32_t tv_nsec; /* Nanoseconds */ >> > +# else >> > + __int32_t tv_nsec; /* Nanoseconds */ >> > + int tv_pad: 32; /* Padding named for checking/setting >> > */ +# endif >> >> No need to use a bitfield, since the padding is not fractional. > > That bitfield is for following reasons: > > 1. Have a backup plan in the case if we need to copy and clear the > padding anyway before passing this structure to the kernel in the future > (as recently we discovered that for example x32 has a kernel bug with > clearing it). > > 2. Kernel syscalls (e.g. clock_settime64) expects on those systems two > values - each 8 bytes. To avoid any nasty surprises the explicit > padding was added. None of that requires a bitfield. Andreas.
Hi Andreas, > On Mai 07 2019, Lukasz Majewski <lukma@denx.de> wrote: > > > Hi Andreas, > > > >> On Mai 07 2019, Lukasz Majewski <lukma@denx.de> wrote: > >> > >> > +struct __timespec64 > >> > +{ > >> > + __time64_t tv_sec; /* Seconds */ > >> > +# if BYTE_ORDER == BIG_ENDIAN > >> > + int tv_pad: 32; /* Padding named for > >> > checking/setting */ > >> > + __int32_t tv_nsec; /* Nanoseconds */ > >> > +# else > >> > + __int32_t tv_nsec; /* Nanoseconds */ > >> > + int tv_pad: 32; /* Padding named for > >> > checking/setting */ +# endif > >> > >> No need to use a bitfield, since the padding is not fractional. > > > > That bitfield is for following reasons: > > > > 1. Have a backup plan in the case if we need to copy and clear the > > padding anyway before passing this structure to the kernel in the > > future (as recently we discovered that for example x32 has a kernel > > bug with clearing it). > > > > 2. Kernel syscalls (e.g. clock_settime64) expects on those systems > > two values - each 8 bytes. To avoid any nasty surprises the explicit > > padding was added. > > None of that requires a bitfield. For 32 bit systems with 64 bit time_t we would have following situation: struct timespec: -> tv_sec (8 B) -> tv_nsec (4 B -> must be long as of POSIX) [*] This would have 16B size (sizeof() would return 16B [1]) My point is if we do need at any point in time to clear this padding (due to e.g. kernel bug when passing data to syscalls [2] or solve issue as presented in [*]). Note: [*] - the x32 ABI of x86_64 has long 8B and there are some issues with POSIX compliance https://sourceware.org/bugzilla/show_bug.cgi?id=16437 [1] - as discussed: http://www.catb.org/esr/structure-packing/#_structure_alignment_and_padding [2] - https://lore.kernel.org/lkml/20190429131951.471701-1-arnd@arndb.de/ > > Andreas. > Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Mai 08 2019, Lukasz Majewski <lukma@denx.de> wrote: > My point is if we do need at any point in time to clear this padding > (due to e.g. kernel bug when passing data to syscalls [2] or solve issue > as presented in [*]). Yes, but you don't need to use a *bitfield*. Andreas.
On Wed, 08 May 2019 10:38:20 +0200 Andreas Schwab <schwab@suse.de> wrote: > On Mai 08 2019, Lukasz Majewski <lukma@denx.de> wrote: > > > My point is if we do need at any point in time to clear this padding > > (due to e.g. kernel bug when passing data to syscalls [2] or solve > > issue as presented in [*]). > > Yes, but you don't need to use a *bitfield*. Could you share your opinion about the type to use to replace the bitfield? The other option would be to use __int32_t tv_pad. Another question - shall we use __int32_t for tv_nsec or __syscall_slong_t for internal representation? We would end up with the same type in the end, but I don't know what is preferable. > > Andreas. > Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Mai 08 2019, Lukasz Majewski <lukma@denx.de> wrote: > On Wed, 08 May 2019 10:38:20 +0200 > Andreas Schwab <schwab@suse.de> wrote: > >> On Mai 08 2019, Lukasz Majewski <lukma@denx.de> wrote: >> >> > My point is if we do need at any point in time to clear this padding >> > (due to e.g. kernel bug when passing data to syscalls [2] or solve >> > issue as presented in [*]). >> >> Yes, but you don't need to use a *bitfield*. > > Could you share your opinion about the type to use to replace the > bitfield? Anything that fits, but not a bitfield. Andreas.
On Wed, 08 May 2019 11:12:27 +0200 Andreas Schwab <schwab@suse.de> wrote: > On Mai 08 2019, Lukasz Majewski <lukma@denx.de> wrote: > > > On Wed, 08 May 2019 10:38:20 +0200 > > Andreas Schwab <schwab@suse.de> wrote: > > > >> On Mai 08 2019, Lukasz Majewski <lukma@denx.de> wrote: > >> > >> > My point is if we do need at any point in time to clear this > >> > padding (due to e.g. kernel bug when passing data to syscalls > >> > [2] or solve issue as presented in [*]). > >> > >> Yes, but you don't need to use a *bitfield*. > > > > Could you share your opinion about the type to use to replace the > > bitfield? > > Anything that fits, but not a bitfield. Ok :-) > > Andreas. > Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff --git a/include/time.h b/include/time.h index ac3163c2a5..814e927645 100644 --- a/include/time.h +++ b/include/time.h @@ -5,6 +5,7 @@ # include <bits/types/locale_t.h> # include <stdbool.h> # include <time/mktime-internal.h> +# include <endian.h> extern __typeof (strftime_l) __strftime_l; libc_hidden_proto (__strftime_l) @@ -51,6 +52,30 @@ extern void __tzset_parse_tz (const char *tz) attribute_hidden; extern void __tz_compute (__time64_t timer, struct tm *tm, int use_localtime) __THROW attribute_hidden; +#if __WORDSIZE == 64 +# define __timespec64 timespec +#else +/* The glibc Y2038-proof struct __timespec64 structure for a time value. + To keep things Posix-ish, we keep the nanoseconds field a 32-bit + signed long, but since the Linux field is a 64-bit signed int, we + pad our tv_nsec with a 32-bit bitfield. + + As a general rule the Linux kernel is ignoring upper 32 bits of + tv_nsec field. However, there are architectures with potential need + for clearing the padding (i.e. 'x32'). */ +struct __timespec64 +{ + __time64_t tv_sec; /* Seconds */ +# if BYTE_ORDER == BIG_ENDIAN + int tv_pad: 32; /* Padding named for checking/setting */ + __int32_t tv_nsec; /* Nanoseconds */ +# else + __int32_t tv_nsec; /* Nanoseconds */ + int tv_pad: 32; /* Padding named for checking/setting */ +# endif +}; +#endif + #if __TIMESIZE == 64 # define __ctime64 ctime #else