Re: [PATCH v2 2/7] y2038: Introduce __ASSUME_64BIT_TIME define

[Lukasz Majewski <lukma at denx dot de>]

Hi Stepan,

Thank you for your reply.

> 30.04.2019 в 11:05:05 +0200 Lukasz Majewski написал:
> > #if __TIMESIZE != 64
> > # if __LINUX_KERNEL_VERSION >= 0x050100
> > #  define __ASSUME_64BIT_TIME 1
> > # endif
> > #endif  
> 
> I think __WORDSIZE would be more appropriate here than __TIMESIZE.
> 

Yes. I do agree.

> 
> > IMHO, the abstraction would be:
> > 
> > 1. The __ASSUME_64BIT_TIME is _never_ defined for 64 bit native
> > systems
> > 
> > 2. It is defined by default in:
> > sysdeps/unix/sysv/linux/kernel-features.h for 32 bit systems (and
> > the actual presence of the syscall is decided upon definitions of
> > __NR_xxx* (i.e. # ifdef __NR_clock_settime64).  
> 
> I think that __NR_clock_settime64 should be used unconditionally when
> __ASSUME_64BIT_TIME is defined.

Could you clarify it a bit?

In the code as proposed in:
https://patchwork.ozlabs.org/patch/1092583/

The call to clock_settime64 is protected with # ifdef
__NR_clock_settime64 - otherwise we do a fallback to (32 bit)
clock_settime.

Moreover, the # ifdef __ASSUME_64BIT_TIME provides a fallback path if
kernel version is older than 5.1.

> 
> (__ASSUME_TIME64_SYSCALLS would probably be better name.)

I do tend to agree. The __ASSUME_TIME64_SYSCALLS is more descriptive
than __ASSUME_64BIT_TIME.

> 
> 
> > As those syscalls are provided on almost every 32 bit system now
> > (5.1-rc6):
> > git grep -n "clock_settime64"
> > 
> > gives support for: arm, arm64 (compat mode), m68k, microblaze, mips,
> > parisc, powerpc, s390, sh, sparc, x86, xtensa
> > 
> > So it would be reasonable to just add this __ASSUME definition code
> > to sysdeps/unix/sysv/linux/kernel-features.h and #undef it for
> > architectures not supporting it (i.e. c-sky and riscv).  
> 
> I believe that the only 32-bit architecture without
> __NR_clock_settime64 is x32. 

Ok, I see. 

Please correct me - would it be feasible to just #undef
__ASSYME_TIME64_SYSCALLS for x32 ?

> While newer 32-bit architectures like
> riscv do not have __NR_clock_settime:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d4c08b9776b392e20efc6198ebe1bc8ec1911d9b

Then it shall use clock_settime64 from the outset if support added.

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