Re: [PATCH v3 3/6] uapi: futex: Add a futex syscall

[Arnd Bergmann]

On Fri, Nov 26, 2021 at 7:00 AM Alistair Francis
<alistair.francis@xxxxxxxxxxxxxxxxxx> wrote:
>
> From: Alistair Francis <alistair.francis@xxxxxxx>
>
> This commit adds two futex syscall wrappers that are exposed to
> userspace.
>
> Neither the kernel or glibc currently expose a futex wrapper, so
> userspace is left performing raw syscalls. This has mostly been because
> the overloading of one of the arguments makes it impossible to provide a
> single type safe function.
>
> Until recently the single syscall has worked fine. With the introduction
> of a 64-bit time_t futex call on 32-bit architectures, this has become
> more complex. The logic of handling the two possible futex syscalls is
> complex and often implemented incorrectly.
>
> This patch adds two futux syscall functions that correctly handle the

s/futux/futex/

> time_t complexity for userspace.
>
> This idea is based on previous discussions:
> https://lore.kernel.org/lkml/CAK8P3a3x_EyCiPDpMK54y=Rtm-Wb08ym2TNiuAZgXhYrThcWTw@xxxxxxxxxxxxxx/
>
> Signed-off-by: Alistair Francis <alistair.francis@xxxxxxx>

The implementation looks correct to me, the calling conventions seem
"good enough"
to me, but I wouldn't object if someone else can suggest a better interface.

One minor detail:

> +/**
> + * futex_syscall_timeout() - __NR_futex/__NR_futex_time64 syscall wrapper
> + * @uaddr:  address of first futex
> + * @op:   futex op code
> + * @val:  typically expected value of uaddr, but varies by op
> + * @timeout:  an absolute struct timespec
> + * @uaddr2: address of second futex for some ops
> + * @val3: varies by op
> + */
> +static inline int
> +__kernel_futex_syscall_timeout(volatile uint32_t *uaddr, int op, uint32_t val,
> +                     struct timespec *timeout, volatile uint32_t *uaddr2, int val3)

The two function names do not match the respective documentation.

        Arnd