2004-06-11 17:32:24

by Martin Schwidefsky

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

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

From: Martin Schwidefsky <[email protected]>

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

Signed-off-by: Martin Schwidefsky <[email protected]>

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);
}

/**


2004-06-11 22:16:46

by Andrew Morton

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

Martin Schwidefsky <[email protected]> wrote:
>
> [PATCH] s390: speedup strn{cpy,len}_from_user.
>
> From: Martin Schwidefsky <[email protected]>
>
> 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'?

2004-06-11 23:49:04

by George Spelvin

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

>> 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.

2004-06-14 08:40:49

by Martin Schwidefsky

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





> 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: [email protected]