Re: 64 bit time_t on riscv32

["Arnd Bergmann" <arnd@arndb.de>]

On Mon, Jan 15, 2024, at 21:15, Adhemerval Zanella Netto wrote:
> On 15/01/24 16:55, Adhemerval Zanella Netto wrote:
>> 
>> 
>> On 15/01/24 10:40, Florian Weimer via Libc-help wrote:
>>> * Antonios Salios via Libc-help:
>>>
>>>> According to a kernel maintainer, the __USE_TIME_BITS64 macro should be
>>>> set on architectures that use 64-bit time [2], otherwise the kernel
>>>> headers will not be able to pick the correct definition.
>>>
>>> __USE_TIME_BITS64 is an internal glibc macro.  It is not used on
>>> architectures which have a 64-bit time_t by default.
>>>
>>> Surely the UAPI headers know which time size the architecture uses in
>>> the kernel interface, and can be written arcordingly?
>> 
>> The use of a glibc internal definition on a kABI header is not really
>> a good design.  This seems to be only user so far, so I suggest to fix
>> on the kernel to not tie to a glibc internal definition. 
>> 
>> CCing Rich from musl that most likely would want to see this fixed. The
>> Android developers might be interested.
>
> So Arnd raised it was the agreement when it was added 2018 between glibc and 
> kernel headers, and we changed the deal three years later. And not sure if 
> it was on y2038 draft documentation, or on the initial patchset. Nor I
> recall the discussion where it was accorded (Arnd could help me here).  
>
> And I am not sure this is a good design, it ties glibc internal definition
> to kABI meaning that glibc won't be able to refactor this code because it
> might eventually break the ABI.  I think we will need at least to proper
> document this somewhere to avoid future breakages.

It's in the design document and was added in

https://sourceware.org/glibc/wiki/Y2038ProofnessDesign?action=diff&rev1=86&rev2=87

which was all we had to work with at the time. I understand
that some aspects of that design changed over time, but I didn't
know this one did.

> For glibc side, I think we can always define the macro so the check would
> be '__USE_TIME_BITS64 == 1' for 64 time_t support.  It would require some
> internal refactoring, but it should be doable.

I took another look at the input_event definition now, and I think
we can actually change it to remove the __USE_TIME_BITS64 check in
this one, dropping that 'timeval' reference from it, since it was
only added here to prevent compile-time errors on architectures
that use the traditional types (all 64-bit ones and 32-bit
architectures with time32):

struct input_event {              
#if (__BITS_PER_LONG != 32 || !defined(__USE_TIME_BITS64)) && !defined(__KERNEL__)
        struct timeval time;
#define input_event_sec time.tv_sec 
#define input_event_usec time.tv_usec
#else
        __kernel_ulong_t __sec;
        __kernel_ulong_t __sec;
#define input_event_sec  __sec
#define input_event_usec __usec
...
};

This is probably ok because by now most users of this structure will
have been fixed to use input_event_sec/input_event_usec instead
of time.tv_sec/time.tv_usec. Anything that has not yet been changed
will now also break on 64-bit targets.

The sound/asound.h header is much harder to change: Removing
the __USE_TIME_BITS64 check here would require using the
new-style ioctl commands everywhere, but anything built with
that new header would break when running on linux-5.5 or
earlier.

      Arnd