diff mbox

[RFC] AARCH64/ILP32: introduce kernel time types

Message ID 1467103498-24243-1-git-send-email-ynorov@caviumnetworks.com
State New, archived
Headers

Commit Message

Yury Norov June 28, 2016, 8:44 a.m. UTC 
  Structures utmp and utmpx are shared between ilp32 and native ABIs,
and so should have same layout. As now, architectures where this
compatibility is required enable __WORDSIZE_TIME64_COMPAT32 option,
and so make lp64 utmp{x} backward-compatible to ilp32 ones.

AARCH64 doesn't require such compatibility, and so we can do opposite
conversion.

This patch introduces new option  __KERNEL_TIME_T_MATCHES_TIME64_T,
and adds internal __ktime_t, __kusecond_t, __ksusecond_t types,
that are native-sized for normal case, and forced to 64-bit size if
__KERNEL_TIME_T_MATCHES_TIME64_T is enabled.

This is generic solution because we are facing y2038 problem, and
all new ABIs should follow this convention. This approach might also
be used for RISC-V as default.

If no objections, I'll incorporate it to v2 of AARCH64/ILP32 patchset.

Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
---
 bits/types.h                                     | 17 +++++++++++++++--
 sysdeps/gnu/bits/utmp.h                          |  6 +++---
 sysdeps/gnu/bits/utmpx.h                         |  4 ++--
 sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h |  7 ++++++-
 sysdeps/unix/sysv/linux/bits/time.h              |  6 ++++++
 5 files changed, 32 insertions(+), 8 deletions(-)

Comments

Florian Weimer June 28, 2016, 8:51 a.m. UTC | #1 
On 06/28/2016 10:44 AM, Yury Norov wrote:
> Structures utmp and utmpx are shared between ilp32 and native ABIs,
> and so should have same layout. As now, architectures where this
> compatibility is required enable __WORDSIZE_TIME64_COMPAT32 option,
> and so make lp64 utmp{x} backward-compatible to ilp32 ones.
>
> AARCH64 doesn't require such compatibility, and so we can do opposite
> conversion.

Thanks, I agree this is the right way forward.

> This patch introduces new option  __KERNEL_TIME_T_MATCHES_TIME64_T,

> +struct __kernel_timeval
> +  {
> +    __ktime_t tv_sec;		/* Seconds.  */
> +    __ksuseconds_t tv_usec;	/* Microseconds.  */
> +  };

We rarely use the __KERNEL_ and __kernel_ prefixes, but the kernel uapi 
includes do.  I would suggest to use a different prefix.

Thanks,
Florian
Yury Norov June 28, 2016, 8:56 a.m. UTC | #2 
On Tue, Jun 28, 2016 at 10:51:49AM +0200, Florian Weimer wrote:
> On 06/28/2016 10:44 AM, Yury Norov wrote:
> >Structures utmp and utmpx are shared between ilp32 and native ABIs,
> >and so should have same layout. As now, architectures where this
> >compatibility is required enable __WORDSIZE_TIME64_COMPAT32 option,
> >and so make lp64 utmp{x} backward-compatible to ilp32 ones.
> >
> >AARCH64 doesn't require such compatibility, and so we can do opposite
> >conversion.
> 
> Thanks, I agree this is the right way forward.
> 
> >This patch introduces new option  __KERNEL_TIME_T_MATCHES_TIME64_T,
> 
> >+struct __kernel_timeval
> >+  {
> >+    __ktime_t tv_sec;		/* Seconds.  */
> >+    __ksuseconds_t tv_usec;	/* Microseconds.  */
> >+  };
> 
> We rarely use the __KERNEL_ and __kernel_ prefixes, but the kernel uapi
> includes do.  I would suggest to use a different prefix.

__kernel_time_t is already imported from linux headers. It is defined
wrong, at least for arm64, and is not used. I don't want to touch it,
as I am not sure other C libraries don't use it.

> 
> Thanks,
> Florian
Arnd Bergmann June 28, 2016, 8:59 a.m. UTC | #3 
On Tuesday, June 28, 2016 11:44:58 AM CEST Yury Norov wrote:
> Structures utmp and utmpx are shared between ilp32 and native ABIs,
> and so should have same layout. As now, architectures where this
> compatibility is required enable __WORDSIZE_TIME64_COMPAT32 option,
> and so make lp64 utmp{x} backward-compatible to ilp32 ones.
> 
> AARCH64 doesn't require such compatibility, and so we can do opposite
> conversion.
> 
> This patch introduces new option  __KERNEL_TIME_T_MATCHES_TIME64_T,
> and adds internal __ktime_t, __kusecond_t, __ksusecond_t types,
> that are native-sized for normal case, and forced to 64-bit size if
> __KERNEL_TIME_T_MATCHES_TIME64_T is enabled.
> 
> This is generic solution because we are facing y2038 problem, and
> all new ABIs should follow this convention. This approach might also
> be used for RISC-V as default.
> 
> If no objections, I'll incorporate it to v2 of AARCH64/ILP32 patchset.
> 
> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>

I don't think it's a good idea to start with AARCH64/ILP32 doing this,
there are way to many interdependencies:

* I've been working on adding 64-bit time_t support to the kernel for
  multiple years now, and most of the work is still pending. For
  now, 32-bit architectures are stuck with 32-bit time_t, but we
  will fix them all.

* x86/x32 tried defining a user space ABI that had 32-bit time_t,
  but that never worked out properly and we still run into new
  bugs because of this. We discussed this for aarch64/ilp32 in
  great length (also over multiple years) and had a very clear
  decision that this would be handled differently, by using the
  regular 32-bit syscall ABI in the kernel and introduce 64-bit
  time_t at the same time as all other 32-bit architectures do.

* There is another long-running effort to add support for 64-bit
  time_t to glibc, currently in the discussion phase but
  now converging on an approach. This will introduce 64-bit
  time_t for all 32-bit architectures as an option (while still
  supporting 32-bit time_t), but it will only work correctly
  in all corner cases when running on a kernel that also support
  it (see first point). See
  https://sourceware.org/glibc/wiki/Y2038ProofnessDesign for
  more details.

I think glibc should do what you have in your patch, but it
should not become a prerequisite for the aarch64/ilp32 support,
otherwise it will take a few more years to have a working port.

	Arnd
Yury Norov June 28, 2016, 9:07 a.m. UTC | #4 
On Tue, Jun 28, 2016 at 10:59:56AM +0200, Arnd Bergmann wrote:
> On Tuesday, June 28, 2016 11:44:58 AM CEST Yury Norov wrote:
> > Structures utmp and utmpx are shared between ilp32 and native ABIs,
> > and so should have same layout. As now, architectures where this
> > compatibility is required enable __WORDSIZE_TIME64_COMPAT32 option,
> > and so make lp64 utmp{x} backward-compatible to ilp32 ones.
> > 
> > AARCH64 doesn't require such compatibility, and so we can do opposite
> > conversion.
> > 
> > This patch introduces new option  __KERNEL_TIME_T_MATCHES_TIME64_T,
> > and adds internal __ktime_t, __kusecond_t, __ksusecond_t types,
> > that are native-sized for normal case, and forced to 64-bit size if
> > __KERNEL_TIME_T_MATCHES_TIME64_T is enabled.
> > 
> > This is generic solution because we are facing y2038 problem, and
> > all new ABIs should follow this convention. This approach might also
> > be used for RISC-V as default.
> > 
> > If no objections, I'll incorporate it to v2 of AARCH64/ILP32 patchset.
> > 
> > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
> 
> I don't think it's a good idea to start with AARCH64/ILP32 doing this,
> there are way to many interdependencies:
> 
> * I've been working on adding 64-bit time_t support to the kernel for
>   multiple years now, and most of the work is still pending. For
>   now, 32-bit architectures are stuck with 32-bit time_t, but we
>   will fix them all.

This is not about 64-bit time_t. This is about new type __ktime_t that
can replace time_t where it's safe.

