Re: 32-bit time_t inside itimerval

[Arnd Bergmann <arnd@arndb.de>]

On Sat, Dec 21, 2019 at 6:19 PM Alistair Francis <alistair23@gmail.com> wrote:
> On Sat, Dec 21, 2019 at 5:31 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Fri, Dec 20, 2019 at 10:35 PM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > What happened here is that originally I thought we would not need
> > setitimer/getitimer
> > and could fall back to timer_settime/timer_gettime, but that turned out to be a
> > misunderstanding (we do need both).
> >
> > By the time we introduced all the other system calls with 64-bit
> > time_t in linux-5.1,
> > there was no replacement yet, but since these interfaces never pass
> > absolute times
> > on the kernel ABI, it was considered good enough. There was a small debate on
> > whether struct itimerval and struct rusage (which has the same problem) should
> > have replacements using struct __kernel_timespec, or a newly added
> > __kernel_timeval, and that discussion never had a conclusion, so we
> > left it at this.
>
> Thanks for clarifying ths Arnd.
>
> Ok, I didn't realise this was the case. It ends up being a bit of a pain.

I'm sorry to hear that this is causing problems now. I tried hard to
get feedback on the question of whether we need the new syscalls
or not, and in the end decided not to do them, as any libc implementation
would need to do some conversion either way, and they already need
to understand about the kernel types as well.

> > For glibc, the only sensible implementation is to implement the time64
> > settimer/getitimer interfaces on top of the time32 setitimer/getitimer
> > system calls,
> > doing the conversion internally. (Same for getrusage and wait4).
>
> Ok, so we need to fix setitimer/getitimer,  getrusage and waitid's
> rusage (wait4 isn't in y2038 safe calls).

Right. To clarify about wait4/waitid (you are probably aware of this,
just pointing it out for other readers): The waitid() libc interface does not
contain a timeval, only the wait3()/wait4() functions do, and they are
implemented on top of the waitid() syscall.

> For the glibc people, can we do something like this?
>
>  1. Add a __old_timeval struct used by the itimerval and rusage structs
>  2. Make __old_timeval use __old_time_t that is always a long (no
> matter what time_t really is)

If you have linux-5.1 kernel headers, there is already __kernel_old_timeval
that is defined specifically for this purpose. Not sure if you can use those
given the state of the kernel headers overall.

> Then the question becomes do we expose __old_timeval (with 32-bit
> time_t) or the real timeval (64-bit time_t) to callers of the
> functions?

I would think this has to be the actual timeval, there is no point in
changing the API now.

> > We may still want to introduce getitimer_time64, setiitimer_timer64,
> > getrusage_time64 and waitid_time64 at some point, using __kernel_timespec
> > to have a saner user space interface, but there is no real point in glibc
> > using those syscalls as the underlying implementation when the fallback
> > to the time32 versions is still required.
>
> I would +1 adding getitimer_time64, setiitimer_timer64,
> getrusage_time64 and waitid_time64 as it simplifies things.

I have a rework of the itimer functions queued up for the kernel, after that
it should become very easy to add another set based on itimerspec.

For waitid/getrusage, a little bit of internal reorganization is still required
but shouldn't be hard to do as long as we can agree on the calling
conventions. We had a bit of discussion recently about adding new
waitid() variants that settled with adding new flags for the moment,
so adding another syscall now may take a while (the getrusage
replacement should not be an issue).

How do you expect the new syscalls to simplify things though?
My guess would be that they add complexity to the implementation
when you have to deal with converting from both the __kernel_timespec
and __kernel_old_timeval formats to the timeval format rather than
just one of the two.

       Arnd