From: Vineet Gupta <[email protected]>
Hi Arnd,
The current asm-generic/uaccess unit copy interface __{get,put}_user( )
defaults to using __copy_{to,from}_user( ). For archs which don't support
unaligned access, latter typically involves generated code for alignment
checks.
It is expected that arch will provide a fast path in __copy_{to,from}_user( )
for such unit sized copies - probably using __builtin_const_p( ) etc -
however the alignment checks still can't be eliminated altogether using that.
Even if they could, it woudl make the implementation messier IMHO.
However given that 2 separate interfaces already exist form beginning
(i.e. __get_user vs. __copy_from_user) will it make sense to allow arch to
provide alternate implementation of former w/o having to mess with latter.
I did a quick hack to that end and I can see that savings are more than
just "noise".
bloat-o-meter vmlinux_pre_uaccess vmlinux_uaccess_part1 | head
add/remove: 0/1 grow/shrink: 10/62 up/down: 1502/-7256 (-5754)
The patch which converts just __get_user follows. I've not
done the full/clean version yet as this is just to gather feedback.
Thx,
-Vineet
Vineet Gupta (2):
asm-generic: uaccess: allow arch to over-ride __get_user_fn()
ARC: provide optimal __get_user_fn()
arch/arc/include/asm/uaccess.h | 51 ++++++++++++++++++++++++++++++++++++++++
include/asm-generic/uaccess.h | 5 ++++
2 files changed, 56 insertions(+), 0 deletions(-)
--
1.7.4.1
From: Vineet Gupta <[email protected]>
Signed-off-by: Vineet Gupta <[email protected]>
---
include/asm-generic/uaccess.h | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/include/asm-generic/uaccess.h b/include/asm-generic/uaccess.h
index 5f6ee61..432d55f 100644
--- a/include/asm-generic/uaccess.h
+++ b/include/asm-generic/uaccess.h
@@ -225,12 +225,17 @@ extern int __put_user_bad(void) __attribute__((noreturn));
-EFAULT; \
})
+#ifndef __get_user_fn
static inline int __get_user_fn(size_t size, const void __user *ptr, void *x)
{
size = __copy_from_user(x, ptr, size);
return size ? -EFAULT : size;
}
+#define __get_user_fn(sz, u, k) __get_user_fn(sz, u, k)
+
+#endif
+
extern int __get_user_bad(void) __attribute__((noreturn));
#ifndef __copy_from_user_inatomic
--
1.7.4.1
From: Vineet Gupta <[email protected]>
Signed-off-by: Vineet Gupta <[email protected]>
---
arch/arc/include/asm/uaccess.h | 51 ++++++++++++++++++++++++++++++++++++++++
1 files changed, 51 insertions(+), 0 deletions(-)
diff --git a/arch/arc/include/asm/uaccess.h b/arch/arc/include/asm/uaccess.h
index 503c98d..d9cbae4 100644
--- a/arch/arc/include/asm/uaccess.h
+++ b/arch/arc/include/asm/uaccess.h
@@ -35,6 +35,57 @@
#define __access_ok(addr, sz) (unlikely(__kernel_ok) || \
likely(__user_ok((addr), (sz))))
+/*********** Single byte/s-word/word copies ******************/
+
+#define __get_user_fn(sz, u, k) \
+({ \
+ long __ret = 0; /* success by default */ \
+ switch (sz) { \
+ case 1: __arc_get_user_one(*(k), u, "ldb", __ret); break; \
+ case 2: __arc_get_user_one(*(k), u, "ldw", __ret); break; \
+ case 4: __arc_get_user_one(*(k), u, "ld", __ret); break; \
+ case 8: __arc_get_user_one_64(*(k), u, __ret); break; \
+ } \
+ __ret; \
+})
+
+#define __arc_get_user_one(dst, src, op, ret) \
+ __asm__ __volatile__( \
+ "1: "op" %1,[%2]\n" \
+ "2: ;nop\n" \
+ " .section .fixup, \"ax\"\n" \
+ " .align 4\n" \
+ "3: mov %0, %3\n" \
+ " j 2b\n" \
+ " .previous\n" \
+ " .section __ex_table, \"a\"\n" \
+ " .align 4\n" \
+ " .word 1b,3b\n" \
+ " .previous\n" \
+ \
+ : "+r" (ret), "=r" (dst) \
+ : "r" (src), "i" (-EFAULT))
+
+#define __arc_get_user_one_64(dst, src, ret) \
+ __asm__ __volatile__( \
+ "1: ld %1,[%2]\n" \
+ "4: ld %R1,[%2, 4]\n" \
+ "2: ;nop\n" \
+ " .section .fixup, \"ax\"\n" \
+ " .align 4\n" \
+ "3: mov %0, %3\n" \
+ " j 2b\n" \
+ " .previous\n" \
+ " .section __ex_table, \"a\"\n" \
+ " .align 4\n" \
+ " .word 1b,3b\n" \
+ " .word 4b,3b\n" \
+ " .previous\n" \
+ \
+ : "+r" (ret), "=r" (dst) \
+ : "r" (src), "i" (-EFAULT))
+
+
static inline unsigned long
__arc_copy_from_user(void *to, const void __user *from, unsigned long n)
{
--
1.7.4.1
On Tuesday 20 November 2012, [email protected] wrote:
> From: Vineet Gupta <[email protected]>
>
> Signed-off-by: Vineet Gupta <[email protected]>
Yes, this looks good to me.
Acked-by: Arnd Bergmann <[email protected]>
On Tuesday 20 November 2012, [email protected] wrote:
> +#ifndef __get_user_fn
> static inline int __get_user_fn(size_t size, const void __user *ptr, void *x)
> {
> size = __copy_from_user(x, ptr, size);
> return size ? -EFAULT : size;
> }
>
> +#define __get_user_fn(sz, u, k) __get_user_fn(sz, u, k)
> +
> +#endif
This is ok as well. The idea with the asm-generic __copy_from_user()
implementation is to separate out the cases where you have just
a few bytes and handle them efficiently inline. If you don't want
to do that for some reason, overriding __get_user_fn works
as well. The resulting object code should be the same.
Arnd