> 
> * x86/x32 tried defining a user space ABI that had 32-bit time_t,
>   but that never worked out properly and we still run into new
>   bugs because of this. We discussed this for aarch64/ilp32 in
>   great length (also over multiple years) and had a very clear
>   decision that this would be handled differently, by using the
>   regular 32-bit syscall ABI in the kernel and introduce 64-bit
>   time_t at the same time as all other 32-bit architectures do.
> 
> * There is another long-running effort to add support for 64-bit
>   time_t to glibc, currently in the discussion phase but
>   now converging on an approach. This will introduce 64-bit
>   time_t for all 32-bit architectures as an option (while still
>   supporting 32-bit time_t), but it will only work correctly
>   in all corner cases when running on a kernel that also support
>   it (see first point). See
>   https://sourceware.org/glibc/wiki/Y2038ProofnessDesign for
>   more details.
> 
> I think glibc should do what you have in your patch, but it
> should not become a prerequisite for the aarch64/ilp32 support,
> otherwise it will take a few more years to have a working port.
> 
> 	Arnd

If generally this approach is OK, I can split the patch to completely
generic part, and small part that only enables __KERNEL_TIME_T_MATCHES_TIME64_T
in aarch64. First one can be applied independently from aarch64/ilp32.

Yury.
Arnd Bergmann June 28, 2016, 9:16 a.m. UTC | #5 
On Tuesday, June 28, 2016 11:56:49 AM CEST Yury Norov wrote:
> On Tue, Jun 28, 2016 at 10:51:49AM +0200, Florian Weimer wrote:
> > On 06/28/2016 10:44 AM, Yury Norov wrote:
> > >Structures utmp and utmpx are shared between ilp32 and native ABIs,
> > >and so should have same layout. As now, architectures where this
> > >compatibility is required enable __WORDSIZE_TIME64_COMPAT32 option,
> > >and so make lp64 utmp{x} backward-compatible to ilp32 ones.
> > >
> > >AARCH64 doesn't require such compatibility, and so we can do opposite
> > >conversion.
> > 
> > Thanks, I agree this is the right way forward.
> > 
> > >This patch introduces new option  __KERNEL_TIME_T_MATCHES_TIME64_T,
> > 
> > >+struct __kernel_timeval
> > >+  {
> > >+    __ktime_t tv_sec;               /* Seconds.  */
> > >+    __ksuseconds_t tv_usec; /* Microseconds.  */
> > >+  };
> > 
> > We rarely use the __KERNEL_ and __kernel_ prefixes, but the kernel uapi
> > includes do.  I would suggest to use a different prefix.
> 
> __kernel_time_t is already imported from linux headers. It is defined
> wrong, at least for arm64, and is not used. I don't want to touch it,
> as I am not sure other C libraries don't use it.

__kernel_time_t is what gets used in the kernel header files and
is always defined as 'long' (except on x86/x32), and it is the type
used for all syscall interfaces.

We have to separate three things here:

- the type for time_t that is used between glibc and applications
- the type for __kernel_time_t that is used between the kernel and glibc
- the type used for utmp/utmpx

My first reply was about the first two, which we cannot really change
freely. What you want here is the third, but that should be entirely
separate from anything that has the word 'kernel' in it.

It's worth noting that apparently the arm64 glibc port has diverged
from the practice of the other 64-bit ports that have an alternative
32-bit architecture, and the utmp/utmpx formats are incompatible
between 32-bit ARM and the existing ARM64. This seems unfixable
at this point, until we have come up with a solution that works
around the y2038 overflow of utmpx on arm32.

We probably want something local to the sysdeps/gnu/bits/utmp{,x}.h
files that makes the timestamps 64-bit for all new architectures,
but defining a 'struct __kernel_timeval' here would not be helpful.

	Arnd
Szabolcs Nagy June 28, 2016, 9:25 a.m. UTC | #6 
On 28/06/16 10:07, Yury Norov wrote:
> This is not about 64-bit time_t. This is about new type __ktime_t that
> can replace time_t where it's safe.
> 

there is no need to introduce new type names
see the __WORDSIZE_TIME64_COMPAT32 case.
Yury Norov June 28, 2016, 9:50 a.m. UTC | #7 
On Tue, Jun 28, 2016 at 11:16:17AM +0200, Arnd Bergmann wrote:
> On Tuesday, June 28, 2016 11:56:49 AM CEST Yury Norov wrote:
> > On Tue, Jun 28, 2016 at 10:51:49AM +0200, Florian Weimer wrote:
> > > On 06/28/2016 10:44 AM, Yury Norov wrote:
> > > >Structures utmp and utmpx are shared between ilp32 and native ABIs,
> > > >and so should have same layout. As now, architectures where this
> > > >compatibility is required enable __WORDSIZE_TIME64_COMPAT32 option,
> > > >and so make lp64 utmp{x} backward-compatible to ilp32 ones.
> > > >
> > > >AARCH64 doesn't require such compatibility, and so we can do opposite
> > > >conversion.
> > > 
> > > Thanks, I agree this is the right way forward.
> > > 
> > > >This patch introduces new option  __KERNEL_TIME_T_MATCHES_TIME64_T,
> > > 
> > > >+struct __kernel_timeval
> > > >+  {
> > > >+    __ktime_t tv_sec;               /* Seconds.  */
> > > >+    __ksuseconds_t tv_usec; /* Microseconds.  */
> > > >+  };
> > > 
> > > We rarely use the __KERNEL_ and __kernel_ prefixes, but the kernel uapi
> > > includes do.  I would suggest to use a different prefix.
> > 
> > __kernel_time_t is already imported from linux headers. It is defined
> > wrong, at least for arm64, and is not used. I don't want to touch it,
> > as I am not sure other C libraries don't use it.
> 
> __kernel_time_t is what gets used in the kernel header files and
> is always defined as 'long' (except on x86/x32), and it is the type
> used for all syscall interfaces.
> 
> We have to separate three things here:
> 
> - the type for time_t that is used between glibc and applications
> - the type for __kernel_time_t that is used between the kernel and glibc
> - the type used for utmp/utmpx
> 
> My first reply was about the first two, which we cannot really change
> freely. What you want here is the third, but that should be entirely
> separate from anything that has the word 'kernel' in it.
> 
> It's worth noting that apparently the arm64 glibc port has diverged
> from the practice of the other 64-bit ports that have an alternative
> 32-bit architecture, and the utmp/utmpx formats are incompatible
> between 32-bit ARM and the existing ARM64. This seems unfixable
> at this point, until we have come up with a solution that works
> around the y2038 overflow of utmpx on arm32.
> 
> We probably want something local to the sysdeps/gnu/bits/utmp{,x}.h
> files that makes the timestamps 64-bit for all new architectures,
> but defining a 'struct __kernel_timeval' here would not be helpful.

64-bit time is needed for struct __kernel_timespec that is used in
struct stat, and probably somewhere else. So it's not so local. For
me it's good idea to use new type for new ABIs, make old type deprecated,
and re-write the code that use it one by one. Right now I try to throw out
platform code that handle struct stat{,fs} - related syscalls in
aarch64/ilp32, and switch to generic ones. I think it's possible with
new types.

I thought your Y2038 series is about relatively same. If you plan just
to switch time_t to 64-bit, and fix all 32-bit world after it, I think
it is very difficult, if possible at all.

Yury.
Andreas Schwab June 28, 2016, 10:07 a.m. UTC | #8 
Yury Norov <ynorov@caviumnetworks.com> writes:

> diff --git a/sysdeps/gnu/bits/utmp.h b/sysdeps/gnu/bits/utmp.h
> index 2a1ffcb..fae9126 100644
> --- a/sysdeps/gnu/bits/utmp.h
> +++ b/sysdeps/gnu/bits/utmp.h
> @@ -38,7 +38,7 @@ struct lastlog
>  #ifdef __WORDSIZE_TIME64_COMPAT32
>      int32_t ll_time;
>  #else
> -    __time_t ll_time;
> +    __ktime_t ll_time;
>  #endif
>      char ll_line[UT_LINESIZE];
>      char ll_host[UT_HOSTSIZE];
> @@ -76,8 +76,8 @@ struct utmp
>      int32_t tv_usec;		/* Microseconds.  */
>    } ut_tv;			/* Time entry was made.  */
>  #else
> -  long int ut_session;		/* Session ID, used for windowing.  */
> -  struct timeval ut_tv;		/* Time entry was made.  */
> +  __ktime_t ut_session;		/* Session ID, used for windowing.  */
> +  struct __kernel_timeval ut_tv;/* Time entry was made.  */
>  #endif

