Re: [RFC PATCH] AARCH64/ILP32: introduce kernel time types

[Yury Norov <ynorov at caviumnetworks dot com>]

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