32-bit time_t inside itimerval

32-bit time_t inside itimerval

[Alistair Francis]

Hey,

I just noticed something strange.

The setitimer syscall is not different for 32/64-bit time_t. So we use
this syscall for both 32/64 time_t [1]

SYSCALL_DEFINE3(setitimer, int, which, struct itimerval __user *, value,
        struct itimerval __user *, ovalue)

Where struct itimerval __user *, value looks like this [2]:

struct itimerval {
    struct timeval it_interval;    /* timer interval */
    struct timeval it_value;    /* current value */
};

Which then uses this structure for timeval [3]:

struct timeval {
    __kernel_old_time_t    tv_sec;        /* seconds */
    __kernel_suseconds_t    tv_usec;    /* microseconds */
};

And __kernel_old_time_t is defined [4] as:

typedef __kernel_long_t    __kernel_old_time_t;
typedef __kernel_long_t    __kernel_time_t;


So __kernel_old_time_t and __kernel_time_t are both 32-bit values
inside the kernel. and the setiimer syscall expects a 32-bit time_t
for the values inside it.

This is different to the current glibc implementation where all time_t
values are treated as 64-bit for RISC-V [5].

What should we do here?

1. Change glibc to use a 32-bit time_t for some structures, such as
the timeval inside itimerval?
2. Convert the kernel time_t to be 64-bit for RV32?

Alistair

1: https://github.com/torvalds/linux/blob/ceb307474506f888e8f16dab183405ff01dffa08/kernel/time/itimer.c#L336
2: https://github.com/torvalds/linux/blob/b01d7cb41ff51b7779977de601a984406e2a5ba9/include/uapi/linux/time.h#L39
3: https://github.com/torvalds/linux/blob/b01d7cb41ff51b7779977de601a984406e2a5ba9/include/uapi/linux/time.h#L16
4: https://github.com/torvalds/linux/blob/b01d7cb41ff51b7779977de601a984406e2a5ba9/include/uapi/asm-generic/posix_types.h#L89
5: https://github.com/alistair23/glibc/commit/71faeb322c4f4a163d138893b6855920dad45e07

Re: 32-bit time_t inside itimerval

[Arnd Bergmann]

On Fri, Dec 20, 2019 at 10:35 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> Hey,
>
> I just noticed something strange.
>
> The setitimer syscall is not different for 32/64-bit time_t. So we use
> this syscall for both 32/64 time_t [1]
>
> SYSCALL_DEFINE3(setitimer, int, which, struct itimerval __user *, value,
>         struct itimerval __user *, ovalue)
>
> Where struct itimerval __user *, value looks like this [2]:
>
> struct itimerval {
>     struct timeval it_interval;    /* timer interval */
>     struct timeval it_value;    /* current value */
> };
>
> Which then uses this structure for timeval [3]:
>
> struct timeval {
>     __kernel_old_time_t    tv_sec;        /* seconds */
>     __kernel_suseconds_t    tv_usec;    /* microseconds */
> };
>
> And __kernel_old_time_t is defined [4] as:
>
> typedef __kernel_long_t    __kernel_old_time_t;
> typedef __kernel_long_t    __kernel_time_t;

Right, with my latest patches, this gets changed to __kernel_old_timeval, with
an unmodified definititon.

> So __kernel_old_time_t and __kernel_time_t are both 32-bit values
> inside the kernel. and the setiimer syscall expects a 32-bit time_t
> for the values inside it.
>
> This is different to the current glibc implementation where all time_t
> values are treated as 64-bit for RISC-V [5].
>
> What should we do here?
>
> 1. Change glibc to use a 32-bit time_t for some structures, such as
> the timeval inside itimerval?
> 2. Convert the kernel time_t to be 64-bit for RV32?

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.

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

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.

        Arnd

Re: 32-bit time_t inside itimerval

