Re: [PATCH] s390: speedup strn{cpy,len}_from_user.

Re: [PATCH] s390: speedup strn{cpy,len}_from_user.

[linux]

>>  static inline long
>>  strncpy_from_user(char *dst, const char *src, long count)
>>  {
>>          long res = -EFAULT;
>>          might_sleep();
>> -        if (access_ok(VERIFY_READ, src, 1))
>> -                res = __strncpy_from_user_asm(dst, src, count);
>> +        if (access_ok(VERIFY_READ, src, 1)) {
>> +                res = __strncpy_from_user_asm(count, dst, src);
>> +	}
>>          return res;
>>  }

> Shouldn't the access_ok() check be passed `count', rather than `1'?

This deserves a comment, but no.

"count" is the maximum length of the string, not the actual length.
If you passed it to access_ok(), you'd falsely EFAULT a short string
near the end of the address space.

Assuming that there's a guard page at the end of the user address space,
__strncpy_from_user_asm is guaranteed to hit it as part of normal
operations, and will get the fault.

Re: [PATCH] s390: speedup strn{cpy,len}_from_user.

[Martin Schwidefsky]

> There were a few conflicts with Arnd's sparse annotation, which I fixed
up.
> Please check that next the -mm has it all right.

Sorry. Arnd and I need to come up with a way to avoid conflicts in the future.

blue skies,
   Martin

Linux/390 Design & Development, IBM Deutschland Entwicklung GmbH
Schönaicherstr. 220, D-71032 Böblingen, Telefon: 49 - (0)7031 - 16-2247
E-Mail: schwidefsky@de.ibm.com

Re: [PATCH] s390: speedup strn{cpy,len}_from_user.

[Andrew Morton]

Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
>
> [PATCH] s390: speedup strn{cpy,len}_from_user.
> 
> From: Martin Schwidefsky <schwidefsky@de.ibm.com>
> 
> Speedup strncpy_from_user and strnlen_from_user by using
> the search string instruction in the secondary space mode.

There were a few conflicts with Arnd's sparse annotation, which I fixed up.
Please check that next the -mm has it all right.

>  static inline long
>  strncpy_from_user(char *dst, const char *src, long count)
>  {
>          long res = -EFAULT;
>          might_sleep();
> -        if (access_ok(VERIFY_READ, src, 1))
> -                res = __strncpy_from_user_asm(dst, src, count);
> +        if (access_ok(VERIFY_READ, src, 1)) {
> +                res = __strncpy_from_user_asm(count, dst, src);
> +	}
>          return res;
>  }

Shouldn't the access_ok() check be passed `count', rather than `1'?

[PATCH] s390: speedup strn{cpy,len}_from_user.

[Martin Schwidefsky]

[PATCH] s390: speedup strn{cpy,len}_from_user.

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Speedup strncpy_from_user and strnlen_from_user by using
the search string instruction in the secondary space mode.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

diffstat:
 arch/s390/lib/uaccess.S    |   67 ++++++++++++++++++++++++++-------------------
 arch/s390/lib/uaccess64.S  |   65 ++++++++++++++++++++++++-------------------
 include/asm-s390/uaccess.h |   11 ++++---
 3 files changed, 82 insertions(+), 61 deletions(-)

diff -urN linux-2.6/arch/s390/lib/uaccess.S linux-2.6-s390/arch/s390/lib/uaccess.S
--- linux-2.6/arch/s390/lib/uaccess.S	Mon May 10 04:32:53 2004
+++ linux-2.6-s390/arch/s390/lib/uaccess.S	Fri Jun 11 19:09:54 2004
@@ -154,46 +154,57 @@
         .align 4
         .text
         .globl __strncpy_from_user_asm
-	# %r2 = dst, %r3 = src, %r4 = count
+	# %r2 = count, %r3 = dst, %r4 = src
 __strncpy_from_user_asm:
 	lhi	%r0,0
-	lhi	%r1,1
-	lhi	%r5,0
-0:	mvcp	0(%r1,%r2),0(%r3),%r0
-	tm	0(%r2),0xff
-	jz	1f
-	la	%r2,1(%r2)
-	la	%r3,1(%r3)
-	ahi	%r5,1
-	clr	%r5,%r4
-	jl	0b
-1:	lr	%r2,%r5
+	lr	%r1,%r4
+	la	%r4,0(%r4)	# clear high order bit from %r4
+	la	%r2,0(%r2,%r4)	# %r2 points to first byte after string
+	sacf	256
+0:	srst	%r2,%r1
+	jo	0b
+	sacf	0
+	lr	%r1,%r2
+	jh	1f		# \0 found in string ?
+	ahi	%r1,1		# include \0 in copy
+1:	slr	%r1,%r4		# %r1 = copy length (without \0)
+	slr	%r2,%r4		# %r2 = return length (including \0)
+2:	mvcp	0(%r1,%r3),0(%r4),%r0
+	jnz	3f
 	br	%r14
-2:	lhi	%r2,-EFAULT
+3:	la	%r3,256(%r3)
+	la	%r4,256(%r4)
+	ahi	%r1,-256
+	mvcp	0(%r1,%r3),0(%r4),%r0
+	jnz	3b
 	br	%r14
-        .section __ex_table,"a"
-	.long	0b,2b
+4:	sacf	0
+	lhi	%r2,-EFAULT
+	br	%r14
+	.section __ex_table,"a"
+	.long	0b,4b
 	.previous
 
         .align 4
         .text
         .globl __strnlen_user_asm
-	# %r2 = src, %r3 = count
+	# %r2 = count, %r3 = src
 __strnlen_user_asm:
 	lhi	%r0,0
