Re: 32-bit time_t inside itimerval

[Alistair Francis <alistair23 at gmail dot com>]

On Mon, Dec 30, 2019 at 2:02 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> 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.

No worries, I understand. Now that I have gotten my head around them
it actually isn't too bad.

>
> > > 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.

Yep!

>
> > 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.

Yeah, agreed. I have updated the RV32 port to internally convert
between 32/64-bit.

>
> > > 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.

Yeah, it might not simplify anything. My thinking was that having
time_t be 64-bit everywhere would be simpler, but converting these
syscalls isn't as painful as I first thought so it currently isn't
really a problem.

Alistair

>
>        Arnd