[Alistair Francis]

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:
> >
> > Hey,
> >
> > I just noticed something strange.
> >
> > The setitimer syscall is not different for 32/64-bit time_t. So we use
> > this syscall for both 32/64 time_t [1]
> >
> > SYSCALL_DEFINE3(setitimer, int, which, struct itimerval __user *, value,
> >         struct itimerval __user *, ovalue)
> >
> > Where struct itimerval __user *, value looks like this [2]:
> >
> > struct itimerval {
> >     struct timeval it_interval;    /* timer interval */
> >     struct timeval it_value;    /* current value */
> > };
> >
> > Which then uses this structure for timeval [3]:
> >
> > struct timeval {
> >     __kernel_old_time_t    tv_sec;        /* seconds */
> >     __kernel_suseconds_t    tv_usec;    /* microseconds */
> > };
> >
> > And __kernel_old_time_t is defined [4] as:
> >
> > typedef __kernel_long_t    __kernel_old_time_t;
> > typedef __kernel_long_t    __kernel_time_t;
>
> Right, with my latest patches, this gets changed to __kernel_old_timeval, with
> an unmodified definititon.
>
> > So __kernel_old_time_t and __kernel_time_t are both 32-bit values
> > inside the kernel. and the setiimer syscall expects a 32-bit time_t
> > for the values inside it.
> >
> > This is different to the current glibc implementation where all time_t
> > values are treated as 64-bit for RISC-V [5].
> >
> > What should we do here?
> >
> > 1. Change glibc to use a 32-bit time_t for some structures, such as
> > the timeval inside itimerval?
> > 2. Convert the kernel time_t to be 64-bit for RV32?
>
> 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.

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

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)

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?

If we are exposing the new one we will have to edit the four functions
to convert 32-bit to 64-bit. Which should all be pretty straight
forward.
If we don't expose the new one it should be even easier.

I'm not sure what is more appropriate for glibc, let me know and I
should be able to get some patches ready. Maybe this can make it in
before freeze :)

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

Alistair

>
>         Arnd

Re: 32-bit time_t inside itimerval

[Arnd Bergmann]

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

Re: 32-bit time_t inside itimerval

[Alistair Francis]

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

Re: 32-bit time_t inside itimerval

[Arnd Bergmann]

On Mon, Dec 30, 2019 at 8:57 PM Alistair Francis <alistair23@gmail.com> wrote:
> 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:
> > > 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.

Any chance of making this the default implementation for 32-bit
rather than RV32 specific? The code should be the same for any
time64 user space regardless of the architecture.

       Arnd

Re: 32-bit time_t inside itimerval

[Alistair Francis]

On Mon, Dec 30, 2019 at 12:11 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Dec 30, 2019 at 8:57 PM Alistair Francis <alistair23@gmail.com> wrote:
> > 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:
> > > > 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.
>
> Any chance of making this the default implementation for 32-bit
> rather than RV32 specific? The code should be the same for any
> time64 user space regardless of the architecture.

I was thinking about this.

I don't know of a good way to make it apply only to 32-bit archs with
a 64-bit time_t.This could actually just apply to all 32-bit archs.
That way ones that are 64-bit time_t are covered and ones that aren't
will just do an extra conversion with no effect.

Making it 32-bit in general seems reasonable to me, I just have to
figure out a way to test it though.

Alistair

>
>        Arnd

Re: 32-bit time_t inside itimerval

[Arnd Bergmann]

On Mon, Dec 30, 2019 at 10:22 PM Alistair Francis <alistair23@gmail.com> wrote:
> On Mon, Dec 30, 2019 at 12:11 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Mon, Dec 30, 2019 at 8:57 PM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > Yeah, agreed. I have updated the RV32 port to internally convert
> > > between 32/64-bit.
> >
> > Any chance of making this the default implementation for 32-bit
> > rather than RV32 specific? The code should be the same for any
> > time64 user space regardless of the architecture.
>
> I was thinking about this.
>
> I don't know of a good way to make it apply only to 32-bit archs with
> a 64-bit time_t.This could actually just apply to all 32-bit archs.
> That way ones that are 64-bit time_t are covered and ones that aren't
> will just do an extra conversion with no effect.
>
> Making it 32-bit in general seems reasonable to me, I just have to
> figure out a way to test it though.

Maybe Lukasz already has a plan for this, I don't think it's
fundamentally different from the other system calls that he has
converted already to work with time64 callers.

      Arnd

Re: 32-bit time_t inside itimerval

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2666 bytes --]

Hi Arnd, Alistair,

Sorry to jump in lately - Christmas period :-)