I don't think we should be using kernel-related types here.  This is a
user-space-only, on-disk struct, so it'd better use fixed size types.

Andreas.
Yury Norov June 28, 2016, 10:14 a.m. UTC | #9 
On Tue, Jun 28, 2016 at 10:25:50AM +0100, Szabolcs Nagy wrote:
> On 28/06/16 10:07, Yury Norov wrote:
> > This is not about 64-bit time_t. This is about new type __ktime_t that
> > can replace time_t where it's safe.
> > 
> 
> there is no need to introduce new type names
> see the __WORDSIZE_TIME64_COMPAT32 case.

It does not use time_t at all, but int32_t. I think it's a hack. If
we follow it, affected structures will be overloaded with ifdefs. In
fact, I tried it first, and didn't like what I get. This approach
moves type declaration where it should be, and maintains user-visible
structures simple and clear.

Yury.
Yury Norov June 28, 2016, 10:20 a.m. UTC | #10 
On Tue, Jun 28, 2016 at 12:07:57PM +0200, Andreas Schwab wrote:
> Yury Norov <ynorov@caviumnetworks.com> writes:
> 
> > diff --git a/sysdeps/gnu/bits/utmp.h b/sysdeps/gnu/bits/utmp.h
> > index 2a1ffcb..fae9126 100644
> > --- a/sysdeps/gnu/bits/utmp.h
> > +++ b/sysdeps/gnu/bits/utmp.h
> > @@ -38,7 +38,7 @@ struct lastlog
> >  #ifdef __WORDSIZE_TIME64_COMPAT32
> >      int32_t ll_time;
> >  #else
> > -    __time_t ll_time;
> > +    __ktime_t ll_time;
> >  #endif
> >      char ll_line[UT_LINESIZE];
> >      char ll_host[UT_HOSTSIZE];
> > @@ -76,8 +76,8 @@ struct utmp
> >      int32_t tv_usec;		/* Microseconds.  */
> >    } ut_tv;			/* Time entry was made.  */
> >  #else
> > -  long int ut_session;		/* Session ID, used for windowing.  */
> > -  struct timeval ut_tv;		/* Time entry was made.  */
> > +  __ktime_t ut_session;		/* Session ID, used for windowing.  */
> > +  struct __kernel_timeval ut_tv;/* Time entry was made.  */
> >  #endif
> 
> I don't think we should be using kernel-related types here.  This is a
> user-space-only, on-disk struct, so it'd better use fixed size types.
> 
> Andreas.

Hi Andreas,

Utmp does not deal with kernel, that's true. But this is time types by
nature, and native ABI keeps it synced with kernel. And ILP ABI has to
be compatible with navive ABI, and so has to have it synced with
kernel as well.

New types are useful to turn ilp32 to handle stat-related syscalls in
generic code. See my answer to Arnd for details.

Yury.
Andreas Schwab June 28, 2016, 10:27 a.m. UTC | #11 
Yury Norov <ynorov@caviumnetworks.com> writes:

> Utmp does not deal with kernel, that's true. But this is time types by
> nature, and native ABI keeps it synced with kernel. And ILP ABI has to
> be compatible with navive ABI, and so has to have it synced with
> kernel as well.

Since it is an on-disk format it needs to be handled specially, to avoid
the difficulties we have now on aarch64 to resurface for future ports.

Andreas.
Yury Norov June 28, 2016, 10:58 a.m. UTC | #12 
On Tue, Jun 28, 2016 at 12:27:34PM +0200, Andreas Schwab wrote:
> Yury Norov <ynorov@caviumnetworks.com> writes:
> 
> > Utmp does not deal with kernel, that's true. But this is time types by
> > nature, and native ABI keeps it synced with kernel. And ILP ABI has to
> > be compatible with navive ABI, and so has to have it synced with
> > kernel as well.
> 
> Since it is an on-disk format it needs to be handled specially, to avoid
> the difficulties we have now on aarch64 to resurface for future ports.
> 
> Andreas.

I still think this is correct way. We have single, kernel-synced type
for communicating with kernel and native ABI (disk), and convert it to
user type where needed (currently for aarch64/stat only). It also
helps to move to 64-bit time types smoothly, as we can make time_t
deprecated and force new APIs use __ktime_t.

What sort of difficulties you see with this approach?

Yury.
Szabolcs Nagy June 28, 2016, 11:22 a.m. UTC | #13 
On 28/06/16 11:58, Yury Norov wrote:
> On Tue, Jun 28, 2016 at 12:27:34PM +0200, Andreas Schwab wrote:
>> Yury Norov <ynorov@caviumnetworks.com> writes:
>>
>>> Utmp does not deal with kernel, that's true. But this is time types by
>>> nature, and native ABI keeps it synced with kernel. And ILP ABI has to
>>> be compatible with navive ABI, and so has to have it synced with
>>> kernel as well.
>>
>> Since it is an on-disk format it needs to be handled specially, to avoid
>> the difficulties we have now on aarch64 to resurface for future ports.
>>
>> Andreas.
> 
> I still think this is correct way. We have single, kernel-synced type
> for communicating with kernel and native ABI (disk), and convert it to
> user type where needed (currently for aarch64/stat only). It also
> helps to move to 64-bit time types smoothly, as we can make time_t
> deprecated and force new APIs use __ktime_t.
> 
> What sort of difficulties you see with this approach?
> 

i agree with andreas.

i don't think kernel names should be involved here.

the hack is to make the on-disk format compatible across
abis, there is no guarantee that the kernel types will
be always the same, using fixed sized types makes the
intent clear.

(ideally the on-disk format would be the same on all
abis not just on the ilp32 and lp64 abi of the same arch,
since the same rootfs may be mounted on different hosts
with a proper multiarch setup..  and ideally the public
struct types would not need any changes to make this work,
they could be serialized into a portable format.)

i call it a hack because it is non-conforming, the utmpx
struct is specified by posix.. which is why i originally
suggested to postpone fixing this in the ilp32 patches.
Arnd Bergmann June 28, 2016, 11:47 a.m. UTC | #14 
On Tuesday, June 28, 2016 12:50:21 PM CEST Yury Norov wrote:
> On Tue, Jun 28, 2016 at 11:16:17AM +0200, Arnd Bergmann wrote:
> > On Tuesday, June 28, 2016 11:56:49 AM CEST Yury Norov wrote:
> > > On Tue, Jun 28, 2016 at 10:51:49AM +0200, Florian Weimer wrote:
> > > > On 06/28/2016 10:44 AM, Yury Norov wrote:
> > > > >Structures utmp and utmpx are shared between ilp32 and native ABIs,
> > > > >and so should have same layout. As now, architectures where this
> > > > >compatibility is required enable __WORDSIZE_TIME64_COMPAT32 option,
> > > > >and so make lp64 utmp{x} backward-compatible to ilp32 ones.
> > > > >
> > > > >AARCH64 doesn't require such compatibility, and so we can do opposite
> > > > >conversion.
> > > > 
> > > > Thanks, I agree this is the right way forward.
> > > > 
> > > > >This patch introduces new option  __KERNEL_TIME_T_MATCHES_TIME64_T,
> > > > 
> > > > >+struct __kernel_timeval
> > > > >+  {
> > > > >+    __ktime_t tv_sec;               /* Seconds.  */
> > > > >+    __ksuseconds_t tv_usec; /* Microseconds.  */
> > > > >+  };
> > > > 
> > > > We rarely use the __KERNEL_ and __kernel_ prefixes, but the kernel uapi
> > > > includes do.  I would suggest to use a different prefix.
> > > 
> > > __kernel_time_t is already imported from linux headers. It is defined
> > > wrong, at least for arm64, and is not used. I don't want to touch it,
> > > as I am not sure other C libraries don't use it.
> > 
> > __kernel_time_t is what gets used in the kernel header files and
> > is always defined as 'long' (except on x86/x32), and it is the type
> > used for all syscall interfaces.
> > 
> > We have to separate three things here:
> > 
> > - the type for time_t that is used between glibc and applications
> > - the type for __kernel_time_t that is used between the kernel and glibc
> > - the type used for utmp/utmpx
> > 
> > My first reply was about the first two, which we cannot really change
> > freely. What you want here is the third, but that should be entirely
> > separate from anything that has the word 'kernel' in it.
> > 
> > It's worth noting that apparently the arm64 glibc port has diverged
> > from the practice of the other 64-bit ports that have an alternative
> > 32-bit architecture, and the utmp/utmpx formats are incompatible
> > between 32-bit ARM and the existing ARM64. This seems unfixable
> > at this point, until we have come up with a solution that works
> > around the y2038 overflow of utmpx on arm32.
> > 
> > We probably want something local to the sysdeps/gnu/bits/utmp{,x}.h
> > files that makes the timestamps 64-bit for all new architectures,
> > but defining a 'struct __kernel_timeval' here would not be helpful.
> 
> 64-bit time is needed for struct __kernel_timespec that is used in
> struct stat, and probably somewhere else. So it's not so local. For
> me it's good idea to use new type for new ABIs, make old type deprecated,
> and re-write the code that use it one by one. Right now I try to throw out
> platform code that handle struct stat{,fs} - related syscalls in
> aarch64/ilp32, and switch to generic ones. I think it's possible with
> new types.

