Hi Zack, > On Fri, Jul 26, 2019 at 6:39 AM Lukasz Majewski 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 , 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. > 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