Re: [RFC v2 03/20] y2038: linux: Provide __clock_settime64 implementation

[Lukasz Majewski <lukma at denx dot de>]

Hi Alistair,

> On Thu, Jun 27, 2019 at 7:08 AM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > Hi Arnd,
> >  
> > > On Thu, Jun 27, 2019 at 12:36 PM Lukasz Majewski <lukma@denx.de>
> > > wrote:  
> > > > > On Thu, Jun 27, 2019 at 12:55 AM Lukasz Majewski
> > > > > <lukma@denx.de>  
> > >  
> > > > After the discussion I do believe that it would be correct to
> > > > change the proposed patch [1] '__clock_settime' to:
> > > >
> > > > #if __TIMESIZE != 64  
> > >
> > > This would hide the function on 64-bit architectures, right?  
> >
> > Yes, this function would be hidden on architectures with
> > __WORDSIZE==64 (e.g. x86_64) (+ x32 ).
> >  
> > >
> > > Maybe
> > > #if (__TIMESIZE == __WORDSIZE) || (defined __x86_64__ && defined
> > > __ILP32__)
> > >
> > > or shortening that by adding a new macro
> > >
> > > #if __ASSUME_TIME32_SYSCALLS  
> >
> > Please see below comment.
> >  
> > >  
> > > > int
> > > > __clock_settime (clockid_t clock_id, const struct timespec *tp)
> > > > {
> > > > /* For archs with WORDSIZE==32, which do not support
> > > > clock_settime64 the clock_settime supporting 32 bit time ABI
> > > > will be used */  
> > >
> > > Same for the comment: __clock_settime would be used both
> > > on 64-bit architectures as the normal path, and for compatiblity
> > > with applications built without _TIME_BITS=64.  
> >
> > With the patch [1] - it would be used in such a way:
> >
> > 1. For archs with WORDSIZE==32 and __TIMESIZE==32 (legacy ones -
> > e.g. arm 32 bits)
> >
> > #if __TIMESIZE != 64
> > int
> > __clock_settime (clockid_t clock_id, const struct timespec *tp)
> > {
> >         return clock_settime(....)
> > }
> > #endif
> >
> >
> > 2. For arch with WORDSIZE==64 (and WORDSIZE_SYSCALL=64 -> x32) and
> > __TIMESIZE==64
> >
> > __clock_settime64 (clockid_t clock_id, const struct __timespec64
> > *tp) {
> > #ifdef __ASSUME_TIME64_SYSCALLS
> > # ifndef __NR_clock_settime64
> > #  define __NR_clock_settime64 __NR_clock_settime
> > # endif
> >   return INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp);
> > #else
> > ....
> >
> >
> > }
> >
> > The __ASSUME_TIME64_SYSCALLS is always defined in this case [2]
> >
> > Here the assumption is that for such architectures (like aarch64 or
> > x86_64) the __NR_clock_settime64 (if ever introduced) has the same
> > ABI as __NR_clock_settime, so there will be no need to adjust the
> > glibc code.
> >
> >
> > 3. New archs added or ones being Y2038 safe (with
> > __NR_clock_settime64 available in the kernel -> starting from 5.1+).
> >
> > For them the __ASSUME_TIME64_SYSCALLS is defined [2] and execution
> > patch from point 2. is used with potential fallback to 32 bit
> > version of clock_settime:
> >
> > __clock_settime64 (clockid_t clock_id, const struct __timespec64
> > *tp) {
> > #ifdef __ASSUME_TIME64_SYSCALLS
> > # ifndef __NR_clock_settime64
> > #  define __NR_clock_settime64 __NR_clock_settime
> > # endif
> >   return INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp);
> > #else
> > # ifdef __NR_clock_settime64
> >   int ret = INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp);
> >   if (ret == 0 || errno != ENOSYS)
> >     return ret;
> > # endif
> >   if (! in_time_t_range (tp->tv_sec))
> >     {
> >       __set_errno (EOVERFLOW);
> >       return -1;
> >     }
> >
> >   struct timespec ts32;
> >   valid_timespec64_to_timespec (tp, &ts32);
> >   return INLINE_SYSCALL_CALL (clock_settime, clock_id, &ts32);
> > #endif
> > }
> >
> > Note for the above code:
> >
> > The __NR_clock_settime64 is used whenever possible and if not
> > available the fallback to 32 bit time supporting clock_settime is
> > executed (with overflow check).  
> 
> So is this the general approach to use for all y2038 problematic
> syscalls?

There is still ongoing discussion regarding the
__ASSUME_TIME64_SYSCALLS flag semantics and the clock_settime
implementation (as an example of syscall conversion).

Until both are pulled upstream - there is still room for changes (and
of course discussion).

> 
> 
> Alistair
> 
> >
> >  
> > >  
> > > > return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp);
> > > > }
> > > > #endif
> > > >
> > > > The "__clock_settime64()" part could be left untouched.  
> > >
> > > Ok.
> > >
> > >       Arnd  
> >
> > Note:
> >
> > [1] - https://patchwork.ozlabs.org/patch/1107235/
> > [2] - https://patchwork.ozlabs.org/patch/1117100/
> >
> >
> > 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  




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: pgp_zOnrmBSwS.pgp
Description: OpenPGP digital signature