On Sun, 2021-02-21 at 16:27 +0100, Philippe Gerum via Xenomai wrote:
> chensong via Xenomai <[email protected]> writes:
> 
> > > +/*
> > > + * Our representation of time at the kernel<->user interface boundary
> > > + * at the moment, until we have fully transitioned to a y2038-safe
> > > + * implementation in libcobalt.
> > > + */
> > > +struct __user_old_timespec {
> > > + long  tv_sec;
> > > + long  tv_nsec;
> > > +};
> > 
> > why not use old_timespec32, which is timespec32 representation defined
> > in include/linux/time32?
> > 
> 
> Using old_timespec32 in this context would mean that any time value
> received from userland by the core should be restricted to 32bit time_t,
> which is not what we want, at least for 64bit platforms:
> 
> include/vdso/time32.h:
> 
> struct old_timespec32 {
>       old_time32_t    tv_sec;
>       s32             tv_nsec;
> };
> 
> __user_old_timespec conveys the notion that we are generically talking
> about "the old timespec type which has a y2038 problem"; this is not
> specifically about the legacy timespec type on 32bit machines.
> 
> Since v5.4-rc6, we do have __kernel_old_timespec though, which could
> have been used instead of __user_old_timespec:

According to my information (based on
https://elixir.bootlin.com/linux/v5.5-rc1/A/ident/__kernel_old_timespec
) this is available since v5.5-rc1.

> 
> include/uapi/asm-generic/posix_types.h:
> 
> typedef long __kernel_long_t;
> 
> include/uapi/linux/time_types.h:
> 
> struct __kernel_old_timespec {
>       __kernel_long_t         tv_sec;         /* seconds */
>       long                    tv_nsec;        /* nanoseconds */
> };
>  
> so I went for adding __user_old_timespec to the Cobalt UAPI. This may be
> seen as a temporary option until stricter requirements on the minimum
> toolchain support for building Cobalt is decided.

__kernel_old_timespec is part of the "common" Linux uapi, so not x86
specific. Could you explain the problem you know of (or even see) with
some of the toolchains? Sounds strange...

> 
> > > 
> > >   COBALT_SYSCALL(clock_settime, current,
> > > -        (clockid_t clock_id, const struct timespec __user *u_ts))
> > > +        (clockid_t clock_id, const struct __user_old_timespec __user 
> > > *u_ts))
> > 
> > I planned to still use timespec as the argument, tv_sec is defined as
> > long, 32bit long in 32bit system, 64bit long in 64bit system, use 
> > timespec64 in implementation.
> > 
> 
> Keeping struct timespec for the argument does not seem the right thing
> to do, as this type has been phased out from recent kernels, precisely
> to stop spreading ambiguity wrt 32/64 bit time_t. It is only available
> from the UAPI section as a compat tweak for user code, not for kernel
> code.

We would need a definition for older kernels anyway, but why are we not
allowed to re-use the definition from the uapi here?

> 
> What should be done wrt addressing the y2038 issue fully is beyond this
> patch series. __user_old_timespec is merely an annotation on the
> existing implementation in order to highlight the kernel<->user
> interface boundary, which may help in fully addressing y2038 later on.
> 
> Eventually, it should be decided whether a legacy timespec32 should
> still be supported for 32bit applications building in dual-time
> configurations (time32_t and time64_t), or single-time configuration
> should be the norm with user-space using time64_t exclusively. This is
> beyond the scope of this patch series to decide of the way to go.

Let's discuss that within the next Community Meeting Minutes. When we
started scanning the code base to get an overview of all the things
that have to be done we assumed that we would need to stay fully
backward compatible and "single time_t" defined by the libc that is
beeing used. That may have changed in the meantime and we could take
other patterns into account as well.

Reply via email to