Re: [PATCH v6] y2038: Introduce __ASSUME_TIME64_SYSCALLS define

[Lukasz Majewski <lukma at denx dot de>]

Hi Joseph,

> On Wed, 29 May 2019, Lukasz Majewski wrote:
> 
> >   List of new syscalls added to v5.1. kernel (they accept data
> > based on struct timespec and timeval with 64 bit tv_sec):
> > 
> >    clock_gettime64
> >    clock_settime64
> >    clock_adjtime64
> >    clock_getres_time64
> >    clock_nanosleep_time64
> >    timer_gettime64
> >    timer_settime64
> >    timerfd_gettime64
> >    timerfd_settime64
> >    utimensat_time64
> >    pselect6_time64
> >    ppoll_time64
> >    io_pgetevents_time64
> >    recvmmsg_time64
> >    mq_timedsend_time64
> >    mq_timedreceive_time64
> >    semtimedop_time64
> >    rt_sigtimedwait_time64
> >    futex_time64
> >    sched_rr_get_interval_time64  
> 
> I do *not* think semtimedop_time64 should be included in the set of 
> syscalls. 

Ok.

> I think that if the macro is defined on systems with
> __WORDSIZE == 64, it should *only* relate to syscalls where the
> syscall semantics are *identical* for both the old syscall (on
> systems where __WORDSIZE or __SYSCALL_WORDSIZE is 64) and for the new
> one, so that no code will ever have to handle the two syscalls
> differently.  And the comment should explicitly say that it is only
> for that subset of syscalls.

I do agree.

> 
> I also think the list has to go in the comment defining the
> semantics, not just the commit message, to make clear exactly what
> syscalls are covered - and thus that it does not cover any other
> syscalls relating to time that may be added in later kernel versions.

I will move the list from the commit message to the comment in the
kernel-features.h file.

> 
> > In other words the __ASSUME_TIME64_SYSCALLS does not indicate the
> > presence of particular syscalls, but the generic ability which they
> > provide (64 bit time support).  
> 
> The whole point of __ASSUME_* macros is to indicate availability of 
> specific interfaces - syscalls, in this case.  It simply happens that
> the name of the syscalls in question depends on the architecture.

Ok.

> 
> > +/* This flag indicates support for Linux kernel syscalls, which
> > are able
> > +   to handle 64 bit time ABI. It is defined for architectures with
> > both
> > +   intrinsic 64 bit time support as well as ones gaining it with
> > new
> > +   syscalls added to Linux kernel version 5.1.  
> 
> "intrinsic 64 bit time support" is the sort of vague description that
> does not belong anywhere in code related to time in glibc.  The
> support on architectures where the syscalls have a name involving
> "64" is neither more nor less intrinsic than the support on
> architectures where the syscalls have names not involving "64".
> 
> I think you mean syscall ABIs (not architectures, this depends on the
> ABI for some architectures) for which the size of "long int" is
> 64-bit.  The size of "long int" for the syscall ABI, in turn, is
> always the same as the size of "long int" in the userspace ABI,
> except for x32.
> 

Correct. The "intrinsic 64 bit time support" was supposed to mean
exactly what you described above.

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