It's fine if you want to create a new type, just don't mix it with the
types defined by the kernel.

__kernel_timespec in particular is exactly the type that I want to
use for the new timespec interface, and that type will be defined
in the kernel headers, so if you define the same type in the libc
headers, they clash. We generally have the convention of prefixing
types from the kernel with __kernel_ precisely so that it's something
the libc will not clash with.

I don't know how glibc defines 'stat' today, I assumed it did not
use the type from the kernel headers but defined its own and converted
between the formats. At least looking at
sysdeps/unix/sysv/linux/mips/kernel_stat.h shows that not all
architectures even use a structure here.

	Arnd
Joseph Myers June 28, 2016, 11:55 a.m. UTC | #15 
On Tue, 28 Jun 2016, Szabolcs Nagy wrote:

> i agree with andreas.
> 
> i don't think kernel names should be involved here.

I also agree.  The types in utmp have *nothing* to do with the kernel.  
Kernel time types should not appear in any public glibc interfaces at all, 
only in code translating between kernel and glibc interfaces.

> (ideally the on-disk format would be the same on all
> abis not just on the ilp32 and lp64 abi of the same arch,
> since the same rootfs may be mounted on different hosts
> with a proper multiarch setup..  and ideally the public
> struct types would not need any changes to make this work,
> they could be serialized into a portable format.)

Yes (subject to a question of whether we actually care about 
endianness-independence of the on-disk format, or only about such cases as 
AArch32 / AArch64 ILP32 / AArch64 LP64 all of same endianness).  I think 
the goal should be that the struct types in the headers are 
POSIX-conforming on all architectures, with translation for file input / 
output as needed so that applications with 32-bit and 64-bit time_t, and 
applications for different ABI variants, can access the same files, those 
files always storing 64-bit time values.  So if any changes to utmp 
structures are being made, let's make them in that direction.
Arnd Bergmann June 28, 2016, 11:58 a.m. UTC | #16 
On Tuesday, June 28, 2016 12:22:16 PM CEST Szabolcs Nagy wrote:
> i don't think kernel names should be involved here.

yup.

> (ideally the on-disk format would be the same on all
> abis not just on the ilp32 and lp64 abi of the same arch,
> since the same rootfs may be mounted on different hosts
> with a proper multiarch setup..  and ideally the public
> struct types would not need any changes to make this work,
> they could be serialized into a portable format.)
> 
> i call it a hack because it is non-conforming, the utmpx
> struct is specified by posix.. which is why i originally
> suggested to postpone fixing this in the ilp32 patches.

We should probably add the file format to
https://sourceware.org/glibc/wiki/Y2038ProofnessDesign

That page already documents the need to change the
utmp/utmpx based APIs, but does not mention two points:


- unlike all others, it will overflow on both 32-bit
  systems and some important 64-bit architectures that
  set __WORDSIZE_TIME64_COMPAT32: mips, powerpc, sparc, tile,
  and x86. The other ones (s390, parisc, arm64) are
  apparently broken because they are incompatibe between
  32-bit and 64-bit user space.

- unlike (AFAICT) all other structures, it is needed in
  a persistent file format on disk, so it cannot simply
  be handled by adding new symbols. (Note: there are
  probably applications that store times in files, and
  they will have the same problem: either they are
  incompatible between 32/64-bit binaries or they may
  overflow in 2038 or 2106 if they use 32-bit types
  unconditionally).

	Arnd
Yury Norov June 28, 2016, 12:06 p.m. UTC | #17 
On Tue, Jun 28, 2016 at 01:47:35PM +0200, Arnd Bergmann wrote:
> On Tuesday, June 28, 2016 12:50:21 PM CEST Yury Norov wrote:
> > On Tue, Jun 28, 2016 at 11:16:17AM +0200, Arnd Bergmann wrote:
> > > On Tuesday, June 28, 2016 11:56:49 AM CEST Yury Norov wrote:
> > > > On Tue, Jun 28, 2016 at 10:51:49AM +0200, Florian Weimer wrote:
> > > > > On 06/28/2016 10:44 AM, Yury Norov wrote:
> > > > > >Structures utmp and utmpx are shared between ilp32 and native ABIs,
> > > > > >and so should have same layout. As now, architectures where this
> > > > > >compatibility is required enable __WORDSIZE_TIME64_COMPAT32 option,
> > > > > >and so make lp64 utmp{x} backward-compatible to ilp32 ones.
> > > > > >
> > > > > >AARCH64 doesn't require such compatibility, and so we can do opposite
> > > > > >conversion.
> > > > > 
> > > > > Thanks, I agree this is the right way forward.
> > > > > 
> > > > > >This patch introduces new option  __KERNEL_TIME_T_MATCHES_TIME64_T,
> > > > > 
> > > > > >+struct __kernel_timeval
> > > > > >+  {
> > > > > >+    __ktime_t tv_sec;               /* Seconds.  */
> > > > > >+    __ksuseconds_t tv_usec; /* Microseconds.  */
> > > > > >+  };
> > > > > 
> > > > > We rarely use the __KERNEL_ and __kernel_ prefixes, but the kernel uapi
> > > > > includes do.  I would suggest to use a different prefix.
> > > > 
> > > > __kernel_time_t is already imported from linux headers. It is defined
> > > > wrong, at least for arm64, and is not used. I don't want to touch it,
> > > > as I am not sure other C libraries don't use it.
> > > 
> > > __kernel_time_t is what gets used in the kernel header files and
> > > is always defined as 'long' (except on x86/x32), and it is the type
> > > used for all syscall interfaces.
> > > 
> > > We have to separate three things here:
> > > 
> > > - the type for time_t that is used between glibc and applications
> > > - the type for __kernel_time_t that is used between the kernel and glibc
> > > - the type used for utmp/utmpx
> > > 
> > > My first reply was about the first two, which we cannot really change
> > > freely. What you want here is the third, but that should be entirely
> > > separate from anything that has the word 'kernel' in it.
> > > 
> > > It's worth noting that apparently the arm64 glibc port has diverged
> > > from the practice of the other 64-bit ports that have an alternative
> > > 32-bit architecture, and the utmp/utmpx formats are incompatible
> > > between 32-bit ARM and the existing ARM64. This seems unfixable
> > > at this point, until we have come up with a solution that works
> > > around the y2038 overflow of utmpx on arm32.
> > > 
> > > We probably want something local to the sysdeps/gnu/bits/utmp{,x}.h
> > > files that makes the timestamps 64-bit for all new architectures,
> > > but defining a 'struct __kernel_timeval' here would not be helpful.
> > 
> > 64-bit time is needed for struct __kernel_timespec that is used in
> > struct stat, and probably somewhere else. So it's not so local. For
> > me it's good idea to use new type for new ABIs, make old type deprecated,
> > and re-write the code that use it one by one. Right now I try to throw out
> > platform code that handle struct stat{,fs} - related syscalls in
> > aarch64/ilp32, and switch to generic ones. I think it's possible with
> > new types.
> 
> It's fine if you want to create a new type, just don't mix it with the
> types defined by the kernel.
> 
> __kernel_timespec in particular is exactly the type that I want to
> use for the new timespec interface, and that type will be defined
> in the kernel headers, so if you define the same type in the libc
> headers, they clash. We generally have the convention of prefixing
> types from the kernel with __kernel_ precisely so that it's something
> the libc will not clash with.

