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: <20210917061040.2270822-1-alistair.francis@opensource.wdc.com>
 <20210917061040.2270822-2-alistair.francis@opensource.wdc.com> <72990864-5ec6-1f73-efd9-61b667a172dd@collabora.com>
In-Reply-To: <72990864-5ec6-1f73-efd9-61b667a172dd@collabora.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Tue, 21 Sep 2021 10:08:22 +0200
Message-ID: <CAK8P3a3x_EyCiPDpMK54y=Rtm-Wb08ym2TNiuAZgXhYrThcWTw@mail.gmail.com>
Subject: Re: [PATCH v3 2/2] perf bench: Add support for 32-bit systems with
 64-bit time_t
To:     =?UTF-8?Q?Andr=C3=A9_Almeida?= <andrealmeid@collabora.com>
Cc:     Alistair Francis <alistair.francis@opensource.wdc.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Alistair Francis <alistair23@gmail.com>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Jiri Olsa <jolsa@redhat.com>, linux-perf-users@vger.kernel.org,
        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"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Tue, Sep 21, 2021 at 12:47 AM André Almeida
<andrealmeid@collabora.com> wrote:
>
> #if defined(__i386__) || __TIMESIZE == 32
> # define NR_gettime64 __NR_clock_gettime64
> #else
> # define NR_gettime64 __NR_clock_gettime
> #endif
>
> struct timespec64 {
>         long long tv_sec;       /* seconds */
>         long long tv_nsec;      /* nanoseconds */
> };
>
> int gettime64(clock_t clockid, struct timespec64 *tv)
> {
>         return syscall(NR_gettime64, clockid, tv);
> }
>
> Then we can just use &timeout at __NR_futex_time64 for 32bit arch and at
> __NR_futex for 64bit arch.

This is still broken when you disable CONFIG_COMPAT_32BIT_TIME,
which disables all system calls that take time32 arguments.

> This might be a simpler solution to the problem that you are facing but
> I'm not entirely sure. Also, futex's selftests do use the timeout
> argument and I think that they also won't compile in 32-bit RISC-V, so
> maybe we can start from there so we can actually test the timeout
> argument and check if it's working.

I would love to see the wrapper that Alistair wrote as part of some kernel
uapi header provided to user space. futex is used by tons of applications,
and we never had a library abstraction for it, so everyone has to do these
by hand, and they all get them slightly wrong in different ways.

We normally don't do this in kernel headers, but I think the benefits
would be far greater compared to today's situation.

      Arnd