> On Mon, Dec 30, 2019 at 10:22 PM Alistair Francis
> <alistair23@gmail.com> wrote:
> > On Mon, Dec 30, 2019 at 12:11 PM Arnd Bergmann <arnd@arndb.de>
> > wrote:  
> > > On Mon, Dec 30, 2019 at 8:57 PM Alistair Francis
> > > <alistair23@gmail.com> wrote:  
> > > >
> > > > Yeah, agreed. I have updated the RV32 port to internally convert
> > > > between 32/64-bit.  
> > >
> > > Any chance of making this the default implementation for 32-bit
> > > rather than RV32 specific? The code should be the same for any
> > > time64 user space regardless of the architecture.  
> >
> > I was thinking about this.
> >
> > I don't know of a good way to make it apply only to 32-bit archs
> > with a 64-bit time_t.This could actually just apply to all 32-bit
> > archs. That way ones that are 64-bit time_t are covered and ones
> > that aren't will just do an extra conversion with no effect.
> >
> > Making it 32-bit in general seems reasonable to me, I just have to
> > figure out a way to test it though.  
> 
> Maybe Lukasz already has a plan for this,

I did not do any work regarding {set|get}itimer. I've instead focused on
timer_fd[sg]ettime for now [1].

> I don't think it's
> fundamentally different from the other system calls that he has
> converted already to work with time64 callers.

I'm not aware of any RV32 specifics, but it seems to me that it would
be appropriate to use the 64 bit version of struct __itimerspec64 in
glibc - as for example in the conversion patch from [1].

As it was already mentioned - those calls set the time to be
decremented and do not operate on "absolute" time values.
Hence, I think that it would be good enough (for now?) to use 32 bit
API wrapped into 64 bit internal glibc values and just return errors
when somebody wants to set timer relative expiration time to overflow
time_t on 32 bit archs (arm,rv32).

(The above approach would also simplify latter glibc switch to *_time64
syscalls when introduced).

> 
>       Arnd

Arnd, am I correct that the struct itimerval to __kernel_old_itimerval
conversion patch can be found here [2]? 


Links:

[1] - https://patchwork.ozlabs.org/patch/1211482/
[2] -
https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/commit/?h=y2038&id=4f9fbd893fe83a1193adceca41c8f7aa6c7382a1


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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: 32-bit time_t inside itimerval

[Arnd Bergmann]

On Thu, Jan 2, 2020 at 1:08 PM Lukasz Majewski <lukma@denx.de> wrote:
> > On Mon, Dec 30, 2019 at 10:22 PM Alistair Francis
> > <alistair23@gmail.com> wrote:
> > > On Mon, Dec 30, 2019 at 12:11 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > I don't think it's
> > fundamentally different from the other system calls that he has
> > converted already to work with time64 callers.
>
> I'm not aware of any RV32 specifics, but it seems to me that it would
> be appropriate to use the 64 bit version of struct __itimerspec64 in
> glibc - as for example in the conversion patch from [1].

What I mean is that rv32 otherwise does not convert between time32
and time64 interfaces because it always uses the time64 version,
so unlike the others, there is probably no helper to convert between
the timeval formats either.

> As it was already mentioned - those calls set the time to be
> decremented and do not operate on "absolute" time values.
> Hence, I think that it would be good enough (for now?) to use 32 bit
> API wrapped into 64 bit internal glibc values and just return errors
> when somebody wants to set timer relative expiration time to overflow
> time_t on 32 bit archs (arm,rv32).

Yes, that's the idea. The kernel already limits the range to 64-bit
nanoseconds because of its timer implementation, so truncating it
to 32-bit seconds does not change the behavior either.

> Arnd, am I correct that the struct itimerval to __kernel_old_itimerval
> conversion patch can be found here [2]?

Yes, that's right. This patch only changes the in-kernel implementation
as a step to removing the timeval definition from the kernel's uapi
headers, it does not change the behavior at all.

       Arnd

Re: 32-bit time_t inside itimerval

[Alistair Francis]