That was my point to end up importing them from kernel. When kernel will
export __kernel_timespec, correct __kernel_time_t etc, it will be better.
We can wrap them with #ifdefs right now. But we need them in glibc because
old kernels will not export it.

> I don't know how glibc defines 'stat' today, I assumed it did not
> use the type from the kernel headers but defined its own and converted
> between the formats. At least looking at
> sysdeps/unix/sysv/linux/mips/kernel_stat.h shows that not all
> architectures even use a structure here.
> 
> 	Arnd
Yury Norov June 28, 2016, 12:06 p.m. UTC | #18 
On Tue, Jun 28, 2016 at 11:55:29AM +0000, Joseph Myers wrote:
> On Tue, 28 Jun 2016, Szabolcs Nagy wrote:
> 
> > i agree with andreas.
> > 
> > i don't think kernel names should be involved here.
> 
> I also agree.  The types in utmp have *nothing* to do with the kernel.  
> Kernel time types should not appear in any public glibc interfaces at all, 
> only in code translating between kernel and glibc interfaces.
> 
> > (ideally the on-disk format would be the same on all
> > abis not just on the ilp32 and lp64 abi of the same arch,
> > since the same rootfs may be mounted on different hosts
> > with a proper multiarch setup..  and ideally the public
> > struct types would not need any changes to make this work,
> > they could be serialized into a portable format.)
> 
> Yes (subject to a question of whether we actually care about 
> endianness-independence of the on-disk format, or only about such cases as 
> AArch32 / AArch64 ILP32 / AArch64 LP64 all of same endianness).  I think 
> the goal should be that the struct types in the headers are 
> POSIX-conforming on all architectures, with translation for file input / 
> output as needed so that applications with 32-bit and 64-bit time_t, and 
> applications for different ABI variants, can access the same files, those 
> files always storing 64-bit time values.  So if any changes to utmp 
> structures are being made, let's make them in that direction.
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com

So, I drop this patch, and do nothing waiting for proper utmp/utmpx fix?

Yury.
Andreas Schwab June 28, 2016, 1:23 p.m. UTC | #19 
Yury Norov <ynorov@caviumnetworks.com> writes:

> So, I drop this patch, and do nothing waiting for proper utmp/utmpx fix?

For the time being, copy from s390.

Andreas.
Andreas Schwab June 28, 2016, 1:26 p.m. UTC | #20 
Arnd Bergmann <arnd@arndb.de> writes:

> - unlike all others, it will overflow on both 32-bit
>   systems and some important 64-bit architectures that
>   set __WORDSIZE_TIME64_COMPAT32: mips, powerpc, sparc, tile,
>   and x86. The other ones (s390, parisc, arm64) are
>   apparently broken because they are incompatibe between
>   32-bit and 64-bit user space.

s390 is using its own utmp.h file which uses 64-bit time fields.

Andreas.
Albert ARIBAUD June 28, 2016, 9:08 p.m. UTC | #21 
Bonjour Joseph,

Le Tue, 28 Jun 2016 11:55:29 +0000, Joseph Myers
<joseph@codesourcery.com> a écrit :

> On Tue, 28 Jun 2016, Szabolcs Nagy wrote:
> 
> > i agree with andreas.
> > 
> > i don't think kernel names should be involved here.  
> 
> I also agree.  The types in utmp have *nothing* to do with the kernel.  
> Kernel time types should not appear in any public glibc interfaces at all, 
> only in code translating between kernel and glibc interfaces.
> 
> > (ideally the on-disk format would be the same on all
> > abis not just on the ilp32 and lp64 abi of the same arch,
> > since the same rootfs may be mounted on different hosts
> > with a proper multiarch setup..  and ideally the public
> > struct types would not need any changes to make this work,
> > they could be serialized into a portable format.)  
> 
> Yes (subject to a question of whether we actually care about 
> endianness-independence of the on-disk format, or only about such cases as 
> AArch32 / AArch64 ILP32 / AArch64 LP64 all of same endianness).  I think 
> the goal should be that the struct types in the headers are 
> POSIX-conforming on all architectures, with translation for file input / 
> output as needed so that applications with 32-bit and 64-bit time_t, and 
> applications for different ABI variants, can access the same files, those 
> files always storing 64-bit time values.  So if any changes to utmp 
> structures are being made, let's make them in that direction.

OK, so, to sum up on utmp.

This is what we want:

- the utmp file structure should not depend on architecture word size
  or, ideally, endianness.

- the utmp/utmpx APIs should be conforming to POSIX.

- the utmp[x] struct should not depend on kernel types.

Here is a possible solution:

1. Create a separate utmp structure for the utmp file.

2. Make the file utmp struct's ut_tv.tv_sec field a signed 64-bit int
   unconditionally, and ideally, its endianness should be constant.

3. The API utmp[x] struct's ut_tv field would be a struct timeval as
   expected by POSIX.

4. GLIBC would translate between API utmp[x] structs and file utmp
   struct, both endianness and size. When translating a 64-bit tv_sec
   into a 32-bit tv_sec, if the value would overflow, then errno would
   be set to EOVERFLOW and a failure value would be returned.

5. Upgrade path for a 'new' glibc where the 'old' and 'new' utmp file
   formats are different in size, endianness or both: this would be best
   handled by the packaging system, all the more since there may also be
   dependency constraints involving some utilities which depend on the
   utmp file structure, like utmpdump.

Cordialement,
Albert ARIBAUD
3ADEV
Arnd Bergmann June 28, 2016, 9:49 p.m. UTC | #22 
On Tuesday, June 28, 2016 11:08:53 PM CEST Albert ARIBAUD wrote:
> OK, so, to sum up on utmp.
> 
> This is what we want:
> 
> - the utmp file structure should not depend on architecture word size
>   or, ideally, endianness.
> 
> - the utmp/utmpx APIs should be conforming to POSIX.
> 
> - the utmp[x] struct should not depend on kernel types.
> 
> Here is a possible solution:
> 
> 1. Create a separate utmp structure for the utmp file.
> 
> 2. Make the file utmp struct's ut_tv.tv_sec field a signed 64-bit int
>    unconditionally, and ideally, its endianness should be constant.
> 
> 3. The API utmp[x] struct's ut_tv field would be a struct timeval as
>    expected by POSIX.
> 
> 4. GLIBC would translate between API utmp[x] structs and file utmp
>    struct, both endianness and size. When translating a 64-bit tv_sec
>    into a 32-bit tv_sec, if the value would overflow, then errno would
>    be set to EOVERFLOW and a failure value would be returned.
> 
> 5. Upgrade path for a 'new' glibc where the 'old' and 'new' utmp file
>    formats are different in size, endianness or both: this would be best
>    handled by the packaging system, all the more since there may also be
>    dependency constraints involving some utilities which depend on the
>    utmp file structure, like utmpdump.

Nothing wrong with this approach, just another idea from how we handle
upgrading on-disk structures elsewhere:

In ext4, we use the two upper bits of the 32-bit nanoseconds to
extend the seconds. This way a structure that stores signed
seconds as 32 bit and can normally represent the range between 1902
and 2038 gets extended by another 3*136 years, so we can go until
2446 with a backwards-compatible extension. If we treat the on-disk
seconds as unsigned (we know that pre-1970 times are all guaranteed
to be invalid for utmp), we get another 68 years.

