Re: Accelerating Y2038 glibc fixes

[Lukasz Majewski <lukma at denx dot de>]

Hi Zack,

> Hi Zack,
> 
> > On Fri, Jul 26, 2019 at 6:39 AM Lukasz Majewski <lukma@denx.de>
> > wrote: ...  
> > > > > See for example [1] - there are just 7 lines of "code".  But
> > > > > Joseph does not accept our patches.  The arguments he gives
> > > > > are not on a technical level;    
> > ...  
> > > Our goal is to add a solid foundation for the Y2038 work, so we
> > > would know the direction where we are heading.    
> > ...  
> > > If you think that it would be better and most of all faster if you
> > > rewrite the description, then I don't mind.
> > >
> > > It would be great if you could do it sooner than latter as this
> > > slows down our development.    
> > ...  
> > > The most recent version of this patch set (v8):
> > > https://patchwork.ozlabs.org/patch/1117100/    
> > 
> > I haven’t been following the details of these patches super
> > carefully, and I’m not sure I understand what _Joseph’s_ concerns
> > with your writing is.  However, I’m a native English speaker, I’ve
> > read over the text in the patch at
> > <https://patchwork.ozlabs.org/patch/1117100/>, I do think I
> > understand the issues at a high level, and I do think the meaning
> > of __ASSUME_TIME64_SYSCALLS could be explained more clearly. I’m
> > prepared to work with you to come up with better wording   
> 
> Thanks for offering your help.

Shall I provide more input to this issue?

