[PATCH 0/2] RFC only - allow arches to over-ride unit sized user copies

[PATCH 0/2] RFC only - allow arches to over-ride unit sized user copies

[Vineet.Gupta1]

From: Vineet Gupta <vgupta@synopsys.com>

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

[PATCH 1/2] asm-generic: uaccess: allow arch to over-ride __get_user_fn()

[Vineet.Gupta1]

From: Vineet Gupta <vgupta@synopsys.com>

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 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

[PATCH 1/2] asm-generic: uaccess: allow arch to over-ride __get_user_fn()

[Vineet.Gupta1]

From: Vineet Gupta <vgupta@synopsys.com>

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 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

[PATCH 2/2] ARC: provide optimal __get_user_fn()

[Vineet.Gupta1]

From: Vineet Gupta <vgupta@synopsys.com>

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 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

[PATCH 2/2] ARC: provide optimal __get_user_fn()

[Vineet.Gupta1]

From: Vineet Gupta <vgupta@synopsys.com>

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 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

Re: [PATCH 2/2] ARC: provide optimal __get_user_fn()

[Arnd Bergmann]

On Tuesday 20 November 2012, Vineet.Gupta1@synopsys.com wrote:
> From: Vineet Gupta <vgupta@synopsys.com>
> 
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>

Yes, this looks good to me.

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 1/2] asm-generic: uaccess: allow arch to over-ride __get_user_fn()

[Arnd Bergmann]

On Tuesday 20 November 2012, Vineet.Gupta1@synopsys.com 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

Re: [PATCH 2/2] ARC: provide optimal __get_user_fn()

[Vineet Gupta]

On Tuesday 20 November 2012 09:21 PM, Arnd Bergmann wrote:
> On Tuesday 20 November 2012, Vineet.Gupta1@synopsys.com wrote:
>> From: Vineet Gupta <vgupta@synopsys.com>
>>
>> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> 
> Yes, this looks good to me.
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 

Can you please also ACK the asm-generic bits of this series [PATCH 1/2]

Thx,
-Vineet

Re: [PATCH 1/2] asm-generic: uaccess: allow arch to over-ride __get_user_fn()

[Arnd Bergmann]

On Tuesday 20 November 2012, Vineet.Gupta1@synopsys.com wrote:
> From: Vineet Gupta <vgupta@synopsys.com>
> 
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 1/2] asm-generic: uaccess: allow arch to over-ride __get_user_fn()

[Vineet Gupta]

On Saturday 22 December 2012 01:13 AM, Arnd Bergmann wrote:
> On Tuesday 20 November 2012, Vineet.Gupta1@synopsys.com wrote:
>> From: Vineet Gupta <vgupta@synopsys.com>
>>
>> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Thx,
-Vineet