On Thu, Jan 2, 2020 at 4:28 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Jan 2, 2020 at 1:08 PM Lukasz Majewski <lukma@denx.de> wrote:
> > > On Mon, Dec 30, 2019 at 10:22 PM Alistair Francis
> > > <alistair23@gmail.com> wrote:
> > > > On Mon, Dec 30, 2019 at 12:11 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > I don't think it's
> > > fundamentally different from the other system calls that he has
> > > converted already to work with time64 callers.
> >
> > I'm not aware of any RV32 specifics, but it seems to me that it would
> > be appropriate to use the 64 bit version of struct __itimerspec64 in
> > glibc - as for example in the conversion patch from [1].
>
> What I mean is that rv32 otherwise does not convert between time32
> and time64 interfaces because it always uses the time64 version,
> so unlike the others, there is probably no helper to convert between
> the timeval formats either.

I have some patches prepared that will convert a 64-bit time_t to
32-bit for the required syscalls. It's generic for 32-bit archs, but
will only apply when __TIMESIZE == 64.

I'll send an RFC out with the RV32 patches soon and then send patches
when the 2.32 merge window opens up.


Alistair

>
> > As it was already mentioned - those calls set the time to be
> > decremented and do not operate on "absolute" time values.
> > Hence, I think that it would be good enough (for now?) to use 32 bit
> > API wrapped into 64 bit internal glibc values and just return errors
> > when somebody wants to set timer relative expiration time to overflow
> > time_t on 32 bit archs (arm,rv32).
>
> Yes, that's the idea. The kernel already limits the range to 64-bit
> nanoseconds because of its timer implementation, so truncating it
> to 32-bit seconds does not change the behavior either.
>
> > Arnd, am I correct that the struct itimerval to __kernel_old_itimerval
> > conversion patch can be found here [2]?
>
> Yes, that's right. This patch only changes the in-kernel implementation
> as a step to removing the timeval definition from the kernel's uapi
> headers, it does not change the behavior at all.
>
>        Arnd

Re: 32-bit time_t inside itimerval

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2622 bytes --]

Hi Alistair,

> On Thu, Jan 2, 2020 at 4:28 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Thu, Jan 2, 2020 at 1:08 PM Lukasz Majewski <lukma@denx.de>
> > wrote:  
> > > > On Mon, Dec 30, 2019 at 10:22 PM Alistair Francis
> > > > <alistair23@gmail.com> wrote:  
> > > > > On Mon, Dec 30, 2019 at 12:11 PM Arnd Bergmann
> > > > > <arnd@arndb.de> wrote:  
> > > > I don't think it's
> > > > fundamentally different from the other system calls that he has
> > > > converted already to work with time64 callers.  
> > >
> > > I'm not aware of any RV32 specifics, but it seems to me that it
> > > would be appropriate to use the 64 bit version of struct
> > > __itimerspec64 in glibc - as for example in the conversion patch
> > > from [1].  
> >
> > What I mean is that rv32 otherwise does not convert between time32
> > and time64 interfaces because it always uses the time64 version,
> > so unlike the others, there is probably no helper to convert between
> > the timeval formats either.  
> 
> I have some patches prepared that will convert a 64-bit time_t to
> 32-bit for the required syscalls. It's generic for 32-bit archs, but
> will only apply when __TIMESIZE == 64.
> 
> I'll send an RFC out with the RV32 patches soon and then send patches
> when the 2.32 merge window opens up.

Great. Thanks :-)

> 
> 
> Alistair
> 
> >  
> > > As it was already mentioned - those calls set the time to be
> > > decremented and do not operate on "absolute" time values.
> > > Hence, I think that it would be good enough (for now?) to use 32
> > > bit API wrapped into 64 bit internal glibc values and just return
> > > errors when somebody wants to set timer relative expiration time
> > > to overflow time_t on 32 bit archs (arm,rv32).  
> >
> > Yes, that's the idea. The kernel already limits the range to 64-bit
> > nanoseconds because of its timer implementation, so truncating it
> > to 32-bit seconds does not change the behavior either.
> >  
> > > Arnd, am I correct that the struct itimerval to
> > > __kernel_old_itimerval conversion patch can be found here [2]?  
> >
> > Yes, that's right. This patch only changes the in-kernel
> > implementation as a step to removing the timeval definition from
> > the kernel's uapi headers, it does not change the behavior at all.
> >
> >        Arnd  




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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]