Re: [PATCH v2 01/16] linux: Fix vDSO macros build with time64 interfaces

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 02/01/2020 09:07, Siddhesh Poyarekar wrote:
> On 18/12/19 3:17 am, Adhemerval Zanella wrote:
>> Changes from previous version:
>>
>>   - Reinstate syscall fallback on INLINE_VSYSCALL, it simplifies
>>     when the macro is used multiple times (as for clock_gettime
>>     and clock_getres).
>>
>> --
>>
>> As indicated on libc-help [1] the ec138c67cb commit broke 32-bit
>> builds when configured with --enable-kernel=5.1 or higher.  The
>> scenario 10 from [2] might also occur in this configuration and
> 
> What is scenario 10 from [2]?

Oops, I forgot to add the link itself
https://sourceware.org/ml/libc-alpha/2019-12/msg00142.html. It is:

  10. Define __NR_clock_gettime64 and __NR_clock_gettime and provide
      a 32-bit vDSO.
      - i.e. sparc32, powerpc32

The architecture will define it has clock_gettime time32 vDSO
(HAVE_CLOCK_GETTIME_VSYSCALL) and the INLINE_VSYSCALL macro will
be defined to call the vDSO symbol.  However since the architecture
only defines it for time32 version, the macro fails because it does
not internally define the function pointer for time64.

I did not take in consideration this scenario is also valid for
32-bit architecture with --enable-kernel=5.1 (the case of the
build failure reported on libc-help), where __ASSUME_TIME64_SYSCALLS
is defined and thus it might call the time64 vDSO.

> 
>> INLINE_VSYSCALL will try to use the vDSO symbol and
>> HAVE_CLOCK_GETTIME64_VSYSCALL does not set HAVE_VSYSCALL prior its
>> usage.
>>
>> Also, there is no easy way to just enable the code to use one
>> vDSO sysmbo since the macro INLINE_VSYSCALL is redefined if
> 
> symbol

Ack.

> 
>> HAVE_VSYSCALL is set.
>>
>> Instead of adding more pre-processor handling and making the code
>> even more convoluted, this patch removes the requirement of defining
>> HAVE_VSYSCALL before including sysdep-vdso.h to enable vDSO usage.
>>
>> The INLINE_VSYSCALL is now expected to be issued inside a
>> HAVE_*_VSYSCALL check, since it will try to use the internal vDSO
>> pointers.
>>
>> Both clock_getres and clock_gettime vDSO code for time64_t were
>> removed since there is no vDSO setup code for the symbol (an
>> architecture can not set HAVE_CLOCK_GETTIME64_VSYSCALL).
>>
>> Checked on i686-linux-gnu (default and with --enable-kernel=5.1),
>> x86_64-linux-gnu, aarch64-linux-gnu, and powerpc64le-linux-gnu.
>> I also checked against a build to mips64-linux-gnu and
>> sparc64-linux-gnu.
>>
>> [1] https://sourceware.org/ml/libc-help/2019-12/msg00014.html
>> ---
>>  .../unix/sysv/linux/aarch64/gettimeofday.c    |  4 --
>>  sysdeps/unix/sysv/linux/clock_getres.c        | 36 +++++++++++-------
>>  sysdeps/unix/sysv/linux/clock_gettime.c       | 38 +++++++++++--------
>>  sysdeps/unix/sysv/linux/getcpu.c              |  9 +----
>>  .../unix/sysv/linux/powerpc/gettimeofday.c    |  4 --
>>  sysdeps/unix/sysv/linux/powerpc/time.c        |  4 --
>>  sysdeps/unix/sysv/linux/sched_getcpu.c        | 15 +++-----
>>  sysdeps/unix/sysv/linux/sysdep-vdso.h         | 34 +----------------
>>  sysdeps/unix/sysv/linux/x86/gettimeofday.c    |  4 --
>>  sysdeps/unix/sysv/linux/x86/time.c            |  8 ++--
>>  10 files changed, 58 insertions(+), 98 deletions(-)
>>
> 
> The change looks OK with the above nits.
> 
> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>