This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v7 0/3] y2038: Linux: Introduce __clock_settime64 function


Hi Alistair,

> On Tue, Sep 17, 2019 at 9:51 AM Joseph Myers
> <joseph@codesourcery.com> wrote:
> >
> > On Tue, 17 Sep 2019, Lukasz Majewski wrote:
> >  
> > > - New 32 bits glibc ports (like RISC-V 32) will get __TIMESIZE ==
> > >   64 (__WORDSIZE == 32) and no need to define the -D_TIME_BITS=64
> > >   during the compilation. They will just get 64 bit time API
> > > support from the outset.  
> >
> > Yes, at least if such ports wish to use 64-bit time; I don't think
> > we've really discussed if we want to *require* 64-bit time for
> > future ports (e.g. the next revised resubmissions of the ARC and
> > NDS32 ports). Certainly the work required right now for ARC or
> > NDS32 to use 64-bit time would be significantly more than the work
> > for RV32 (because they also support older kernel versions without
> > the 64-bit-time syscalls, so all the Y2038 work for fallback at
> > runtime to older syscalls becomes relevant), unless they decide on
> > 5.1 or later as minimum kernel version. 
> > > - Already supported 32 bits architectures (like armv7-a with
> > > __WORDSIZE == 32) will keep __TIMESIZE == 32 and require
> > > -D_TIME_BITS=64 for compilation.  
> >
> > Yes.
> >  
> > >   After glibc sets the minimal supported kernel version to 5.1
> > > and all conversions for syscalls to support 64 bit time API are
> > > done the __TIMESIZE will be set to 64 and -D_TIME_BITS=64 will
> > > not be required anymore for compilation.  
> >
> > No.  __TIMESIZE means the size of time_t in the unsuffixed ABIs in
> > glibc, not the _TIME_BITS-dependent size of time_t in the current
> > compilation. We hope in future to make _TIME_BITS=64 the default
> > and only API supported for new compilations (which is independent
> > of what the minimum kernel version is), but __TIMESIZE would still
> > be 32, because the unsuffixed ABIs would remain compatible with
> > existing binaries using 32-bit time. 
> > > Ok. So then we shall keep the condition:
> > >
> > > #if __WORDSIZE == 64 \
> > >   || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > > # define __timespec64 timespec
> > > #else  
> >
> > No.  __timespec64 should be defined to timespec whenever __TIMESIZE
> > == 64. The timespec to which it is defined, in the public header,
> > would gain padding.
> >
> > The condition
> >
> > #if __WORDSIZE == 64 \
> >   || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> >
> > is correct as a condition for struct timespec (in the public
> > header) *not* to have padding.  
> 
> Are you going to incorporate this into your series Lukasz?
> 
> I currently have this diff which fixes the build failures for me:
> 
> diff --git a/include/time.h b/include/time.h
> index 7ed3aa61d1d..91f6280eb4d 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -50,8 +50,7 @@ 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 \
> -  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> +#if __TIMESIZE == 64

I've prepared v8 of __clock_settime64 conversion patches with the above
change:
https://patchwork.ozlabs.org/cover/1164209/

I've tested it with meta-y2038:
https://github.com/lmajewski/meta-y2038

 as well as
../src/scripts/build-many-glibcs.py

It seems to work as expected.	

>  # define __timespec64 timespec
>  #else
>  /* The glibc Y2038-proof struct __timespec64 structure for a time
> value. diff --git a/time/bits/types/struct_timespec.h
> b/time/bits/types/struct_timespec.h
> index 5b77c52b4f0..48405c4f08a 100644
> --- a/time/bits/types/struct_timespec.h
> +++ b/time/bits/types/struct_timespec.h
> @@ -3,13 +3,25 @@
>  #define _STRUCT_TIMESPEC 1
> 
>  #include <bits/types.h>
> +#include <endian.h>
> 
>  /* POSIX.1b structure for a time value.  This is like a `struct
> timeval' but has nanoseconds instead of microseconds.  */
>  struct timespec
>  {
>    __time_t tv_sec;             /* Seconds.  */
> +#if __WORDSIZE == 64 \
> +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
>    __syscall_slong_t tv_nsec;   /* Nanoseconds.  */
> +#else
> +# if BYTE_ORDER == BIG_ENDIAN
> +  __int32_t tv_pad;           /* Padding */
> +  __syscall_slong_t tv_nsec;  /* Nanoseconds */
> +# else
> +  __int32_t tv_nsec;          /* Nanoseconds */
> +  __syscall_slong_t tv_pad;   /* Padding */
> +# endif
> +#endif
>  };

I did not incorporated the above change to v8 of __clock_settime64 as
there are some issues raised by Joseph.

Last but not least - we can get away with the above change as the
implicit padding works for RV32, and ARM32 (which both are LE).

> 
>  #endif
> 
> As well as that the timeval struct has the same issue. I'll have to
> look into that and see what the solution is there.
> 
> Alistair
> 
> >
> > --
> > Joseph S. Myers
> > joseph@codesourcery.com  




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

Attachment: pgp6vhLiRckwH.pgp
Description: OpenPGP digital signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]