> 
> > but I
> > need to ask you a bunch of questions.  Could you please reply to
> > each of the queries marked Qn below?
> > 
> > As I understand it, we have five distinct cases to consider:
> > 
> > 1. All future LP64 ABIs and all but one existing LP64 ABI,
> > identified by __WORDSIZE == 64: time_t is already a 64-bit integer
> > type and all of the relevant system calls already accept it in that
> > form. glibc’s implementation of, for instance, clock_gettime may
> > continue to invoke the system call numbered __NR_clock_gettime.  
> 
> This is exactly how we shall proceed with machines having
> __WORDSIZE==64 (e.g. x86_64, armv8, etc).
> 
> They now support 64 bit time with non suffixed syscalls.
> 
> In the patch [1] the __WORDSIZE == 64 check covers this.
> 
> > 
> > 2. The exception to (1) is Alpha.  That is an exclusively LP64
> >    architecture, but in glibc 2.0 it used 32-bit time_t, and we
> > still have compatibility code for that case.  The compatibility
> > symbols invoke a set of compatibility syscalls with ‘osf’ in their
> > names: for instance, gettimeofday@GLIBC_2.0 invokes
> > __NR_osf_gettimeofday. Not all of the time-related functions in
> > glibc have compatibility symbols, only those that existed in
> > version 2.0.
> > 
> >    Your patches do not touch this compatibility code at all, as far
> > as I can see.   
> 
> Yes, you are correct. I was not even aware of such a case (as I found
> Alpha as 64 bit arch when I did my checking).
> 
> > Alpha being out of production, and binaries compiled
> >    against glibc 2.0 being rare anyway, it would only make sense to
> >    involve this code in your patches if it reduced the overall
> > volume of compatibility code somehow, but regardless we need to
> > make sure it doesn't break.  
> 
> As you mentioned - we shall not break existing binaries. However, I'm
> not sure if we shall spent more time/effort on the arch being near EOL
> (or at least being out of production now).
> 
> > 
> > 3. x32 (recently new 32-bit ABI for x86): like case 1, time_t is
> >    already 64-bit and we use unsuffixed system calls.  The text says
> >    this case is identified by __WORDSIZE == 32 && __TIMESIZE == 64,
> >    but the code actually checks __SYSCALL_WORDSIZE.
> > 
> >    Q1: Which condition correctly identifies this case, __TIMESIZE ==
> > 64 or __SYSCALL_WORDSIZE == 64?  
> 
> It is:
> 
> (__WORDSIZE == 32) && ((defined __SYSCALL_WORDSIZE
> &&__SYSCALL_WORDSIZE == 64))
> 
> Only x32 defines the __SYSCALL_WORDSIZE == 64 (as it has __WORDSIZE ==
> 32, but supports 64 bit syscalls ABI).
> 
> > 
> >    Q2: Could we perhaps ensure that __TIMESIZE and/or
> > __SYSCALL_WORDSIZE is defined to 64 whenever __WORDSIZE == 64? Then
> > we could collapse this into case 1.  
> 
> __TIMESIZE == 64 for x32. 
> 
> The x32 uses the same set of syscalls (e.g. clock_gettime) as in
> point 1 (as for example x86_64).
> 
> > 
> > 4. Brand-new (added in kernel 5.1 or later) 32-bit ABIs: time_t will
> >    always be 64-bit,  
> 
> This would be true after we make the "switch" to support Y2038 aware
> systems. Please find example implementation [2] from this patch series
> [3] (it adds example code for converting __clock_settime to support 64
> bit time when __ASSUME_TIME64_SYSCALLS is defined).
> 
> > _but_ glibc’s implementation of time-related APIs
> >    may need to invoke system calls with suffixed names:
> > clock_gettime invoking __NR_clock_gettime64, for instance.  Also,
> > the kernel will not provide all of the time-related system calls
> > that have historically existed; glibc must, for instance, implement
> >    gettimeofday in terms of clock_gettime.  
> 
> Yes, correct. Some syscalls would be emulated (as they are not or will
> not be converted to 64 bit version).
> 
> > 
> >    Q3: What macros are defined for this case?  
> 
> There are no macros yet available.
> 
> > 
> >    Q4: Does glibc need to call system calls with suffixed names in
> >    this case?  
> 
> I think yes - for example the gettimeofday would internally use
> clock_gettime64 (vDSO if available).
> 
> > 
> >    Q4a: If the answer to Q4 is ‘yes’, why is that, and can we change
> >    the kernel so that it’s the same as x32 and the LP64
> > architectures?  
> 
> We need new set of syscalls for 64 bit time support on 32 bit archs
> (WORDSIZE==32); for example x32/LP64 would still use clock_settime
> syscall (number X). To have the same functionality (64 bit time
> support) 32 bit archs would need to use clock_settime64 (number 404 on
> armv7)
> 
> And here the __ASSUME_TIME64_SYSCALLS comes into play. If the arch is
> capable of providing 64 bit time, (no matter if it uses clock_settime
> or clock_settime64), then __ASSUME_TIME64_SYSCALLS is defined.
> 
> It is also assumed that both clock_settime64 and clock_settime provide
> the same ABI, so no code needs to be adjusted in glibc.
> 
> If code needs to be adjusted (as the calls are not compatible) - a new
> flag will be introduced (like with semtimedop)
> 
> >    (Either by removing the suffixes, or by _adding_ suffixed aliases
> >    to asm/unistd.h for x32 and LP64 architectures.)  
> 
> Wouldn't this caused the ABI break?
> 
> > 
> > 5. Historical 32-bit ABIs, where the existing set of system calls
> >    takes 32-bit time_t, and Linux 5.1 added a matching set that
> > takes 64-bit time_t.  For compatibility with old programs that make
> >    direct system calls, the kernel will not rename the __NR_
> > constants for the old (32-bit) system calls; instead new constants
> > with ‘64’ or ‘time64’ suffixes will be added.  As in case 4, the
> > new set of system calls does not cover all of the historic
> > time-related system calls.
> > 
> >    In this case, and only this case, glibc’s code needs to account
> > for the possibility that the new __NR_ constants are not defined
> >    (because glibc is being compiled against kernel headers from a
> >    version older than 5.1), or that the new system calls are not
> >    available at runtime (glibc was compiled against new kernel
> > headers but is running with an old kernel).
> > 
> >    The #if is complicated enough that I’m not sure, but I _think_
> > your code only defines __ASSUME_TIME64_SYSCALLS when the new
> > constants are _guaranteed_ to be defined.
> > 
> >    Q5: Is it correct that __ASSUME_TIME64_SYSCALLS is only defined
> >    when the new constants are guaranteed to be defined?  
> 
> No.
> 
> The __ASSUME_TIME64_SYSCALLS is defined only when the architecture
> supports 64 bit time related ABI.
> 
> (either via clock_settime on e.g. x86_64/arm64 or clock_settime64 on
> arm).
> 
> Please consult the code for clock_settime [4]. It shows how the
> __ASSUME_TIME64_SYSCALLS flag is used in practice.
> 
> > 
> >    Q6: All of the other __ASSUME_ constants mean both that we assume
> >    the kernel headers are new enough to provide all the necessary
> >    declarations, and that we assume the feature is available at
> >    runtime: no fallback code will be included in the library.  Is
> > this also the intended meaning of __ASSUME_TIME64_SYSCALLS?  
> 
> The patch [1] defines the __ASSUME_TIME64_SYSCALLS as the ability of
> the architecture (via kernel syscalls) to provide 64 bit time support.
> 
> As shown in [4] - the fallback code is called when
> __ASSUME_TIME64_SYSCALLS is NOT defined and if architecture doesn't
> support clock_settime64.
> 
> > 
> > zw  
> 
> 
> Note:
> 
> [1] - https://patchwork.ozlabs.org/patch/1117100/ 
> 
> [2] -
> https://github.com/lmajewski/y2038_glibc/commit/3d5f3512438de7426acba58c1edf53f756f8570b#diff-c051022b496f12bd9028edb46b8ec04d
> 
> [3] -
> https://github.com/lmajewski/y2038_glibc/commits/Y2038-2.29-glibc-__clock-internal-struct-timespec-v6
> 
> [4] -
> https://github.com/lmajewski/y2038_glibc/commit/69f842a8519ca13ed11fab0ff1bcc6fa1a524192
> 
> 
> 
> 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: pgpyXOVFJfzAW.pgp
Description: OpenPGP digital signature