Obviously this is a trick that only works when you have full control
over all code that reads or writes the timestamps, and it doesn't
solve the endianess problem, but it avoids introducing an incompatible
format. Also, there is no reason for new architectures to do that,
they should just do what you describe above.

	Arnd
Andreas Schwab June 29, 2016, 7:03 a.m. UTC | #23 
Albert ARIBAUD <albert.aribaud@3adev.fr> writes:

> - the utmp file structure should not depend on architecture word size
>   or, ideally, endianness.

Endianness is not an issue as you cannot run LE and BE at the same time.

Andreas.
Albert ARIBAUD June 29, 2016, 7:05 a.m. UTC | #24 
Hi Arnd,

Le Tue, 28 Jun 2016 23:49:11 +0200, Arnd Bergmann <arnd@arndb.de> a
écrit :

> On Tuesday, June 28, 2016 11:08:53 PM CEST Albert ARIBAUD wrote:
> > OK, so, to sum up on utmp.
> > 
> > This is what we want:
> > 
> > - the utmp file structure should not depend on architecture word size
> >   or, ideally, endianness.
> > 
> > - the utmp/utmpx APIs should be conforming to POSIX.
> > 
> > - the utmp[x] struct should not depend on kernel types.
> > 
> > Here is a possible solution:
> > 
> > 1. Create a separate utmp structure for the utmp file.
> > 
> > 2. Make the file utmp struct's ut_tv.tv_sec field a signed 64-bit int
> >    unconditionally, and ideally, its endianness should be constant.
> > 
> > 3. The API utmp[x] struct's ut_tv field would be a struct timeval as
> >    expected by POSIX.
> > 
> > 4. GLIBC would translate between API utmp[x] structs and file utmp
> >    struct, both endianness and size. When translating a 64-bit tv_sec
> >    into a 32-bit tv_sec, if the value would overflow, then errno would
> >    be set to EOVERFLOW and a failure value would be returned.
> > 
> > 5. Upgrade path for a 'new' glibc where the 'old' and 'new' utmp file
> >    formats are different in size, endianness or both: this would be best
> >    handled by the packaging system, all the more since there may also be
> >    dependency constraints involving some utilities which depend on the
> >    utmp file structure, like utmpdump.  
> 
> Nothing wrong with this approach, just another idea from how we handle
> upgrading on-disk structures elsewhere:
> 
> In ext4, we use the two upper bits of the 32-bit nanoseconds to
> extend the seconds. This way a structure that stores signed
> seconds as 32 bit and can normally represent the range between 1902
> and 2038 gets extended by another 3*136 years, so we can go until
> 2446 with a backwards-compatible extension. If we treat the on-disk
> seconds as unsigned (we know that pre-1970 times are all guaranteed
> to be invalid for utmp), we get another 68 years.
> 
> Obviously this is a trick that only works when you have full control
> over all code that reads or writes the timestamps, and it doesn't
> solve the endianess problem, but it avoids introducing an incompatible
> format. Also, there is no reason for new architectures to do that,
> they should just do what you describe above.

As far as choosing between tricks, I would favor switching to an
unsigned 32-bit timestamp, which would be much simpler to code and less
run-time-error prone.

But this only buys us more time beyond Y2038 just to make a clean
transition to the full-64-bit solution, and we already have 22 years
for that, and in my experience, pushing the 'doomsdate' further back
just makes the issue more likely to be considered SEP and not be fixed
in time.

Plus, this transition would still be temporary, and we would still need
a transition from the 'old' (now 'possibly extended') format to the
'new' one.

Therefore, I would prefer avoiding any transitional state in which the
'old' format is 'just tweaked to allow for some time extension', and I
would prefer to transition straight from the current utmp file format
to the 64-bit time format.

Note: transitioning wtmp and btmp would be much easier, as they are log
files which can be rotated. The problem is with utmp because it is a
state file.

Actually, the problem is only between a GLIBC upgrade and the next
reboot, where utmp will be re-created, including the shutdown phase
itself, where some code might want to access the ('old') utmp file but
might accidentally use the new GLIBC.

A solution could be to reserve utmp, wtmp and btmp for the 'old' format
files and use utmpx, wtmpx and btmpx for the 'new' format files (which
would better match the POSIX header and struct name btw), keep the
'old' API implementations alongside the 'new' ones, and choose 'old' or
'new' as follows:

- (future) nominal case: if the utmp file does not exist, only use the
  utmpx file.

- transitioning: if the utmp file exists, use it for reading, and write
  to both the utmp and utmpx files.

Pros:

- as long as the utmp file exists (and as long as we don't reach
  Y2038...) the system remains (as) compatible (as it was before) with
  any application code written to handle utmp records.

- development versions of applications which read utmp can be tested
  against the utmpx file while the system itself still runs on the utmp
  file.

- transitioning is under system control: remove utmp when the whole
  system is ready for it.

Cons:

- this forces the GLIBC utmp code to keep the 'old' code for some
  time, which is a code phrase for 'future bitrot'.

- this doubles utmp-related file I/O. This might not be desirable on
  systems where there is a high rate of logins and logouts.

While we cannot avoid bitrot, we can at least make sure the whole
compatible code is kept within conditionals so that it can be easily
disabled, then removed; the trigger for disabling/removing should be
explicitly stated (in code as well as in docs) as "make sure this
is removed before Y2038 happens".

Regarding the file I/O rise, it cannot be avoided, and will have to be
contained by planning the system upgrade so that the transition is as
short as possible.

Cordialement,
Albert ARIBAUD
3ADEV
Arnd Bergmann June 29, 2016, 8:12 a.m. UTC | #25 
On Wednesday, June 29, 2016 9:05:02 AM CEST Albert ARIBAUD wrote:
> Le Tue, 28 Jun 2016 23:49:11 +0200, Arnd Bergmann <arnd@arndb.de> a écrit :
> > On Tuesday, June 28, 2016 11:08:53 PM CEST Albert ARIBAUD wrote:
> > > 5. Upgrade path for a 'new' glibc where the 'old' and 'new' utmp file
> > >    formats are different in size, endianness or both: this would be best
> > >    handled by the packaging system, all the more since there may also be
> > >    dependency constraints involving some utilities which depend on the
> > >    utmp file structure, like utmpdump.  
> > 
> > Nothing wrong with this approach, just another idea from how we handle
> > upgrading on-disk structures elsewhere:
> > 
> > In ext4, we use the two upper bits of the 32-bit nanoseconds to
> > extend the seconds. This way a structure that stores signed
> > seconds as 32 bit and can normally represent the range between 1902
> > and 2038 gets extended by another 3*136 years, so we can go until
> > 2446 with a backwards-compatible extension. If we treat the on-disk
> > seconds as unsigned (we know that pre-1970 times are all guaranteed
> > to be invalid for utmp), we get another 68 years.
> > 
> > Obviously this is a trick that only works when you have full control
> > over all code that reads or writes the timestamps, and it doesn't
> > solve the endianess problem, but it avoids introducing an incompatible
> > format. Also, there is no reason for new architectures to do that,
> > they should just do what you describe above.
> 
> As far as choosing between tricks, I would favor switching to an
> unsigned 32-bit timestamp, which would be much simpler to code and less
> run-time-error prone.
> 
> But this only buys us more time beyond Y2038 just to make a clean
> transition to the full-64-bit solution, and we already have 22 years
> for that, and in my experience, pushing the 'doomsdate' further back
> just makes the issue more likely to be considered SEP and not be fixed
> in time.
> 
> Plus, this transition would still be temporary, and we would still need
> a transition from the 'old' (now 'possibly extended') format to the
> 'new' one.

The idea would be that the transition can be done simply by
reinterpreting the current values. If we were to pick the
simplest method (using unsigned seconds unstead of signed),
the only changes we would see are:

- timestamps prevously written with an incorrect pre-1970 timestamp
  would become interpreted as incorrect post-2038 timestamps with
  a new glibc. They are incorrect either way, so we can choose to
  ignore this case

- correct post-2038 timestamps written by a new glibc would get
  interpreted as incorrect pre-1970 timestamps by an old glibc
  build. This is slightly more relevant than the first, but you
  could argue that it is no worse than not being able to read the
  file at all, which is what you get with an incompatible
  format change.

