Re: [RFC v2 0/4] futex2: Add new futex interface

[Andrà Almeida]

Hi

On 7/10/20 10:23 AM, Oleksandr Natalenko wrote:
> Hello.
> 
> On 09.07.2020 19:59, Andrà Almeida wrote:
>> This RFC is a followup to the previous discussion initiated from my last
>> patch "futex: Implement mechanism to wait on any of several futexes"[1].
>> As stated in the thread, the correct approach to move forward with the
>> wait multiple operation would be to create a new syscall that would have
>> all new cool features.
>>
>> The first patch adds the new interface and just translate the call for
>> the old interface, without implementing new features. The goal here is
>> to establish the interface and to check if everyone is happy with this
>> API. The rest of patches are selftests to show the interface in action.
>> I have the following questions:
>>
>> - What suggestions do you have to implement this? Start from scratch or
>> Â reuse the most code possible?
>>
>> - The interface seems correct and implements the requirements asked by
>> you?
>>
>> Those are the cool new features that this syscall should address some
>> day:
>>
>> - Operate with variable bit size futexes, not restricted to 32:
>> Â 8, 16 and 64
>>
>> - Wait on multiple futexes, using the following semantics:
>>
>> Â struct futex_wait {
>> ÂÂÂÂvoid *uaddr;
>> ÂÂÂÂunsigned long val;
>> ÂÂÂÂunsigned long flags;
>> Â };
>>
>> Â sys_futex_waitv(struct futex_wait *waiters, unsigned int nr_waiters,
>> ÂÂÂÂÂÂÂÂÂ unsigned long flags, struct __kernel_timespec *timo);
>>
>> - Have NUMA optimizations: if FUTEX_NUMA_FLAG is set, the `void *uaddr`
>> Â argument won't be a value of type u{8, 16, 32, 64} anymore, but a
>> struct
>> Â containing a NUMA node hint:
>>
>> Â struct futex32_numa {
>> ÂÂÂÂÂ u32 value __attribute__ ((aligned (8)));
>> ÂÂÂÂÂ u32 hint;
>> Â };
>>
>> Â struct futex64_numa {
>> ÂÂÂÂÂ u64 value __attribute__ ((aligned (16)));
>> ÂÂÂÂÂ u64 hint;
>> Â };
>>
>> Thanks,
>> ÂÂÂÂAndrÃ
>>
>> Changes since v1:
>> Â- The timeout argument now uses __kernel_timespec as type
>> Â- time32 interface was removed
>> Âv1: https://lore.kernel.org/patchwork/cover/1255437/
>>
>> [1] https://lore.kernel.org/patchwork/patch/1194339/
>>
>> Andrà Almeida (4):
>> Â futex2: Add new futex interface
>> Â selftests: futex: Add futex2 wake/wait test
>> Â selftests: futex: Add futex2 timeout test
>> Â selftests: futex: Add futex2 wouldblock test
>>
>> ÂMAINTAINERSÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂ 2 +-
>> Âarch/x86/entry/syscalls/syscall_32.tblÂÂÂÂÂÂÂ |ÂÂ 2 +
>> Âarch/x86/entry/syscalls/syscall_64.tblÂÂÂÂÂÂÂ |ÂÂ 2 +
>> Âinclude/linux/syscalls.hÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂ 7 ++
>> Âinclude/uapi/asm-generic/unistd.hÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂ 8 +-
>> Âinclude/uapi/linux/futex.hÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |Â 10 ++
>> Âinit/KconfigÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂ 7 ++
>> Âkernel/MakefileÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂ 1 +
>> Âkernel/futex2.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |Â 73 ++++++++++++
>> Âkernel/sys_ni.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂ 4 +
>> Âtools/include/uapi/asm-generic/unistd.hÂÂÂÂÂÂ |ÂÂ 7 +-
>> Â.../selftests/futex/functional/.gitignoreÂÂÂÂ |ÂÂ 1 +
>> Â.../selftests/futex/functional/MakefileÂÂÂÂÂÂ |ÂÂ 4 +-
>> Â.../selftests/futex/functional/futex2_wait.c | 111 ++++++++++++++++++
>> Â.../futex/functional/futex_wait_timeout.cÂÂÂÂ |Â 38 ++++--
>> Â.../futex/functional/futex_wait_wouldblock.c | 33 +++++-
>> Â.../testing/selftests/futex/functional/run.sh |ÂÂ 3 +
>> Â.../selftests/futex/include/futex2test.hÂÂÂÂÂ |Â 77 ++++++++++++
>> Â18 files changed, 373 insertions(+), 17 deletions(-)
>> Âcreate mode 100644 kernel/futex2.c
>> Âcreate mode 100644
>> tools/testing/selftests/futex/functional/futex2_wait.c
>> Âcreate mode 100644 tools/testing/selftests/futex/include/futex2test.h
> 
> What branch/tag this submission is based on please? It seems it is not a
> 5.8 but rather 5.7 since the second patch misses faccessat2() syscall
> and fails to be applied cleanly.
> 

As stated in MAINTAINERS[1], this submission is based on locking/core
branch from tip/tip[2] tree. The most updated release tag in this tree
is v5.8-rc1. My patches applied on top of locking/core are available in
my tree: https://gitlab.collabora.com/tonyk/linux/-/commits/futex2

According to 6c3c184fc420 ("tools headers API: Update faccessat2
affected files"), it seems that `faccessat2` entry at
`tools/include/uapi/asm-generic/unistd.h` was added after the syscall
was merged, so that's why 5.8-rc1 misses the syscall in this specific
file. Rebasing locking/core in 5.8-rc2 or above will fix that.

> Thanks.
> 

Thanks,
	AndrÃ

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/MAINTAINERS?h=v5.8-rc4#n7102

[2]
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=locking/core