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

[Lukasz Majewski <lukma@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 6678 bytes --]

Hi Alistair,

> On Wed, Sep 18, 2019 at 2:28 PM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > 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.  
> 
> That's fine, I can fix up his comments and include that in my series.
> 
> >
> > 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).  
> 
> RV32 is actually both BE and LE. The spec allows it to be either. 

Ok. I was not aware of this - and blindly assumed that it is LE.

> At
> the moment there are only LE implementations, but we should try to
> handle both.

Ok. Then if you don't mind, please add the above change to your series.

> 
> Alistair
> 
> >  
> > >
> > >  #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  




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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]