-	lhi	%r1,1
-	lhi	%r5,0
-0:	mvcp	24(%r1,%r15),0(%r2),%r0
-	ahi	%r5,1
-	tm	24(%r15),0xff
-	jz	1f
-	la	%r2,1(%r2)
-	clr	%r5,%r3
-	jl	0b
-1:	lr	%r2,%r5
+	lr	%r1,%r3
+	la	%r3,0(%r3)	# clear high order bit from %r4
+	la	%r2,0(%r2,%r3)	# %r2 points to first byte after string
+	sacf	256
+0:	srst	%r2,%r1
+	jo	0b
+	sacf	0
+	jh	1f		# \0 found in string ?
+	ahi	%r2,1		# strnlen_user result includes the \0
+1:	slr	%r2,%r3
 	br	%r14
-2:	lhi	%r2,-EFAULT
+2:	sacf	0
+	lhi	%r2,-EFAULT
 	br	%r14
-        .section __ex_table,"a"
+	.section __ex_table,"a"
 	.long	0b,2b
 	.previous
diff -urN linux-2.6/arch/s390/lib/uaccess64.S linux-2.6-s390/arch/s390/lib/uaccess64.S
--- linux-2.6/arch/s390/lib/uaccess64.S	Mon May 10 04:33:13 2004
+++ linux-2.6-s390/arch/s390/lib/uaccess64.S	Fri Jun 11 19:09:54 2004
@@ -152,46 +152,55 @@
         .align 4
         .text
         .globl __strncpy_from_user_asm
-	# %r2 = dst, %r3 = src, %r4 = count
+	# %r2 = count, %r3 = dst, %r4 = src
 __strncpy_from_user_asm:
 	lghi	%r0,0
-	lghi	%r1,1
-	lghi	%r5,0
-0:	mvcp	0(%r1,%r2),0(%r3),%r0
-	tm	0(%r2),0xff
-	jz	1f
-	la	%r2,1(%r2)
-	la	%r3,1(%r3)
-	aghi	%r5,1
-	clgr	%r5,%r4
-	jl	0b
-1:	lgr	%r2,%r5
+	lgr	%r1,%r4
+	la	%r2,0(%r2,%r4)	# %r2 points to first byte after string
+	sacf	256
+0:	srst	%r2,%r1
+	jo	0b
+	sacf	0
+	lgr	%r1,%r2
+	jh	1f		# \0 found in string ?
+	aghi	%r1,1		# include \0 in copy
+1:	slgr	%r1,%r4		# %r1 = copy length (without \0)
+	slgr	%r2,%r4		# %r2 = return length (including \0)
+2:	mvcp	0(%r1,%r3),0(%r4),%r0
+	jnz	3f
 	br	%r14
-2:	lghi	%r2,-EFAULT
+3:	la	%r3,256(%r3)
+	la	%r4,256(%r4)
+	aghi	%r1,-256
+	mvcp	0(%r1,%r3),0(%r4),%r0
+	jnz	3b
 	br	%r14
-        .section __ex_table,"a"
-	.quad	0b,2b
+4:	sacf	0
+	lghi	%r2,-EFAULT
+	br	%r14
+	.section __ex_table,"a"
+	.quad	0b,4b
 	.previous
 
         .align 4
         .text
         .globl __strnlen_user_asm
-	# %r2 = src, %r3 = count
+	# %r2 = count, %r3 = src
 __strnlen_user_asm:
 	lghi	%r0,0
-	lghi	%r1,1
-	lghi	%r5,0
-0:	mvcp	24(%r1,%r15),0(%r2),%r0
-	aghi	%r5,1
-	tm	24(%r15),0xff
-	jz	1f
-	la	%r2,1(%r2)
-	clgr	%r5,%r3
-	jl	0b
-1:	lgr	%r2,%r5
+	lgr	%r1,%r3
+	la	%r2,0(%r2,%r3)	# %r2 points to first byte after string
+	sacf	256
+0:	srst	%r2,%r1
+	jo	0b
+	sacf	0
+	jh	1f		# \0 found in string ?
+	aghi	%r2,1		# strnlen_user result includes the \0
+1:	slgr	%r2,%r3
 	br	%r14
-2:	lghi	%r2,-EFAULT
+2:	sacf	0
+	lghi	%r2,-EFAULT
 	br	%r14
-        .section __ex_table,"a"
+	.section __ex_table,"a"
 	.quad	0b,2b
 	.previous
diff -urN linux-2.6/include/asm-s390/uaccess.h linux-2.6-s390/include/asm-s390/uaccess.h
--- linux-2.6/include/asm-s390/uaccess.h	Fri Jun 11 19:09:28 2004
+++ linux-2.6-s390/include/asm-s390/uaccess.h	Fri Jun 11 19:09:54 2004
@@ -346,26 +346,27 @@
 /*
  * Copy a null terminated string from userspace.
  */
-extern long __strncpy_from_user_asm(char *dst, const char *src, long count);
+extern long __strncpy_from_user_asm(long count, char *dst, const char *src);
 
 static inline long
 strncpy_from_user(char *dst, const char *src, long count)
 {
         long res = -EFAULT;
         might_sleep();
-        if (access_ok(VERIFY_READ, src, 1))
-                res = __strncpy_from_user_asm(dst, src, count);
+        if (access_ok(VERIFY_READ, src, 1)) {
+                res = __strncpy_from_user_asm(count, dst, src);
+	}
         return res;
 }
 
 
-extern long __strnlen_user_asm(const char *src, long count);
+extern long __strnlen_user_asm(long count, const char *src);
 
 static inline unsigned long
 strnlen_user(const char * src, unsigned long n)
 {
 	might_sleep();
-	return __strnlen_user_asm(src, n);
+	return __strnlen_user_asm(n, src);
 }
 
 /**