On Thu, Oct 14, 2021 at 7:55 AM Alistair Francis
[off-list ref] wrote:

From: Alistair Francis <redacted>

Convert the futex selftests to only use a 64-bit time_t. On 64-bit
architectures this isn't a functional change. On 32-bit architectures
we now only perform 64-bit time_t syscalls (__NR_futex_time64) and
use a struct timespec64.

This won't work on kernels before 5.1, but as perf is tied to the kernel
that's ok.

This allows the tests to run and pass on RISC-V 32-bit.

Signed-off-by: Alistair Francis <redacted>

This looks correct to me, two minor comments:

+struct timespec64 {
+       long long tv_sec;       /* seconds */
+       long long tv_nsec;      /* nanoseconds */
+};

This is a bit different from the normal timespec definition, which has to
use a '__kernel_long_t' tv_nsec for POSIX compliance. The difference
is harmless, because the bit layout has the lower 32 bits in the same
position, and the kernel zeroes the upper 32 bits on the syscall boundary.

I would just use

#define timespec64 __kernel_timespec

for the same effect, since that is what __kernel_timespec is meant for.

+#if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
+# define futex(uaddr, op, val, timeout, uaddr2, val3, opflags) \
+       syscall(__NR_futex, uaddr, op | opflags, val, timeout, uaddr2, val3)
+#else
+# define futex(uaddr, op, val, timeout, uaddr2, val3, opflags) \
+       syscall(__NR_futex_time64, uaddr, op | opflags, val, timeout, uaddr2, val3)
+#endif

The check for x32 user space looks correct here, but as I commented in
the other patch, I would keep it simple and use futex_time64() on all 32-bit
ABIs. There are approximately zero users of x32, and they don't benefit from
being able to run new perf binaries on pre-5.1 kernels when everyone else
can't.

        Arnd