From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ivanoab7.miniserver.com ([37.128.132.42] helo=www.kot-begemot.co.uk) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1knf7z-00023K-Ud for linux-um@lists.infradead.org; Fri, 11 Dec 2020 09:54:21 +0000 From: Anton Ivanov Subject: Re: [PATCH v2] um: add a UML specific futex implementation References: <20201112195718.6070-1-anton.ivanov@cambridgegreys.com> Message-ID: Date: Fri, 11 Dec 2020 09:54:14 +0000 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-um" Errors-To: linux-um-bounces+geert=linux-m68k.org@lists.infradead.org To: Richard Weinberger Cc: Richard Weinberger , linux-um On 10/12/2020 22:38, Richard Weinberger wrote: > On Thu, Nov 12, 2020 at 8:57 PM wrote: >> >> From: Anton Ivanov >> >> The generic asm futex implementation emulates atomic access to >> memory by doing a get_user followed by put_user. These translate >> to two mapping operations on UML with paging enabled in the >> meantime. This, in turn may end up changing interrupts, >> invoking the signal loop, etc. >> >> This replaces the generic implementation by a mapping followed >> by an operation on the mapped segment. It is for now limited >> to 64 bit as the 32 bit may also need to take care of highmem, >> etc. >> >> Signed-off-by: Anton Ivanov >> --- >> arch/um/include/asm/futex.h | 20 ++++++ >> arch/um/kernel/skas/uaccess.c | 119 ++++++++++++++++++++++++++++++++++ >> 2 files changed, 139 insertions(+) >> create mode 100644 arch/um/include/asm/futex.h >> >> diff --git a/arch/um/include/asm/futex.h b/arch/um/include/asm/futex.h >> new file mode 100644 >> index 000000000000..9af35ab66b6e >> --- /dev/null >> +++ b/arch/um/include/asm/futex.h >> @@ -0,0 +1,20 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _ASM_GENERIC_FUTEX_H >> +#define _ASM_GENERIC_FUTEX_H >> + >> +#ifdef CONFIG_64BIT >> + >> +#include >> +#include >> +#include >> + >> + >> +extern int arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr); >> +extern int futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr, >> + u32 oldval, u32 newval); >> + >> +#else >> +#include <"asm-generic/futex.h"> >> +#endif /* CONFIG_64BIT */ >> + >> +#endif >> diff --git a/arch/um/kernel/skas/uaccess.c b/arch/um/kernel/skas/uaccess.c >> index 2dec915abe6f..21a8f2b283bb 100644 >> --- a/arch/um/kernel/skas/uaccess.c >> +++ b/arch/um/kernel/skas/uaccess.c >> @@ -11,6 +11,7 @@ >> #include >> #include >> #include >> +#include >> #include >> >> pte_t *virt_to_pte(struct mm_struct *mm, unsigned long addr) >> @@ -248,3 +249,121 @@ long __strnlen_user(const void __user *str, long len) >> return 0; >> } >> EXPORT_SYMBOL(__strnlen_user); >> + >> +/** >> + * arch_futex_atomic_op_inuser() - Atomic arithmetic operation with constant >> + * argument and comparison of the previous >> + * futex value with another constant. >> + * >> + * @encoded_op: encoded operation to execute >> + * @uaddr: pointer to user space address >> + * >> + * Return: >> + * 0 - On success >> + * -EFAULT - User access resulted in a page fault >> + * -EAGAIN - Atomic operation was unable to complete due to contention >> + * -ENOSYS - Operation not supported >> + */ >> +int arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr) > > Does this even build in 32bits mode? > AFAICT the generic futex implementation has > arch_futex_atomic_op_inuser() as static inline... > It's an inline, but it includes two very slow non-inline calls: https://elixir.bootlin.com/linux/latest/source/include/asm-generic/futex.h#L39 if (unlikely(get_user(oldval, uaddr) != 0)) and https://elixir.bootlin.com/linux/latest/source/include/asm-generic/futex.h#L65 if (ret == 0 && unlikely(put_user(tmp, uaddr) != 0)) So better to have not inline, but actually fast and done in a single atomic operation on the target memory location. I will fix all 32 bit and resubmit (and promise to test it on 32 next time :) -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um