Re: [PATCH] perf bench: Add support for 32-bit systems with 64-bit time_t

From: Arnd Bergmann
Date: Thu Sep 09 2021 - 04:20:31 EST

On Thu, Sep 9, 2021 at 6:25 AM Alistair Francis
<alistair.francis@xxxxxxxxxxxxxxxxxx> wrote:
> From: Alistair Francis <alistair.francis@xxxxxxx>
> Some 32-bit architectures (such are 32-bit RISC-V) only have a 64-bit
> time_t and as such don't have the SYS_futex syscall. This patch will
> allow us to use the SYS_futex_time64 syscall on those platforms.
> This patch does not attempt to gracefully allow 32-bit architectures
> with both SYS_futex and SYS_futex_time64 to support a 64-bit time_t.
> This patch only applies to 32-bit architectures with a 64-bit time_t.
> Signed-off-by: Alistair Francis <alistair.francis@xxxxxxx>

Hi Alistair,

I know you've made this mistake before and I've pointed it out
several times. Please don't do this again, and try to fix up the
ones you already broke!

> +/**
> + * Some newer 32-bit architectures (such as RISC-V 32-bit) don't have
> + * the SYS_futex syscall and instead only have the SYS_futex_time64 call.
> + * Let's ensure that those still compile and run by just using the
> + * SYS_futex_time64 syscall. On these systems `struct timespec` will use a
> + * 64-bit time_t so the SYS_futex_time64 call will work.
> + */
> +#if !defined(SYS_futex) && defined(SYS_futex_time64)
> + #define SYS_futex SYS_futex_time64
> +#endif

This cannot work, as two system calls take different arguments: futex() takes
a __kernel_old_timespec and futex_time64() takes a __kernel_timespec.

You cannot derive anything about the ABI of the C library based on whether
the macros are defined or not. Either you convert the arguments passed into
the system call into the format expected by the kernel, or you pick the
correct system call based on sizeof(struct timespec).