* [PATCH 1/3] futex: remove duplicated code
@ 2017-03-03 12:27 Jiri Slaby
2017-03-03 14:08 ` Heiko Carstens
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Jiri Slaby @ 2017-03-03 12:27 UTC (permalink / raw)
To: linux-snps-arc
There is code duplicated over all architecture's headers for
futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
and comparison of the result.
Remove this duplication and leave up to the arches only the needed
assembly which is now in arch_futex_atomic_op_inuser.
Note that s390 removed access_ok check in d12a29703 ("s390/uaccess:
remove pointless access_ok() checks") as access_ok there returns true.
We introduce it back to the helper for the sake of simplicity (it gets
optimized away anyway).
Signed-off-by: Jiri Slaby <jslaby at suse.cz>
Cc: Richard Henderson <rth at twiddle.net>
Cc: Ivan Kokshaysky <ink at jurassic.park.msu.ru>
Cc: Matt Turner <mattst88 at gmail.com>
Cc: Vineet Gupta <vgupta at synopsys.com>
Cc: Russell King <linux at armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas at arm.com>
Cc: Will Deacon <will.deacon at arm.com>
Cc: Richard Kuo <rkuo at codeaurora.org>
Cc: Tony Luck <tony.luck at intel.com>
Cc: Fenghua Yu <fenghua.yu at intel.com>
Cc: Michal Simek <monstr at monstr.eu>
Cc: Ralf Baechle <ralf at linux-mips.org>
Cc: Jonas Bonn <jonas at southpole.se>
Cc: Stefan Kristiansson <stefan.kristiansson at saunalahti.fi>
Cc: Stafford Horne <shorne at gmail.com>
Cc: "James E.J. Bottomley" <jejb at parisc-linux.org>
Cc: Helge Deller <deller at gmx.de>
Cc: Benjamin Herrenschmidt <benh at kernel.crashing.org>
Cc: Paul Mackerras <paulus at samba.org>
Cc: Michael Ellerman <mpe at ellerman.id.au>
Cc: Martin Schwidefsky <schwidefsky at de.ibm.com>
Cc: Heiko Carstens <heiko.carstens at de.ibm.com>
Cc: Yoshinori Sato <ysato at users.sourceforge.jp>
Cc: Rich Felker <dalias at libc.org>
Cc: "David S. Miller" <davem at davemloft.net>
Cc: Chris Metcalf <cmetcalf at mellanox.com>
Cc: Thomas Gleixner <tglx at linutronix.de>
Cc: Ingo Molnar <mingo at redhat.com>
Cc: "H. Peter Anvin" <hpa at zytor.com>
Cc: Chris Zankel <chris at zankel.net>
Cc: Max Filippov <jcmvbkbc at gmail.com>
Cc: Arnd Bergmann <arnd at arndb.de>
Cc: <x86 at kernel.org>
Cc: <linux-alpha at vger.kernel.org>
Cc: <linux-kernel at vger.kernel.org>
Cc: <linux-snps-arc at lists.infradead.org>
Cc: <linux-arm-kernel at lists.infradead.org>
Cc: <linux-hexagon at vger.kernel.org>
Cc: <linux-ia64 at vger.kernel.org>
Cc: <linux-mips at linux-mips.org>
Cc: <openrisc at lists.librecores.org>
Cc: <linux-parisc at vger.kernel.org>
Cc: <linuxppc-dev at lists.ozlabs.org>
Cc: <linux-s390 at vger.kernel.org>
Cc: <linux-sh at vger.kernel.org>
Cc: <sparclinux at vger.kernel.org>
Cc: <linux-xtensa at linux-xtensa.org>
Cc: <linux-arch at vger.kernel.org>
---
arch/alpha/include/asm/futex.h | 26 ++++---------------
arch/arc/include/asm/futex.h | 40 ++++-------------------------
arch/arm/include/asm/futex.h | 26 +++----------------
arch/arm64/include/asm/futex.h | 26 +++----------------
arch/frv/include/asm/futex.h | 3 ++-
arch/frv/kernel/futex.c | 27 +++-----------------
arch/hexagon/include/asm/futex.h | 38 +++-------------------------
arch/ia64/include/asm/futex.h | 25 +++----------------
arch/microblaze/include/asm/futex.h | 38 +++-------------------------
arch/mips/include/asm/futex.h | 25 +++----------------
arch/openrisc/include/asm/futex.h | 39 +++--------------------------
arch/parisc/include/asm/futex.h | 26 +++----------------
arch/powerpc/include/asm/futex.h | 26 ++++---------------
arch/s390/include/asm/futex.h | 23 ++++-------------
arch/sh/include/asm/futex.h | 26 +++----------------
arch/sparc/include/asm/futex_64.h | 26 ++++---------------
arch/tile/include/asm/futex.h | 40 ++++-------------------------
arch/x86/include/asm/futex.h | 40 ++++-------------------------
arch/xtensa/include/asm/futex.h | 27 ++++----------------
include/asm-generic/futex.h | 50 +++++++------------------------------
kernel/futex.c | 36 ++++++++++++++++++++++++++
21 files changed, 127 insertions(+), 506 deletions(-)
diff --git a/arch/alpha/include/asm/futex.h b/arch/alpha/include/asm/futex.h
index f939794363ac..56474690e685 100644
--- a/arch/alpha/include/asm/futex.h
+++ b/arch/alpha/include/asm/futex.h
@@ -29,18 +29,10 @@
: "r" (uaddr), "r"(oparg) \
: "memory")
-static inline int futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
+ u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
pagefault_disable();
@@ -66,17 +58,9 @@ static inline int futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/arc/include/asm/futex.h b/arch/arc/include/asm/futex.h
index 11e1b1f3acda..eb887dd13e74 100644
--- a/arch/arc/include/asm/futex.h
+++ b/arch/arc/include/asm/futex.h
@@ -73,20 +73,11 @@
#endif
-static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
+ u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
- return -EFAULT;
-
#ifndef CONFIG_ARC_HAS_LLSC
preempt_disable(); /* to guarantee atomic r-m-w of futex op */
#endif
@@ -118,30 +109,9 @@ static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
preempt_enable();
#endif
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ:
- ret = (oldval == cmparg);
- break;
- case FUTEX_OP_CMP_NE:
- ret = (oldval != cmparg);
- break;
- case FUTEX_OP_CMP_LT:
- ret = (oldval < cmparg);
- break;
- case FUTEX_OP_CMP_GE:
- ret = (oldval >= cmparg);
- break;
- case FUTEX_OP_CMP_LE:
- ret = (oldval <= cmparg);
- break;
- case FUTEX_OP_CMP_GT:
- ret = (oldval > cmparg);
- break;
- default:
- ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/arm/include/asm/futex.h b/arch/arm/include/asm/futex.h
index 6795368ad023..cc414382dab4 100644
--- a/arch/arm/include/asm/futex.h
+++ b/arch/arm/include/asm/futex.h
@@ -128,20 +128,10 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
#endif /* !SMP */
static inline int
-futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
+arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret, tmp;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
-
#ifndef CONFIG_SMP
preempt_disable();
#endif
@@ -172,17 +162,9 @@ futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
preempt_enable();
#endif
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
index 85c4a8981d47..5bb2fd4674e7 100644
--- a/arch/arm64/include/asm/futex.h
+++ b/arch/arm64/include/asm/futex.h
@@ -48,20 +48,10 @@ do { \
} while (0)
static inline int
-futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
+arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret, tmp;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
-
pagefault_disable();
switch (op) {
@@ -91,17 +81,9 @@ futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/frv/include/asm/futex.h b/arch/frv/include/asm/futex.h
index 2e1da71e27a4..ab346f5f8820 100644
--- a/arch/frv/include/asm/futex.h
+++ b/arch/frv/include/asm/futex.h
@@ -7,7 +7,8 @@
#include <asm/errno.h>
#include <linux/uaccess.h>
-extern int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr);
+extern int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
+ u32 __user *uaddr);
static inline int
futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
diff --git a/arch/frv/kernel/futex.c b/arch/frv/kernel/futex.c
index d155ca9e5098..37f7b2bf7f73 100644
--- a/arch/frv/kernel/futex.c
+++ b/arch/frv/kernel/futex.c
@@ -186,20 +186,10 @@ static inline int atomic_futex_op_xchg_xor(int oparg, u32 __user *uaddr, int *_o
/*
* do the futex operations
*/
-int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+int arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
-
pagefault_disable();
switch (op) {
@@ -225,18 +215,9 @@ int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS; break;
- }
- }
+ if (!ret)
+ *oval = oldval;
return ret;
-} /* end futex_atomic_op_inuser() */
+} /* end arch_futex_atomic_op_inuser() */
diff --git a/arch/hexagon/include/asm/futex.h b/arch/hexagon/include/asm/futex.h
index 7e597f8434da..c607b77c8215 100644
--- a/arch/hexagon/include/asm/futex.h
+++ b/arch/hexagon/include/asm/futex.h
@@ -31,18 +31,9 @@
static inline int
-futex_atomic_op_inuser(int encoded_op, int __user *uaddr)
+arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
- return -EFAULT;
pagefault_disable();
@@ -72,30 +63,9 @@ futex_atomic_op_inuser(int encoded_op, int __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ:
- ret = (oldval == cmparg);
- break;
- case FUTEX_OP_CMP_NE:
- ret = (oldval != cmparg);
- break;
- case FUTEX_OP_CMP_LT:
- ret = (oldval < cmparg);
- break;
- case FUTEX_OP_CMP_GE:
- ret = (oldval >= cmparg);
- break;
- case FUTEX_OP_CMP_LE:
- ret = (oldval <= cmparg);
- break;
- case FUTEX_OP_CMP_GT:
- ret = (oldval > cmparg);
- break;
- default:
- ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/ia64/include/asm/futex.h b/arch/ia64/include/asm/futex.h
index 76acbcd5c060..6d67dc1eaf2b 100644
--- a/arch/ia64/include/asm/futex.h
+++ b/arch/ia64/include/asm/futex.h
@@ -45,18 +45,9 @@ do { \
} while (0)
static inline int
-futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
+arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (! access_ok (VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
pagefault_disable();
@@ -84,17 +75,9 @@ futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/microblaze/include/asm/futex.h b/arch/microblaze/include/asm/futex.h
index 01848f056f43..a9dad9e5e132 100644
--- a/arch/microblaze/include/asm/futex.h
+++ b/arch/microblaze/include/asm/futex.h
@@ -29,18 +29,9 @@
})
static inline int
-futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
pagefault_disable();
@@ -66,30 +57,9 @@ futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ:
- ret = (oldval == cmparg);
- break;
- case FUTEX_OP_CMP_NE:
- ret = (oldval != cmparg);
- break;
- case FUTEX_OP_CMP_LT:
- ret = (oldval < cmparg);
- break;
- case FUTEX_OP_CMP_GE:
- ret = (oldval >= cmparg);
- break;
- case FUTEX_OP_CMP_LE:
- ret = (oldval <= cmparg);
- break;
- case FUTEX_OP_CMP_GT:
- ret = (oldval > cmparg);
- break;
- default:
- ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/mips/include/asm/futex.h b/arch/mips/include/asm/futex.h
index 1de190bdfb9c..a9e61ea54ca9 100644
--- a/arch/mips/include/asm/futex.h
+++ b/arch/mips/include/asm/futex.h
@@ -83,18 +83,9 @@
}
static inline int
-futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (! access_ok (VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
pagefault_disable();
@@ -125,17 +116,9 @@ futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/openrisc/include/asm/futex.h b/arch/openrisc/include/asm/futex.h
index 778087341977..8fed278a24b8 100644
--- a/arch/openrisc/include/asm/futex.h
+++ b/arch/openrisc/include/asm/futex.h
@@ -30,20 +30,10 @@
})
static inline int
-futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
-
pagefault_disable();
switch (op) {
@@ -68,30 +58,9 @@ futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ:
- ret = (oldval == cmparg);
- break;
- case FUTEX_OP_CMP_NE:
- ret = (oldval != cmparg);
- break;
- case FUTEX_OP_CMP_LT:
- ret = (oldval < cmparg);
- break;
- case FUTEX_OP_CMP_GE:
- ret = (oldval >= cmparg);
- break;
- case FUTEX_OP_CMP_LE:
- ret = (oldval <= cmparg);
- break;
- case FUTEX_OP_CMP_GT:
- ret = (oldval > cmparg);
- break;
- default:
- ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/parisc/include/asm/futex.h b/arch/parisc/include/asm/futex.h
index ac8bd586ace8..06a1a883c72f 100644
--- a/arch/parisc/include/asm/futex.h
+++ b/arch/parisc/include/asm/futex.h
@@ -32,22 +32,12 @@ _futex_spin_unlock_irqrestore(u32 __user *uaddr, unsigned long int *flags)
}
static inline int
-futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
+arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
{
unsigned long int flags;
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval, ret;
u32 tmp;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(*uaddr)))
- return -EFAULT;
-
_futex_spin_lock_irqsave(uaddr, &flags);
pagefault_disable();
@@ -85,17 +75,9 @@ futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
pagefault_enable();
_futex_spin_unlock_irqrestore(uaddr, &flags);
- if (ret == 0) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/powerpc/include/asm/futex.h b/arch/powerpc/include/asm/futex.h
index eaada6c92344..719ed9b61ea7 100644
--- a/arch/powerpc/include/asm/futex.h
+++ b/arch/powerpc/include/asm/futex.h
@@ -29,18 +29,10 @@
: "b" (uaddr), "i" (-EFAULT), "r" (oparg) \
: "cr0", "memory")
-static inline int futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
+ u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (! access_ok (VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
pagefault_disable();
@@ -66,17 +58,9 @@ static inline int futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/s390/include/asm/futex.h b/arch/s390/include/asm/futex.h
index a4811aa0304d..8f8eec9e1198 100644
--- a/arch/s390/include/asm/futex.h
+++ b/arch/s390/include/asm/futex.h
@@ -21,17 +21,12 @@
: "0" (-EFAULT), "d" (oparg), "a" (uaddr), \
"m" (*uaddr) : "cc");
-static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
+ u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, newval, ret;
load_kernel_asce();
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
pagefault_disable();
switch (op) {
@@ -60,17 +55,9 @@ static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
}
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/sh/include/asm/futex.h b/arch/sh/include/asm/futex.h
index d0078747d308..8f8cf941a8cd 100644
--- a/arch/sh/include/asm/futex.h
+++ b/arch/sh/include/asm/futex.h
@@ -27,21 +27,12 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
return atomic_futex_op_cmpxchg_inatomic(uval, uaddr, oldval, newval);
}
-static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval,
+ u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- u32 oparg = (encoded_op << 8) >> 20;
- u32 cmparg = (encoded_op << 20) >> 20;
u32 oldval, newval, prev;
int ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
-
pagefault_disable();
do {
@@ -80,17 +71,8 @@ static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = ((int)oldval < (int)cmparg); break;
- case FUTEX_OP_CMP_GE: ret = ((int)oldval >= (int)cmparg); break;
- case FUTEX_OP_CMP_LE: ret = ((int)oldval <= (int)cmparg); break;
- case FUTEX_OP_CMP_GT: ret = ((int)oldval > (int)cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
return ret;
}
diff --git a/arch/sparc/include/asm/futex_64.h b/arch/sparc/include/asm/futex_64.h
index 4e899b0dabf7..1cfd89d92208 100644
--- a/arch/sparc/include/asm/futex_64.h
+++ b/arch/sparc/include/asm/futex_64.h
@@ -29,22 +29,14 @@
: "r" (uaddr), "r" (oparg), "i" (-EFAULT) \
: "memory")
-static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
+ u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret, tem;
- if (unlikely(!access_ok(VERIFY_WRITE, uaddr, sizeof(u32))))
- return -EFAULT;
if (unlikely((((unsigned long) uaddr) & 0x3UL)))
return -EINVAL;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
pagefault_disable();
switch (op) {
@@ -69,17 +61,9 @@ static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/tile/include/asm/futex.h b/arch/tile/include/asm/futex.h
index e64a1b75fc38..83c1e639b411 100644
--- a/arch/tile/include/asm/futex.h
+++ b/arch/tile/include/asm/futex.h
@@ -106,12 +106,9 @@
lock = __atomic_hashed_lock((int __force *)uaddr)
#endif
-static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval,
+ u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int uninitialized_var(val), ret;
__futex_prolog();
@@ -119,12 +116,6 @@ static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
/* The 32-bit futex code makes this assumption, so validate it here. */
BUILD_BUG_ON(sizeof(atomic_t) != sizeof(int));
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
-
pagefault_disable();
switch (op) {
case FUTEX_OP_SET:
@@ -148,30 +139,9 @@ static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
}
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ:
- ret = (val == cmparg);
- break;
- case FUTEX_OP_CMP_NE:
- ret = (val != cmparg);
- break;
- case FUTEX_OP_CMP_LT:
- ret = (val < cmparg);
- break;
- case FUTEX_OP_CMP_GE:
- ret = (val >= cmparg);
- break;
- case FUTEX_OP_CMP_LE:
- ret = (val <= cmparg);
- break;
- case FUTEX_OP_CMP_GT:
- ret = (val > cmparg);
- break;
- default:
- ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = val;
+
return ret;
}
diff --git a/arch/x86/include/asm/futex.h b/arch/x86/include/asm/futex.h
index b4c1f5453436..f4dc9b63bdda 100644
--- a/arch/x86/include/asm/futex.h
+++ b/arch/x86/include/asm/futex.h
@@ -41,20 +41,11 @@
"+m" (*uaddr), "=&r" (tem) \
: "r" (oparg), "i" (-EFAULT), "1" (0))
-static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
+ u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret, tem;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
-
pagefault_disable();
switch (op) {
@@ -80,30 +71,9 @@ static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ:
- ret = (oldval == cmparg);
- break;
- case FUTEX_OP_CMP_NE:
- ret = (oldval != cmparg);
- break;
- case FUTEX_OP_CMP_LT:
- ret = (oldval < cmparg);
- break;
- case FUTEX_OP_CMP_GE:
- ret = (oldval >= cmparg);
- break;
- case FUTEX_OP_CMP_LE:
- ret = (oldval <= cmparg);
- break;
- case FUTEX_OP_CMP_GT:
- ret = (oldval > cmparg);
- break;
- default:
- ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/arch/xtensa/include/asm/futex.h b/arch/xtensa/include/asm/futex.h
index b39531babec0..eaaf1ebcc7a4 100644
--- a/arch/xtensa/include/asm/futex.h
+++ b/arch/xtensa/include/asm/futex.h
@@ -44,18 +44,10 @@
: "r" (uaddr), "I" (-EFAULT), "r" (oparg) \
: "memory")
-static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
+ u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
#if !XCHAL_HAVE_S32C1I
return -ENOSYS;
@@ -89,19 +81,10 @@ static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (ret)
- return ret;
+ if (!ret)
+ *oval = oldval;
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: return (oldval == cmparg);
- case FUTEX_OP_CMP_NE: return (oldval != cmparg);
- case FUTEX_OP_CMP_LT: return (oldval < cmparg);
- case FUTEX_OP_CMP_GE: return (oldval >= cmparg);
- case FUTEX_OP_CMP_LE: return (oldval <= cmparg);
- case FUTEX_OP_CMP_GT: return (oldval > cmparg);
- }
-
- return -ENOSYS;
+ return ret;
}
static inline int
diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h
index bf2d34c9d804..f0d8b1c51343 100644
--- a/include/asm-generic/futex.h
+++ b/include/asm-generic/futex.h
@@ -13,7 +13,7 @@
*/
/**
- * futex_atomic_op_inuser() - Atomic arithmetic operation with constant
+ * arch_futex_atomic_op_inuser() - Atomic arithmetic operation with constant
* argument and comparison of the previous
* futex value with another constant.
*
@@ -25,18 +25,11 @@
* <0 - On error
*/
static inline int
-futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval, ret;
u32 tmp;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
preempt_disable();
pagefault_disable();
@@ -74,17 +67,9 @@ futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
pagefault_enable();
preempt_enable();
- if (ret == 0) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (ret == 0)
+ *oval = oldval;
+
return ret;
}
@@ -126,18 +111,9 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
#else
static inline int
-futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
+arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
{
- int op = (encoded_op >> 28) & 7;
- int cmp = (encoded_op >> 24) & 15;
- int oparg = (encoded_op << 8) >> 20;
- int cmparg = (encoded_op << 20) >> 20;
int oldval = 0, ret;
- if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
- oparg = 1 << oparg;
-
- if (! access_ok (VERIFY_WRITE, uaddr, sizeof(u32)))
- return -EFAULT;
pagefault_disable();
@@ -153,17 +129,9 @@ futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
pagefault_enable();
- if (!ret) {
- switch (cmp) {
- case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
- case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
- case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
- case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
- case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
- case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
- default: ret = -ENOSYS;
- }
- }
+ if (!ret)
+ *oval = oldval;
+
return ret;
}
diff --git a/kernel/futex.c b/kernel/futex.c
index b687cb22301c..c5ff9850952f 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -1457,6 +1457,42 @@ futex_wake(u32 __user *uaddr, unsigned int flags, int nr_wake, u32 bitset)
return ret;
}
+static int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
+{
+ int op = (encoded_op >> 28) & 7;
+ int cmp = (encoded_op >> 24) & 15;
+ int oparg = (encoded_op << 8) >> 20;
+ int cmparg = (encoded_op << 20) >> 20;
+ int oldval, ret;
+
+ if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
+ oparg = 1 << oparg;
+
+ if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
+ return -EFAULT;
+
+ ret = arch_futex_atomic_op_inuser(op, oparg, &oldval, uaddr);
+ if (ret)
+ return ret;
+
+ switch (cmp) {
+ case FUTEX_OP_CMP_EQ:
+ return oldval == cmparg;
+ case FUTEX_OP_CMP_NE:
+ return oldval != cmparg;
+ case FUTEX_OP_CMP_LT:
+ return oldval < cmparg;
+ case FUTEX_OP_CMP_GE:
+ return oldval >= cmparg;
+ case FUTEX_OP_CMP_LE:
+ return oldval <= cmparg;
+ case FUTEX_OP_CMP_GT:
+ return oldval > cmparg;
+ default:
+ return -ENOSYS;
+ }
+}
+
/*
* Wake up all waiters hashed on the physical page that is mapped
* to this virtual address:
--
2.12.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 1/3] futex: remove duplicated code
2017-03-03 12:27 [PATCH 1/3] futex: remove duplicated code Jiri Slaby
@ 2017-03-03 14:08 ` Heiko Carstens
2017-03-03 14:48 ` Chris Metcalf
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Heiko Carstens @ 2017-03-03 14:08 UTC (permalink / raw)
To: linux-snps-arc
On Fri, Mar 03, 2017@01:27:10PM +0100, Jiri Slaby wrote:
> There is code duplicated over all architecture's headers for
> futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
> and comparison of the result.
>
> Remove this duplication and leave up to the arches only the needed
> assembly which is now in arch_futex_atomic_op_inuser.
>
> Note that s390 removed access_ok check in d12a29703 ("s390/uaccess:
> remove pointless access_ok() checks") as access_ok there returns true.
> We introduce it back to the helper for the sake of simplicity (it gets
> optimized away anyway).
>
> Signed-off-by: Jiri Slaby <jslaby at suse.cz>
> ---
> arch/s390/include/asm/futex.h | 23 ++++-------------
> include/asm-generic/futex.h | 50 +++++++------------------------------
> kernel/futex.c | 36 ++++++++++++++++++++++++++
Looks good to me and still boots on s390. Therefore for the s390 bits:
Acked-by: Heiko Carstens <heiko.carstens at de.ibm.com>
Thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] futex: remove duplicated code
2017-03-03 12:27 [PATCH 1/3] futex: remove duplicated code Jiri Slaby
2017-03-03 14:08 ` Heiko Carstens
@ 2017-03-03 14:48 ` Chris Metcalf
2017-03-04 12:49 ` Michael Ellerman
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Chris Metcalf @ 2017-03-03 14:48 UTC (permalink / raw)
To: linux-snps-arc
On 3/3/2017 7:27 AM, Jiri Slaby wrote:
> There is code duplicated over all architecture's headers for
> futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
> and comparison of the result.
>
> Remove this duplication and leave up to the arches only the needed
> assembly which is now in arch_futex_atomic_op_inuser.
>
> Note that s390 removed access_ok check in d12a29703 ("s390/uaccess:
> remove pointless access_ok() checks") as access_ok there returns true.
> We introduce it back to the helper for the sake of simplicity (it gets
> optimized away anyway).
>
> Signed-off-by: Jiri Slaby<jslaby at suse.cz>
Acked-by: Chris Metcalf <cmetcalf at mellanox.com> [for tile]
--
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] futex: remove duplicated code
2017-03-03 12:27 [PATCH 1/3] futex: remove duplicated code Jiri Slaby
2017-03-03 14:08 ` Heiko Carstens
2017-03-03 14:48 ` Chris Metcalf
@ 2017-03-04 12:49 ` Michael Ellerman
2017-03-04 13:05 ` Russell King - ARM Linux
2017-03-06 1:52 ` Rich Felker
4 siblings, 0 replies; 12+ messages in thread
From: Michael Ellerman @ 2017-03-04 12:49 UTC (permalink / raw)
To: linux-snps-arc
Jiri Slaby <jslaby at suse.cz> writes:
> There is code duplicated over all architecture's headers for
> futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
> and comparison of the result.
>
> Remove this duplication and leave up to the arches only the needed
> assembly which is now in arch_futex_atomic_op_inuser.
Looks OK and boots on powerpc. But I don't think anything's actually
calling those futex ops. Is there a test suite I should run?
Acked-by: Michael Ellerman <mpe at ellerman.id.au> (powerpc)
cheers
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] futex: remove duplicated code
2017-03-03 12:27 [PATCH 1/3] futex: remove duplicated code Jiri Slaby
` (2 preceding siblings ...)
2017-03-04 12:49 ` Michael Ellerman
@ 2017-03-04 13:05 ` Russell King - ARM Linux
2017-03-04 19:15 ` H. Peter Anvin
2017-03-09 4:16 ` Rob Landley
2017-03-06 1:52 ` Rich Felker
4 siblings, 2 replies; 12+ messages in thread
From: Russell King - ARM Linux @ 2017-03-04 13:05 UTC (permalink / raw)
To: linux-snps-arc
On Fri, Mar 03, 2017@01:27:10PM +0100, Jiri Slaby wrote:
> diff --git a/arch/arm/include/asm/futex.h b/arch/arm/include/asm/futex.h
> index 6795368ad023..cc414382dab4 100644
> --- a/arch/arm/include/asm/futex.h
> +++ b/arch/arm/include/asm/futex.h
> @@ -128,20 +128,10 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
> #endif /* !SMP */
>
> static inline int
> -futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
> +arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
> {
> - int op = (encoded_op >> 28) & 7;
> - int cmp = (encoded_op >> 24) & 15;
> - int oparg = (encoded_op << 8) >> 20;
> - int cmparg = (encoded_op << 20) >> 20;
> int oldval = 0, ret, tmp;
>
> - if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
> - oparg = 1 << oparg;
> -
> - if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
> - return -EFAULT;
> -
> #ifndef CONFIG_SMP
> preempt_disable();
> #endif
> @@ -172,17 +162,9 @@ futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
> preempt_enable();
> #endif
>
> - if (!ret) {
> - switch (cmp) {
> - case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break;
> - case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break;
> - case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break;
> - case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break;
> - case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break;
> - case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break;
> - default: ret = -ENOSYS;
> - }
> - }
> + if (!ret)
> + *oval = oldval;
> +
> return ret;
> }
>
> diff --git a/kernel/futex.c b/kernel/futex.c
> index b687cb22301c..c5ff9850952f 100644
> --- a/kernel/futex.c
> +++ b/kernel/futex.c
> @@ -1457,6 +1457,42 @@ futex_wake(u32 __user *uaddr, unsigned int flags, int nr_wake, u32 bitset)
> return ret;
> }
>
> +static int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
> +{
> + int op = (encoded_op >> 28) & 7;
> + int cmp = (encoded_op >> 24) & 15;
> + int oparg = (encoded_op << 8) >> 20;
> + int cmparg = (encoded_op << 20) >> 20;
Hmm. oparg and cmparg look like they're doing these shifts to get sign
extension of the 12-bit values by assuming that "int" is 32-bit -
probably worth a comment, or for safety, they should be "s32" so it's
not dependent on the bit-width of "int".
> + int oldval, ret;
> +
> + if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28))
> + oparg = 1 << oparg;
I guess it doesn't matter that oparg can be >= the bit size of oparg
(so large values produce an undefined result) as it's no different
from userspace trying to do the same with large shifts.
> +
> + if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
> + return -EFAULT;
> +
> + ret = arch_futex_atomic_op_inuser(op, oparg, &oldval, uaddr);
> + if (ret)
> + return ret;
> +
> + switch (cmp) {
> + case FUTEX_OP_CMP_EQ:
> + return oldval == cmparg;
> + case FUTEX_OP_CMP_NE:
> + return oldval != cmparg;
> + case FUTEX_OP_CMP_LT:
> + return oldval < cmparg;
> + case FUTEX_OP_CMP_GE:
> + return oldval >= cmparg;
> + case FUTEX_OP_CMP_LE:
> + return oldval <= cmparg;
> + case FUTEX_OP_CMP_GT:
> + return oldval > cmparg;
> + default:
> + return -ENOSYS;
> + }
> +}
> +
> /*
> * Wake up all waiters hashed on the physical page that is mapped
> * to this virtual address:
As it's no worse than our existing code, for the above,
Acked-by: Russell King <rmk+kernel at armlinux.org.uk>
Thanks.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] futex: remove duplicated code
2017-03-04 13:05 ` Russell King - ARM Linux
@ 2017-03-04 19:15 ` H. Peter Anvin
2017-03-04 21:38 ` Stafford Horne
2017-03-09 4:16 ` Rob Landley
1 sibling, 1 reply; 12+ messages in thread
From: H. Peter Anvin @ 2017-03-04 19:15 UTC (permalink / raw)
To: linux-snps-arc
On 03/04/17 05:05, Russell King - ARM Linux wrote:
>>
>> +static int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
>> +{
>> + int op = (encoded_op >> 28) & 7;
>> + int cmp = (encoded_op >> 24) & 15;
>> + int oparg = (encoded_op << 8) >> 20;
>> + int cmparg = (encoded_op << 20) >> 20;
>
> Hmm. oparg and cmparg look like they're doing these shifts to get sign
> extension of the 12-bit values by assuming that "int" is 32-bit -
> probably worth a comment, or for safety, they should be "s32" so it's
> not dependent on the bit-width of "int".
>
For readability, perhaps we should make sign- and zero-extension an
explicit facility?
/*
* Truncate an integer x to n bits, using sign- or
* zero-extension, respectively.
*/
static inline __const_func__ s32 sex32(s32 x, int n)
{
return (x << (32-n)) >> (32-n);
}
static inline __const_func__ s64 sex64(s64 x, int n)
{
return (x << (64-n)) >> (64-n);
}
#define sex(x,y) \
((__typeof__(x)) \
(((__builtin_constant_p(y) && ((y) <= 32)) || \
(sizeof(x) <= sizeof(s32))) \
? sex32((x),(y)) : sex64((x),(y))))
static inline __const_func__ u32 zex32(u32 x, int n)
{
return (x << (32-n)) >> (32-n);
}
static inline __const_func__ u64 zex64(u64 x, int n)
{
return (x << (64-n)) >> (64-n);
}
#define zex(x,y) \
((__typeof__(x)) \
(((__builtin_constant_p(y) && ((y) <= 32)) || \
(sizeof(x) <= sizeof(u32))) \
? zex32((x),(y)) : zex64((x),(y))))
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] futex: remove duplicated code
2017-03-04 19:15 ` H. Peter Anvin
@ 2017-03-04 21:38 ` Stafford Horne
2017-03-04 23:03 ` H. Peter Anvin
2017-03-04 23:08 ` H. Peter Anvin
0 siblings, 2 replies; 12+ messages in thread
From: Stafford Horne @ 2017-03-04 21:38 UTC (permalink / raw)
To: linux-snps-arc
On Sat, Mar 04, 2017@11:15:17AM -0800, H. Peter Anvin wrote:
> On 03/04/17 05:05, Russell King - ARM Linux wrote:
> >>
> >> +static int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
> >> +{
> >> + int op = (encoded_op >> 28) & 7;
> >> + int cmp = (encoded_op >> 24) & 15;
> >> + int oparg = (encoded_op << 8) >> 20;
> >> + int cmparg = (encoded_op << 20) >> 20;
> >
> > Hmm. oparg and cmparg look like they're doing these shifts to get sign
> > extension of the 12-bit values by assuming that "int" is 32-bit -
> > probably worth a comment, or for safety, they should be "s32" so it's
> > not dependent on the bit-width of "int".
> >
>
> For readability, perhaps we should make sign- and zero-extension an
> explicit facility?
There is some of this in already here, 32 and 64 bit versions:
include/linux/bitops.h
Do we really need zero extension? It seems the same.
Example implementation from bitops.h
static inline __s32 sign_extend32(__u32 value, int index)
{
__u8 shift = 31 - index;
return (__s32)(value << shift) >> shift;
}
> /*
> * Truncate an integer x to n bits, using sign- or
> * zero-extension, respectively.
> */
> static inline __const_func__ s32 sex32(s32 x, int n)
> {
> return (x << (32-n)) >> (32-n);
> }
>
> static inline __const_func__ s64 sex64(s64 x, int n)
> {
> return (x << (64-n)) >> (64-n);
> }
>
> #define sex(x,y) \
> ((__typeof__(x)) \
> (((__builtin_constant_p(y) && ((y) <= 32)) || \
> (sizeof(x) <= sizeof(s32))) \
> ? sex32((x),(y)) : sex64((x),(y))))
>
> static inline __const_func__ u32 zex32(u32 x, int n)
> {
> return (x << (32-n)) >> (32-n);
> }
>
> static inline __const_func__ u64 zex64(u64 x, int n)
> {
> return (x << (64-n)) >> (64-n);
> }
>
> #define zex(x,y) \
> ((__typeof__(x)) \
> (((__builtin_constant_p(y) && ((y) <= 32)) || \
> (sizeof(x) <= sizeof(u32))) \
> ? zex32((x),(y)) : zex64((x),(y))))
>
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] futex: remove duplicated code
2017-03-04 21:38 ` Stafford Horne
@ 2017-03-04 23:03 ` H. Peter Anvin
2017-03-04 23:08 ` H. Peter Anvin
1 sibling, 0 replies; 12+ messages in thread
From: H. Peter Anvin @ 2017-03-04 23:03 UTC (permalink / raw)
To: linux-snps-arc
<davem at davemloft.net>,Chris Metcalf <cmetcalf at mellanox.com>,Thomas Gleixner <tglx at linutronix.de>,Ingo Molnar <mingo at redhat.com>,Chris Zankel <chris at zankel.net>,Max Filippov <jcmvbkbc at gmail.com>,Arnd Bergmann <arnd at arndb.de>,x86 at kernel.org,linux-alpha at vger.kernel.org,linux-snps-arc at lists.infradead.org,linux-arm-kernel at lists.infradead.org,linux-hexagon at vger.kernel.org,linux-ia64 at vger.kernel.org,linux-mips at linux-mips.org,openrisc at lists.librecores.org,linux-parisc at vger.kernel.org,linuxppc-dev at lists.ozlabs.org,linux-s390 at vger.kernel.org,linux-sh at vger.kernel.org,sparclinux at vger.kernel.org,linux-xtensa at linux-xtensa.org,linux-arch at vger.kernel.org
From: hpa@zytor.com
List-ID: <linux-snps-arc@lists.infradead.org>
To: linux-snps-arc@lists.infradead.org
Message-ID: <9B46AFA6-C422-41FD-8E8A-356E44A6DCD2 at zytor.com>
On March 4, 2017 1:38:05 PM PST, Stafford Horne <shorne@gmail.com> wrote:
>On Sat, Mar 04, 2017@11:15:17AM -0800, H. Peter Anvin wrote:
>> On 03/04/17 05:05, Russell King - ARM Linux wrote:
>> >>
>> >> +static int futex_atomic_op_inuser(int encoded_op, u32 __user
>*uaddr)
>> >> +{
>> >> + int op = (encoded_op >> 28) & 7;
>> >> + int cmp = (encoded_op >> 24) & 15;
>> >> + int oparg = (encoded_op << 8) >> 20;
>> >> + int cmparg = (encoded_op << 20) >> 20;
>> >
>> > Hmm. oparg and cmparg look like they're doing these shifts to get
>sign
>> > extension of the 12-bit values by assuming that "int" is 32-bit -
>> > probably worth a comment, or for safety, they should be "s32" so
>it's
>> > not dependent on the bit-width of "int".
>> >
>>
>> For readability, perhaps we should make sign- and zero-extension an
>> explicit facility?
>
>There is some of this in already here, 32 and 64 bit versions:
>
> include/linux/bitops.h
>
>Do we really need zero extension? It seems the same.
>
>Example implementation from bitops.h
>
>static inline __s32 sign_extend32(__u32 value, int index)
>{
> __u8 shift = 31 - index;
> return (__s32)(value << shift) >> shift;
>}
>
>> /*
>> * Truncate an integer x to n bits, using sign- or
>> * zero-extension, respectively.
>> */
>> static inline __const_func__ s32 sex32(s32 x, int n)
>> {
>> return (x << (32-n)) >> (32-n);
>> }
>>
>> static inline __const_func__ s64 sex64(s64 x, int n)
>> {
>> return (x << (64-n)) >> (64-n);
>> }
>>
>> #define sex(x,y) \
>> ((__typeof__(x)) \
>> (((__builtin_constant_p(y) && ((y) <= 32)) || \
>> (sizeof(x) <= sizeof(s32))) \
>> ? sex32((x),(y)) : sex64((x),(y))))
>>
>> static inline __const_func__ u32 zex32(u32 x, int n)
>> {
>> return (x << (32-n)) >> (32-n);
>> }
>>
>> static inline __const_func__ u64 zex64(u64 x, int n)
>> {
>> return (x << (64-n)) >> (64-n);
>> }
>>
>> #define zex(x,y) \
>> ((__typeof__(x)) \
>> (((__builtin_constant_p(y) && ((y) <= 32)) || \
>> (sizeof(x) <= sizeof(u32))) \
>> ? zex32((x),(y)) : zex64((x),(y))))
>>
Signed versus unsigned...
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] futex: remove duplicated code
2017-03-04 21:38 ` Stafford Horne
2017-03-04 23:03 ` H. Peter Anvin
@ 2017-03-04 23:08 ` H. Peter Anvin
1 sibling, 0 replies; 12+ messages in thread
From: H. Peter Anvin @ 2017-03-04 23:08 UTC (permalink / raw)
To: linux-snps-arc
<davem at davemloft.net>,Chris Metcalf <cmetcalf at mellanox.com>,Thomas Gleixner <tglx at linutronix.de>,Ingo Molnar <mingo at redhat.com>,Chris Zankel <chris at zankel.net>,Max Filippov <jcmvbkbc at gmail.com>,Arnd Bergmann <arnd at arndb.de>,x86 at kernel.org,linux-alpha at vger.kernel.org,linux-snps-arc at lists.infradead.org,linux-arm-kernel at lists.infradead.org,linux-hexagon at vger.kernel.org,linux-ia64 at vger.kernel.org,linux-mips at linux-mips.org,openrisc at lists.librecores.org,linux-parisc at vger.kernel.org,linuxppc-dev at lists.ozlabs.org,linux-s390 at vger.kernel.org,linux-sh at vger.kernel.org,sparclinux at vger.kernel.org,linux-xtensa at linux-xtensa.org,linux-arch at vger.kernel.org
From: hpa@zytor.com
List-ID: <linux-snps-arc@lists.infradead.org>
To: linux-snps-arc@lists.infradead.org
Message-ID: <CF18535E-39E7-44D3-88D0-80B9961E6681 at zytor.com>
On March 4, 2017 1:38:05 PM PST, Stafford Horne <shorne@gmail.com> wrote:
>On Sat, Mar 04, 2017@11:15:17AM -0800, H. Peter Anvin wrote:
>> On 03/04/17 05:05, Russell King - ARM Linux wrote:
>> >>
>> >> +static int futex_atomic_op_inuser(int encoded_op, u32 __user
>*uaddr)
>> >> +{
>> >> + int op = (encoded_op >> 28) & 7;
>> >> + int cmp = (encoded_op >> 24) & 15;
>> >> + int oparg = (encoded_op << 8) >> 20;
>> >> + int cmparg = (encoded_op << 20) >> 20;
>> >
>> > Hmm. oparg and cmparg look like they're doing these shifts to get
>sign
>> > extension of the 12-bit values by assuming that "int" is 32-bit -
>> > probably worth a comment, or for safety, they should be "s32" so
>it's
>> > not dependent on the bit-width of "int".
>> >
>>
>> For readability, perhaps we should make sign- and zero-extension an
>> explicit facility?
>
>There is some of this in already here, 32 and 64 bit versions:
>
> include/linux/bitops.h
>
>Do we really need zero extension? It seems the same.
>
>Example implementation from bitops.h
>
>static inline __s32 sign_extend32(__u32 value, int index)
>{
> __u8 shift = 31 - index;
> return (__s32)(value << shift) >> shift;
>}
>
>> /*
>> * Truncate an integer x to n bits, using sign- or
>> * zero-extension, respectively.
>> */
>> static inline __const_func__ s32 sex32(s32 x, int n)
>> {
>> return (x << (32-n)) >> (32-n);
>> }
>>
>> static inline __const_func__ s64 sex64(s64 x, int n)
>> {
>> return (x << (64-n)) >> (64-n);
>> }
>>
>> #define sex(x,y) \
>> ((__typeof__(x)) \
>> (((__builtin_constant_p(y) && ((y) <= 32)) || \
>> (sizeof(x) <= sizeof(s32))) \
>> ? sex32((x),(y)) : sex64((x),(y))))
>>
>> static inline __const_func__ u32 zex32(u32 x, int n)
>> {
>> return (x << (32-n)) >> (32-n);
>> }
>>
>> static inline __const_func__ u64 zex64(u64 x, int n)
>> {
>> return (x << (64-n)) >> (64-n);
>> }
>>
>> #define zex(x,y) \
>> ((__typeof__(x)) \
>> (((__builtin_constant_p(y) && ((y) <= 32)) || \
>> (sizeof(x) <= sizeof(u32))) \
>> ? zex32((x),(y)) : zex64((x),(y))))
>>
Also, i strongly believe that making it syntactically cumbersome encodes people to open-code it which is bad...
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] futex: remove duplicated code
2017-03-04 13:05 ` Russell King - ARM Linux
2017-03-04 19:15 ` H. Peter Anvin
@ 2017-03-09 4:16 ` Rob Landley
2017-03-09 4:36 ` H. Peter Anvin
1 sibling, 1 reply; 12+ messages in thread
From: Rob Landley @ 2017-03-09 4:16 UTC (permalink / raw)
To: linux-snps-arc
On 03/04/2017 07:05 AM, Russell King - ARM Linux wrote:
> On Fri, Mar 03, 2017@01:27:10PM +0100, Jiri Slaby wrote:
>> diff --git a/kernel/futex.c b/kernel/futex.c
>> index b687cb22301c..c5ff9850952f 100644
>> --- a/kernel/futex.c
>> +++ b/kernel/futex.c
>> @@ -1457,6 +1457,42 @@ futex_wake(u32 __user *uaddr, unsigned int flags, int nr_wake, u32 bitset)
>> return ret;
>> }
>>
>> +static int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr)
>> +{
>> + int op = (encoded_op >> 28) & 7;
>> + int cmp = (encoded_op >> 24) & 15;
>> + int oparg = (encoded_op << 8) >> 20;
>> + int cmparg = (encoded_op << 20) >> 20;
>
> Hmm. oparg and cmparg look like they're doing these shifts to get sign
> extension of the 12-bit values by assuming that "int" is 32-bit -
> probably worth a comment, or for safety, they should be "s32" so it's
> not dependent on the bit-width of "int".
I thought Linux depended on the LP64 standard for all architectures?
Standard: http://www.unix.org/whitepapers/64bit.html
Rationale: http://www.unix.org/version2/whatsnew/lp64_wp.html
So int has a defined bit width (32) on linux?
Rob
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] futex: remove duplicated code
2017-03-09 4:16 ` Rob Landley
@ 2017-03-09 4:36 ` H. Peter Anvin
0 siblings, 0 replies; 12+ messages in thread
From: H. Peter Anvin @ 2017-03-09 4:36 UTC (permalink / raw)
To: linux-snps-arc
<cmetcalf at mellanox.com>,Thomas Gleixner <tglx at linutronix.de>,Ingo Molnar <mingo at redhat.com>,Chris Zankel <chris at zankel.net>,Max Filippov <jcmvbkbc at gmail.com>,Arnd Bergmann <arnd at arndb.de>,x86 at kernel.org,linux-alpha at vger.kernel.org,linux-snps-arc at lists.infradead.org,linux-arm-kernel at lists.infradead.org,linux-hexagon at vger.kernel.org,linux-ia64 at vger.kernel.org,linux-mips at linux-mips.org,openrisc at lists.librecores.org,linux-parisc at vger.kernel.org,linuxppc-dev at lists.ozlabs.org,linux-s390 at vger.kernel.org,linux-sh at vger.kernel.org,sparclinux at vger.kernel.org,linux-xtensa at linux-xtensa.org,linux-arch at vger.kernel.org
From: hpa@zytor.com
List-ID: <linux-snps-arc@lists.infradead.org>
To: linux-snps-arc@lists.infradead.org
Message-ID: <83324528-AAA1-4BED-B0C7-48426ECBA261 at zytor.com>
On March 8, 2017 8:16:49 PM PST, Rob Landley <rob@landley.net> wrote:
>On 03/04/2017 07:05 AM, Russell King - ARM Linux wrote:
>> On Fri, Mar 03, 2017@01:27:10PM +0100, Jiri Slaby wrote:
>>> diff --git a/kernel/futex.c b/kernel/futex.c
>>> index b687cb22301c..c5ff9850952f 100644
>>> --- a/kernel/futex.c
>>> +++ b/kernel/futex.c
>>> @@ -1457,6 +1457,42 @@ futex_wake(u32 __user *uaddr, unsigned int
>flags, int nr_wake, u32 bitset)
>>> return ret;
>>> }
>>>
>>> +static int futex_atomic_op_inuser(int encoded_op, u32 __user
>*uaddr)
>>> +{
>>> + int op = (encoded_op >> 28) & 7;
>>> + int cmp = (encoded_op >> 24) & 15;
>>> + int oparg = (encoded_op << 8) >> 20;
>>> + int cmparg = (encoded_op << 20) >> 20;
>>
>> Hmm. oparg and cmparg look like they're doing these shifts to get
>sign
>> extension of the 12-bit values by assuming that "int" is 32-bit -
>> probably worth a comment, or for safety, they should be "s32" so it's
>> not dependent on the bit-width of "int".
>
>I thought Linux depended on the LP64 standard for all architectures?
>
>Standard: http://www.unix.org/whitepapers/64bit.html
>Rationale: http://www.unix.org/version2/whatsnew/lp64_wp.html
>
>So int has a defined bit width (32) on linux?
>
>Rob
Linux is ILP32 on 32-bit architectures and LP64 on 64-bit architectures, but that doesn't inherently make this stuff clear.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] futex: remove duplicated code
2017-03-03 12:27 [PATCH 1/3] futex: remove duplicated code Jiri Slaby
` (3 preceding siblings ...)
2017-03-04 13:05 ` Russell King - ARM Linux
@ 2017-03-06 1:52 ` Rich Felker
4 siblings, 0 replies; 12+ messages in thread
From: Rich Felker @ 2017-03-06 1:52 UTC (permalink / raw)
To: linux-snps-arc
On Fri, Mar 03, 2017@01:27:10PM +0100, Jiri Slaby wrote:
> There is code duplicated over all architecture's headers for
> futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr,
> and comparison of the result.
>
> Remove this duplication and leave up to the arches only the needed
> assembly which is now in arch_futex_atomic_op_inuser.
>
> Note that s390 removed access_ok check in d12a29703 ("s390/uaccess:
> remove pointless access_ok() checks") as access_ok there returns true.
> We introduce it back to the helper for the sake of simplicity (it gets
> optimized away anyway).
Overall I'm in favor of this patch, and it's close to what I had in
mind in the commit message for
00b73d8d1b7131da03aec73011a7286f566fe87f. But I'd actually like to see
it go further. These ops are mainly (only?) used for the (almost never
used) FUTEX_WAKE_OP operation, and there's very little sense in trying
to optimize them with dedicated arch-specific forms like "lock xadd".
Instead the entire logic should be in an arch-generic file, and all
the arch should need to provide is a cmpxchg-on-user-memory primitive
for it to use. On most archs, the same cmpxchg used in kernelspace
should also work for user addresses, meaning a huge amount of
unmaintained, largely untested, junk code can be removed.
Rich
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-03-09 4:36 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-03 12:27 [PATCH 1/3] futex: remove duplicated code Jiri Slaby
2017-03-03 14:08 ` Heiko Carstens
2017-03-03 14:48 ` Chris Metcalf
2017-03-04 12:49 ` Michael Ellerman
2017-03-04 13:05 ` Russell King - ARM Linux
2017-03-04 19:15 ` H. Peter Anvin
2017-03-04 21:38 ` Stafford Horne
2017-03-04 23:03 ` H. Peter Anvin
2017-03-04 23:08 ` H. Peter Anvin
2017-03-09 4:16 ` Rob Landley
2017-03-09 4:36 ` H. Peter Anvin
2017-03-06 1:52 ` Rich Felker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox