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

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

[Alistair Francis]

From: Alistair Francis <alistair.francis@wdc.com>

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@wdc.com>
---
 tools/perf/bench/futex.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/bench/futex.h b/tools/perf/bench/futex.h
index b3853aac3021..912342d7f594 100644
--- a/tools/perf/bench/futex.h
+++ b/tools/perf/bench/futex.h
@@ -27,6 +27,17 @@ struct bench_futex_parameters {
 	unsigned int nrequeue;
 };
 
+/**
+ * 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
+
 /**
  * futex() - SYS_futex syscall wrapper
  * @uaddr:	address of first futex
-- 
2.31.1

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

[Arnd Bergmann]

On Thu, Sep 9, 2021 at 6:25 AM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> 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@wdc.com>

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).

       Arnd

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

[Alistair Francis]

On Thu, Sep 9, 2021 at 6:20 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Sep 9, 2021 at 6:25 AM Alistair Francis
> <alistair.francis@opensource.wdc.com> wrote:
> >
> > From: Alistair Francis <alistair.francis@wdc.com>
> >
> > 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@wdc.com>
>
> 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).

Thanks Arnd. Sorry I hadn't looked at this in a while and forgot that
it's more complex.

I have some patches to fix this up. I'll send them later today.

Alistair

>
>        Arnd