> Actually, the problem is only between a GLIBC upgrade and the next
> reboot, where utmp will be re-created, including the shutdown phase
> itself, where some code might want to access the ('old') utmp file but
> might accidentally use the new GLIBC.

What about the case of having multiple glibc installations on the
same machine: 

With a multiarch installation that has both 32-bit and 64-bit
binaries, both environments are accessing the same file, but
they might not be the same version.

> A solution could be to reserve utmp, wtmp and btmp for the 'old' format
> files and use utmpx, wtmpx and btmpx for the 'new' format files (which
> would better match the POSIX header and struct name btw), keep the
> 'old' API implementations alongside the 'new' ones, and choose 'old' or
> 'new' as follows:
> 
> - (future) nominal case: if the utmp file does not exist, only use the
>   utmpx file.
> 
> - transitioning: if the utmp file exists, use it for reading, and write
>   to both the utmp and utmpx files.
> 
> Pros:
> 
> - as long as the utmp file exists (and as long as we don't reach
>   Y2038...) the system remains (as) compatible (as it was before) with
>   any application code written to handle utmp records.
> 
> - development versions of applications which read utmp can be tested
>   against the utmpx file while the system itself still runs on the utmp
>   file.
> 
> - transitioning is under system control: remove utmp when the whole
>   system is ready for it.
> 
> Cons:
> 
> - this forces the GLIBC utmp code to keep the 'old' code for some
>   time, which is a code phrase for 'future bitrot'.
> 
> - this doubles utmp-related file I/O. This might not be desirable on
>   systems where there is a high rate of logins and logouts.
> 
> While we cannot avoid bitrot, we can at least make sure the whole
> compatible code is kept within conditionals so that it can be easily
> disabled, then removed; the trigger for disabling/removing should be
> explicitly stated (in code as well as in docs) as "make sure this
> is removed before Y2038 happens".
> 
> Regarding the file I/O rise, it cannot be avoided, and will have to be
> contained by planning the system upgrade so that the transition is as
> short as possible.

I think this approach can work, but I find the utmpx naming really confusing
here: As you say, it uses the same naming as the POSIX utmpx structure,
but it really does something else, as we'd likely keep both struct utmp
and struct utmpx identical, except that we'd get a new version of both
for the library interface.

	Arnd
Albert ARIBAUD June 29, 2016, 8:41 a.m. UTC | #26 
Hi Arnd,

Le Wed, 29 Jun 2016 10:12:59 +0200, Arnd Bergmann <arnd@arndb.de> a
écrit :

> On Wednesday, June 29, 2016 9:05:02 AM CEST Albert ARIBAUD wrote:
> > Le Tue, 28 Jun 2016 23:49:11 +0200, Arnd Bergmann <arnd@arndb.de> a écrit :  
> > > On Tuesday, June 28, 2016 11:08:53 PM CEST Albert ARIBAUD wrote:  
>  [...]  
> > > 
> > > Nothing wrong with this approach, just another idea from how we handle
> > > upgrading on-disk structures elsewhere:
> > > 
> > > In ext4, we use the two upper bits of the 32-bit nanoseconds to
> > > extend the seconds. This way a structure that stores signed
> > > seconds as 32 bit and can normally represent the range between 1902
> > > and 2038 gets extended by another 3*136 years, so we can go until
> > > 2446 with a backwards-compatible extension. If we treat the on-disk
> > > seconds as unsigned (we know that pre-1970 times are all guaranteed
> > > to be invalid for utmp), we get another 68 years.
> > > 
> > > Obviously this is a trick that only works when you have full control
> > > over all code that reads or writes the timestamps, and it doesn't
> > > solve the endianess problem, but it avoids introducing an incompatible
> > > format. Also, there is no reason for new architectures to do that,
> > > they should just do what you describe above.  
> > 
> > As far as choosing between tricks, I would favor switching to an
> > unsigned 32-bit timestamp, which would be much simpler to code and less
> > run-time-error prone.
> > 
> > But this only buys us more time beyond Y2038 just to make a clean
> > transition to the full-64-bit solution, and we already have 22 years
> > for that, and in my experience, pushing the 'doomsdate' further back
> > just makes the issue more likely to be considered SEP and not be fixed
> > in time.
> > 
> > Plus, this transition would still be temporary, and we would still need
> > a transition from the 'old' (now 'possibly extended') format to the
> > 'new' one.  
> 
> The idea would be that the transition can be done simply by
> reinterpreting the current values. If we were to pick the
> simplest method (using unsigned seconds unstead of signed),
> the only changes we would see are:
> 
> - timestamps prevously written with an incorrect pre-1970 timestamp
>   would become interpreted as incorrect post-2038 timestamps with
>   a new glibc. They are incorrect either way, so we can choose to
>   ignore this case
> 
> - correct post-2038 timestamps written by a new glibc would get
>   interpreted as incorrect pre-1970 timestamps by an old glibc
>   build. This is slightly more relevant than the first, but you
>   could argue that it is no worse than not being able to read the
>   file at all, which is what you get with an incompatible
>   format change.
> 
> > Actually, the problem is only between a GLIBC upgrade and the next
> > reboot, where utmp will be re-created, including the shutdown phase
> > itself, where some code might want to access the ('old') utmp file but
> > might accidentally use the new GLIBC.  
> 
> What about the case of having multiple glibc installations on the
> same machine: 
> 
> With a multiarch installation that has both 32-bit and 64-bit
> binaries, both environments are accessing the same file, but
> they might not be the same version.
> 
> > A solution could be to reserve utmp, wtmp and btmp for the 'old' format
> > files and use utmpx, wtmpx and btmpx for the 'new' format files (which
> > would better match the POSIX header and struct name btw), keep the
> > 'old' API implementations alongside the 'new' ones, and choose 'old' or
> > 'new' as follows:
> > 
> > - (future) nominal case: if the utmp file does not exist, only use the
> >   utmpx file.
> > 
> > - transitioning: if the utmp file exists, use it for reading, and write
> >   to both the utmp and utmpx files.
> > 
> > Pros:
> > 
> > - as long as the utmp file exists (and as long as we don't reach
> >   Y2038...) the system remains (as) compatible (as it was before) with
> >   any application code written to handle utmp records.
> > 
> > - development versions of applications which read utmp can be tested
> >   against the utmpx file while the system itself still runs on the utmp
> >   file.
> > 
> > - transitioning is under system control: remove utmp when the whole
> >   system is ready for it.
> > 
> > Cons:
> > 
> > - this forces the GLIBC utmp code to keep the 'old' code for some
> >   time, which is a code phrase for 'future bitrot'.
> > 
> > - this doubles utmp-related file I/O. This might not be desirable on
> >   systems where there is a high rate of logins and logouts.
> > 
> > While we cannot avoid bitrot, we can at least make sure the whole
> > compatible code is kept within conditionals so that it can be easily
> > disabled, then removed; the trigger for disabling/removing should be
> > explicitly stated (in code as well as in docs) as "make sure this
> > is removed before Y2038 happens".
> > 
> > Regarding the file I/O rise, it cannot be avoided, and will have to be
> > contained by planning the system upgrade so that the transition is as
> > short as possible.  
> 
> I think this approach can work, but I find the utmpx naming really confusing
> here: As you say, it uses the same naming as the POSIX utmpx structure,
> but it really does something else, as we'd likely keep both struct utmp
> and struct utmpx identical, except that we'd get a new version of both
> for the library interface.

Fair points. How about this:

- use file names utmp and utmp.trans

- (future) nominal case: if the utmp.trans file does not exist, then use
  the utmp file for reading and writing in the 'new' format.

- transitioning: if the utmp.trans file exists, then use the utmp file
  for reading in the 'old' format, and write to both the utmp file in
  'old' format and the utmp.trans file in 'new' format respectively.

Switching from transitional state to nominal state would amount to
renaming the utmp.trans file as utmp.

This way, the 'new' file name becomes unrelated to the POSIX struct
name, and any 'old' and 'new' GLIBCs on the system will agree on the
utmp file structure ('old' for all) while the system is in the
transitioning state.

