| Message ID | 20201112195718.6070-1-anton.ivanov@cambridgegreys.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] um: add a UML specific futex implementation | expand |
On Thu, Nov 12, 2020 at 8:57 PM <anton.ivanov@cambridgegreys.com> wrote: > > From: Anton Ivanov <anton.ivanov@cambridgegreys.com> > > 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 <anton.ivanov@cambridgegreys.com> > --- > 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 <linux/futex.h> > +#include <linux/uaccess.h> > +#include <asm/errno.h> > + > + > +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 <asm/current.h> > #include <asm/page.h> > #include <kern_util.h> > +#include <asm/futex.h> > #include <os.h> > > 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...
On 10/12/2020 22:38, Richard Weinberger wrote: > On Thu, Nov 12, 2020 at 8:57 PM <anton.ivanov@cambridgegreys.com> wrote: >> From: Anton Ivanov <anton.ivanov@cambridgegreys.com> >> >> 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 <anton.ivanov@cambridgegreys.com> >> --- >> 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 <linux/futex.h> >> +#include <linux/uaccess.h> >> +#include <asm/errno.h> >> + >> + >> +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 <asm/current.h> >> #include <asm/page.h> >> #include <kern_util.h> >> +#include <asm/futex.h> >> #include <os.h> >> >> 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... > I did not try, mea culpa. It was pulling the generic before. So generic should work. I can retest this one and the others in 32 bit mode tomorrow (most of them were written to fall back to generic implementation for 32). A.
On 10/12/2020 22:38, Richard Weinberger wrote: > On Thu, Nov 12, 2020 at 8:57 PM <anton.ivanov@cambridgegreys.com> wrote: >> >> From: Anton Ivanov <anton.ivanov@cambridgegreys.com> >> >> 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 <anton.ivanov@cambridgegreys.com> >> --- >> 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 <linux/futex.h> >> +#include <linux/uaccess.h> >> +#include <asm/errno.h> >> + >> + >> +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 <asm/current.h> >> #include <asm/page.h> >> #include <kern_util.h> >> +#include <asm/futex.h> >> #include <os.h> >> >> 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 :)
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 <linux/futex.h> +#include <linux/uaccess.h> +#include <asm/errno.h> + + +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 <asm/current.h> #include <asm/page.h> #include <kern_util.h> +#include <asm/futex.h> #include <os.h> 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) +{ + int oldval, ret; + struct page *page; + pte_t *pte; + + ret = -EFAULT; + if (!access_ok(uaddr, sizeof(*uaddr))) + return -EFAULT; + preempt_disable(); + pte = maybe_map((unsigned long) uaddr, 1); + if (pte == NULL) + goto out_inuser; + + page = pte_page(*pte); + pagefault_disable(); + uaddr = page_address(page) + (((unsigned long) uaddr) & ~PAGE_MASK); + + oldval = *uaddr; + + ret = 0; + + switch (op) { + case FUTEX_OP_SET: + *uaddr = oparg; + break; + case FUTEX_OP_ADD: + *uaddr += oparg; + break; + case FUTEX_OP_OR: + *uaddr |= oparg; + break; + case FUTEX_OP_ANDN: + *uaddr &= ~oparg; + break; + case FUTEX_OP_XOR: + *uaddr ^= oparg; + break; + default: + ret = -ENOSYS; + } + pagefault_enable(); + +out_inuser: + preempt_enable(); + + if (ret == 0) + *oval = oldval; + + return ret; +} +EXPORT_SYMBOL(arch_futex_atomic_op_inuser); + +/** + * futex_atomic_cmpxchg_inatomic() - Compare and exchange the content of the + * uaddr with newval if the current value is + * oldval. + * @uval: pointer to store content of @uaddr + * @uaddr: pointer to user space address + * @oldval: old value + * @newval: new value to store to @uaddr + * + * Return: + * 0 - On success + * -EFAULT - User access resulted in a page fault + * -EAGAIN - Atomic operation was unable to complete due to contention + * -ENOSYS - Function not implemented (only if !HAVE_FUTEX_CMPXCHG) + */ + +int futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr, + u32 oldval, u32 newval) +{ + u32 val; + struct page *page; + pte_t *pte; + int ret = -EFAULT; + + if (!access_ok(uaddr, sizeof(*uaddr))) + return -EFAULT; + + preempt_disable(); + pte = maybe_map((unsigned long) uaddr, 1); + if (pte == NULL) + goto out_inatomic; + + page = pte_page(*pte); + pagefault_disable(); + uaddr = page_address(page) + (((unsigned long) uaddr) & ~PAGE_MASK); + + val = *uaddr; + + if (val == oldval) + *uaddr = newval; + + *uval = val; + pagefault_enable(); + ret = 0; + +out_inatomic: + preempt_enable(); + return ret; +} +EXPORT_SYMBOL(futex_atomic_cmpxchg_inatomic);