Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210909042543.1982893-1-alistair.francis@opensource.wdc.com>
In-Reply-To: <20210909042543.1982893-1-alistair.francis@opensource.wdc.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Thu, 9 Sep 2021 10:20:10 +0200
Message-ID: <CAK8P3a1Udjzy=M4Vs8pGCKdedwtv-vPRjxbTcvTL6kP=yXSs0A@mail.gmail.com>
Subject: Re: [PATCH] perf bench: Add support for 32-bit systems with 64-bit time_t
To:     Alistair Francis <alistair.francis@opensource.wdc.com>
Cc:     linux-riscv <linux-riscv@lists.infradead.org>,
        linux-perf-users@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Alistair Francis <alistair23@gmail.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Jiri Olsa <jolsa@redhat.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Davidlohr Bueso <dave@stgolabs.net>,
        Darren Hart <dvhart@infradead.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Atish Patra <atish.patra@wdc.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Alistair Francis <alistair.francis@wdc.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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