Of course, the switch from transitioning to nominal state requires that
all GLIBCs on the system be 'new', since at this point, the utmp file
will have 'new' format for all.

Cordialement,
Albert ARIBAUD
3ADEV
Dan Horák June 30, 2016, 9:09 a.m. UTC | #27 
On Wed, 29 Jun 2016 09:03:18 +0200
Andreas Schwab <schwab@suse.de> wrote:

> Albert ARIBAUD <albert.aribaud@3adev.fr> writes:
> 
> > - the utmp file structure should not depend on architecture word
> > size or, ideally, endianness.
> 
> Endianness is not an issue as you cannot run LE and BE at the same
> time.

AFAIK the Linux kernel for POWER technically supports that, running
both BE and LE processes at the same time.


		Dan
Steven Munroe June 30, 2016, 6:12 p.m. UTC | #28 
On Thu, 2016-06-30 at 11:09 +0200, Dan Horák wrote:
> On Wed, 29 Jun 2016 09:03:18 +0200
> Andreas Schwab <schwab@suse.de> wrote:
> 
> > Albert ARIBAUD <albert.aribaud@3adev.fr> writes:
> > 
> > > - the utmp file structure should not depend on architecture word
> > > size or, ideally, endianness.
> > 
> > Endianness is not an issue as you cannot run LE and BE at the same
> > time.
> 
> AFAIK the Linux kernel for POWER technically supports that, running
> both BE and LE processes at the same time.
> 
But the distro supplied runtime libraries do not. 

Shared memory and mmapped files would require special handling. Which is
not provided.
diff mbox

Patch

diff --git a/bits/types.h b/bits/types.h
index 01753bd..47535eb 100644
--- a/bits/types.h
+++ b/bits/types.h
@@ -137,8 +137,11 @@  __STD_TYPE __RLIM_T_TYPE __rlim_t;	/* Type for resource measurement.  */
 __STD_TYPE __RLIM64_T_TYPE __rlim64_t;	/* Type for resource measurement (LFS).  */
 __STD_TYPE __ID_T_TYPE __id_t;		/* General type for IDs.  */
 __STD_TYPE __TIME_T_TYPE __time_t;	/* Seconds since the Epoch.  */
-__STD_TYPE __USECONDS_T_TYPE __useconds_t; /* Count of microseconds.  */
-__STD_TYPE __SUSECONDS_T_TYPE __suseconds_t; /* Signed count of microseconds.  */
+__STD_TYPE __TIME64_T_TYPE __time64_t;	/* Seconds since the Epoch.  */
+__STD_TYPE __USECONDS_T_TYPE __useconds_t;	/* Count of microseconds.  */
+__STD_TYPE __USECONDS64_T_TYPE __useconds64_t;	/* Count of microseconds.  */
+__STD_TYPE __SUSECONDS_T_TYPE __suseconds_t;	/* Signed count of microseconds.  */
+__STD_TYPE __SUSECONDS64_T_TYPE __suseconds64_t;/* Signed count of microseconds.  */
 
 __STD_TYPE __DADDR_T_TYPE __daddr_t;	/* The type of a disk address.  */
 __STD_TYPE __KEY_T_TYPE __key_t;	/* Type of an IPC key.  */
@@ -188,6 +191,16 @@  __STD_TYPE __SWORD_TYPE __intptr_t;
 /* Duplicate info from sys/socket.h.  */
 __STD_TYPE __U32_TYPE __socklen_t;
 
+/* Kernel time types.  */
+#ifdef __KERNEL_TIME_T_MATCHES_TIME64_T
+typedef __time64_t	__ktime_t;
+typedef __suseconds64_t	__ksuseconds_t;
+typedef __useconds64_t	__kuseconds_t;
+#else
+typedef __time_t	__ktime_t;
+typedef __suseconds_t	__ksuseconds_t;
+typedef __useconds_t	__kuseconds_t;
+#endif
 
 #undef __STD_TYPE
 
diff --git a/sysdeps/gnu/bits/utmp.h b/sysdeps/gnu/bits/utmp.h
index 2a1ffcb..fae9126 100644
--- a/sysdeps/gnu/bits/utmp.h
+++ b/sysdeps/gnu/bits/utmp.h
@@ -38,7 +38,7 @@  struct lastlog
 #ifdef __WORDSIZE_TIME64_COMPAT32
     int32_t ll_time;
 #else
-    __time_t ll_time;
+    __ktime_t ll_time;
 #endif
     char ll_line[UT_LINESIZE];
     char ll_host[UT_HOSTSIZE];
@@ -76,8 +76,8 @@  struct utmp
     int32_t tv_usec;		/* Microseconds.  */
   } ut_tv;			/* Time entry was made.  */
 #else
-  long int ut_session;		/* Session ID, used for windowing.  */
-  struct timeval ut_tv;		/* Time entry was made.  */
+  __ktime_t ut_session;		/* Session ID, used for windowing.  */
+  struct __kernel_timeval ut_tv;/* Time entry was made.  */
 #endif
 
   int32_t ut_addr_v6[4];	/* Internet address of remote host.  */
diff --git a/sysdeps/gnu/bits/utmpx.h b/sysdeps/gnu/bits/utmpx.h
index b41548b..58eeef4 100644
--- a/sysdeps/gnu/bits/utmpx.h
+++ b/sysdeps/gnu/bits/utmpx.h
@@ -74,8 +74,8 @@  struct utmpx
     __int32_t tv_usec;		/* Microseconds.  */
   } ut_tv;			/* Time entry was made.  */
 #else
-  long int ut_session;		/* Session ID, used for windowing.  */
-  struct timeval ut_tv;		/* Time entry was made.  */
+  __ktime_t ut_session;		/* Session ID, used for windowing.  */
+  struct __kernel_timeval ut_tv;/* Time entry was made.  */
 #endif
   __int32_t ut_addr_v6[4];	/* Internet address of remote host.  */
   char __glibc_reserved[20];		/* Reserved for future use.  */
diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
index 39c0c81..97ca37c 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
@@ -48,8 +48,11 @@ 
 #define	__ID_T_TYPE		__U32_TYPE
 #define __CLOCK_T_TYPE		__SLONGWORD_TYPE
 #define __TIME_T_TYPE		__SLONGWORD_TYPE
+#define __TIME64_T_TYPE		__SQUAD_TYPE
 #define __USECONDS_T_TYPE	__U32_TYPE
+#define __USECONDS64_T_TYPE	__UQUAD_TYPE
 #define __SUSECONDS_T_TYPE	__SLONGWORD_TYPE
+#define __SUSECONDS64_T_TYPE	__SQUAD_TYPE
 #define __DADDR_T_TYPE		__S32_TYPE
 #define __KEY_T_TYPE		__S32_TYPE
 #define __CLOCKID_T_TYPE	__S32_TYPE
@@ -79,8 +82,10 @@ 
 /* And for __fsbilcnt_t and __fsbilcnt64_t.  */
 # define __FSFILCNT_T_TYPE_MATCHES_FSFILCNT64_T_TYPE	1
 
+/* And for kernel time types.  */
+#define	__KERNEL_TIME_T_MATCHES_TIME64_T		1
+
 /* Number of descriptors that can fit in an `fd_set'.  */
 #define	__FD_SETSIZE		1024
 
-
 #endif /* bits/typesizes.h */
diff --git a/sysdeps/unix/sysv/linux/bits/time.h b/sysdeps/unix/sysv/linux/bits/time.h
index 87eb51f..d329ff4 100644
--- a/sysdeps/unix/sysv/linux/bits/time.h
+++ b/sysdeps/unix/sysv/linux/bits/time.h
@@ -32,6 +32,12 @@  struct timeval
     __time_t tv_sec;		/* Seconds.  */
     __suseconds_t tv_usec;	/* Microseconds.  */
   };
+
+struct __kernel_timeval
+  {
+    __ktime_t tv_sec;		/* Seconds.  */
+    __ksuseconds_t tv_usec;	/* Microseconds.  */
+  };
 # endif	/* struct timeval */
 #endif