Optimi[zs]e copy_*_user routines
RFC 1/1 Optimi[zs]e copy_*_user routines
This has been on my todo list for sometime -- the parisc kernel currently uses a one-byte-at-a-time implementation for copy_{to,from,in}_user. Obviously there is much room for improvement. Here's an attempt at a more optimal version.
There is a somewhat lengthy comment below in memcpy.c about how this works, so I will not repeat that here. This seems to work ok for me for 64-bit-smp. Test reports on other systems and different workloads are welcome (but see the warning below).
Unfortunately this doesn't speed up as many things as I thought it would. There are quite a bit of userspace copying that go through copy_user_page(), which already uses an unrolled register copy loop. I suspect this implementation is still slightly faster than that, so possibly that can be slightly optimized as well. You will see the most improvement in pipe or local socket intensive workloads. On the a500 that i tested on, this improved bw_pipe and bw_unix (from lmbench) results about 400%. But for e.g. ftp or make vmlinux there was no measureable performance difference.
I am still doing some verifications on the error handling, so there is a chance that this patch will corrupt your data. Use with care! With that cavaet, suggestions for further optimizations and improvements are much appreciated!!
Many thanks to Grant and Carlos for looking over and helping me spot some problems in my earlier attempts.
randolph
Index: arch/parisc/lib/Makefile
===================================================================
RCS file: /var/cvs/linux-2.6/arch/parisc/lib/Makefile,v retrieving revision 1.2
parisc/lib/Makefile +1 -1
@@ -2,6 +2,6 @@ # Makefile for parisc-specific library files # -lib-y := lusercopy.o bitops.o checksum.o io.o memset.o +lib-y := lusercopy.o bitops.o checksum.o io.o memset.o memcpy.o lib-$(CONFIG_SMP) += debuglocks.o
Index: arch/parisc/lib/memcpy.c
===================================================================
RCS file: arch/parisc/lib/memcpy.c
parisc/lib/memcpy.c new file +457 -0
457 lines not on this page · raw message
Index: arch/parisc/Makefile
===================================================================
RCS file: /var/cvs/linux-2.6/arch/parisc/Makefile,v retrieving revision 1.13
parisc/Makefile +1 -1
@@ -38,7 +38,7 @@ cflags-y := -pipe cflags-y += -mno-space-regs -mfast-indirect-calls # No fixed-point multiply -cflags-y += -mdisable-fpregs +#cflags-y += -mdisable-fpregs # Without this, "ld -r" results in .text sections that are too big # (> 0x40000) for branches to reach stubs.
Index: include/asm-parisc/uaccess.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-parisc/uaccess.h,v retrieving revision 1.13
asm-parisc/uaccess.h +6 -6
@@ -273,11 +273,11 @@ extern long lstrnlen_user(const char __u#define clear_user lclear_user#define __clear_user lclear_user -#define copy_from_user lcopy_from_user -#define __copy_from_user lcopy_from_user -#define copy_to_user lcopy_to_user -#define __copy_to_user lcopy_to_user -#define copy_in_user lcopy_in_user -#define __copy_in_user lcopy_in_user +unsigned long copy_to_user (void __user *dst,const void *src,unsigned long len); +#define __copy_to_user copy_to_user +unsigned long copy_from_user (void *dst,const void __user *src,unsigned long len); +#define __copy_from_user copy_from_user +unsigned long copy_in_user (void __user *dst,const void __user *src,unsigned long len); +#define __copy_in_user copy_in_user#endif /* __PARISC_UACCESS_H */
Signature · 7 lines
Randolph Chung Debian GNU/Linux Developer, hppa/ia64 ports http://www.tausq.org/ _______________________________________________ parisc-linux mailing list [email protected] http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
In reference to a message from Randolph Chung, dated Sep 09:
Quoted 4 lines
This has been on my todo list for sometime -- the parisc kernel currently uses a one-byte-at-a-time implementation for copy_{to,from,in}_user. Obviously there is much room for improvement. Here's an attempt at a more optimal version.
A few more addon questions/RFCs:
- this patch relies on using fp regs for copy operations. In our current kernel we actually have -mdisable-fpregs, but i was not able to ascertain why. we don't currently have fp lazy state saving. there's been some talk about doing this, but it requires some help from gcc. is this ok?
- i have a test suite for testing the copy routine in userspace, so memcpy.c is written in a way to make it easily sharable between userspace and kernel. is that ok to include the userspace support code in the kernel?
- this is the first time the kernel has had to change sr2 in the kernel (thus Carlos' earlier patch to restore it in the syscall path). Arguably we should not do the sr2 save/restore in the syscall path and instead do it in the copy routines, in order to make syscall as fast as possible. Thoughts?
Alternatively, if we play some tricks with the preprocessor we can get away with only using sr1, but having 3 copies of the copy routines in the code. I actually tried this, but found the hacks required to do it to be rather ugly.
- gcc gurus (Dave :) might think what I'm doing with exceptions to be too ugly^Wfragile. are there better ways to do it? :)
this is a somewhat compelling reason to write the whole thing in assembly (which will also allow for a few more small optimizations). do you think the advantages of maintaining C code outweigh the slight performance penality and hacks required?
randolph
Signature · 7 lines
Randolph Chung Debian GNU/Linux Developer, hppa/ia64 ports http://www.tausq.org/ _______________________________________________ parisc-linux mailing list [email protected] http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
Quoted 5 lines
- this is the first time the kernel has had to change sr2 in the kernel (thus Carlos' earlier patch to restore it in the syscall path). Arguably we should not do the sr2 save/restore in the syscall path and instead do it in the copy routines, in order to make syscall as fast as possible. Thoughts?
In an optimal world:
- Syscalls must only copy sr7 (user space id) to sr3 (where the kernel expects user space id to live), and then set sr7 to kernel space id zero, when ready.
- Light weight syscalls clear sr2 so that users don't abuse security.
- syscall_restore should set sr7 to user space id and clear sr2 for syscalls (don't reload from TASK_PT).
- syscall_restore_rfi should store sr3 into sr7, and r0 into sr2 for restoration in restore_rfi.
- When starting a new process sr7 should be set to the processes space id, and sr2 should be cleared for making syscalls with.
a. No userspace code will be using scratch space registers because we
don't support that. We will be clobbering sr2 for the user. If in the future we decided to change the kernels space, we just clobber it to something else. As long as no userspace code is setting sr2 and expecting it to stay set.
b. The kernel might use temp scratch registers, in which case the
kernel can use sr1,sr2,sr5 and sr6, though for each sr we use as a temporary we have to incur the penalty of a save/restore during an interruption (I think that's right eh?).
c. On a schedule we should be saving only the state of those sr's that
are used as temp and are important to the process?
--- I need to review code here, I don't know what path is
taken during a context switch to save/restore state ---
Quoted 4 lines
Alternatively, if we play some tricks with the preprocessor we can get away with only using sr1, but having 3 copies of the copy routines in the code. I actually tried this, but found the hacks required to do it to be rather ugly.
Difficult to maintain.
Quoted 2 lines
- gcc gurus (Dave :) might think what I'm doing with exceptions to be too ugly^Wfragile. are there better ways to do it? :)
GCC might break this if it gets smarter. Though I like the current method of indicating volatility.
Quoted 4 lines
this is a somewhat compelling reason to write the whole thing in assembly (which will also allow for a few more small optimizations). do you think the advantages of maintaining C code outweigh the slight performance penality and hacks required?
Perhaps. The C code is nice though, you could *almost* abstract it to all architectures that have double,single word,byte load store insns :)
Thanks for this wicked work.
c.
_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
A message that is not in the archive (sent off-list or not yet received): [email protected]
In reference to a message from John David Anglin, dated Sep 10:
Quoted 1 line
Have you looked at using stby or stdby for unaligned copies?
do you mean for the entire copy, or just to copy enough bytes to get it aligned?
i've thought about it, for replacing the copy-till-aligned bits, but haven't tried... i always get confused when i try to understand how those insns work :)
randolph
Signature · 7 lines
Randolph Chung Debian GNU/Linux Developer, hppa/ia64 ports http://www.tausq.org/ _______________________________________________ parisc-linux mailing list [email protected] http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
Quoted 5 lines
In reference to a message from John David Anglin, dated Sep 10:
Quoted 1 line
Have you looked at using stby or stdby for unaligned copies?
do you mean for the entire copy, or just to copy enough bytes to get it aligned?
I believe that the "b" completer can be used to write the first set of bytes needed to get you to an alignment boundary. The "e" completer can be used to store the final residual. The PA block move in GCC uses the latter. Basically, you use aligned loads and stby/stdby to store the right set of bytes at the beginning and end. It's much more efficient than doing the beginning and end byte by byte.
Dave
Signature · 6 lines
J. David Anglin [email protected] National Research Council of Canada (613) 990-0752 (FAX: 952-6602) _______________________________________________ parisc-linux mailing list [email protected] http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
Version 2 of optimized copy routines. Changes compared to the previous version:
- uses new exception mechanism to get the error semantics right
- cleaned up debugs
- added TODO items
If there are no more comments, i'll check this in in a couple of days.
thanks
randolph
Index: arch/parisc/Makefile
===================================================================
RCS file: /var/cvs/linux-2.6/arch/parisc/Makefile,v retrieving revision 1.14
parisc/Makefile +1 -1
@@ -38,7 +38,7 @@ cflags-y := -pipe cflags-y += -mno-space-regs -mfast-indirect-calls # No fixed-point multiply -cflags-y += -mdisable-fpregs +#cflags-y += -mdisable-fpregs # Without this, "ld -r" results in .text sections that are too big # (> 0x40000) for branches to reach stubs.
Index: arch/parisc/lib/Makefile
===================================================================
RCS file: /var/cvs/linux-2.6/arch/parisc/lib/Makefile,v retrieving revision 1.3
parisc/lib/Makefile +1 -1
@@ -2,6 +2,6 @@ # Makefile for parisc-specific library files # -lib-y := lusercopy.o bitops.o checksum.o io.o memset.o fixup.o +lib-y := lusercopy.o bitops.o checksum.o io.o memset.o fixup.o memcpy.o lib-$(CONFIG_SMP) += debuglocks.o
Index: arch/parisc/lib/memcpy.c
===================================================================
RCS file: arch/parisc/lib/memcpy.c
parisc/lib/memcpy.c new file +499 -0
499 lines not on this page · raw message
Index: include/asm-parisc/uaccess.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-parisc/uaccess.h,v retrieving revision 1.16
asm-parisc/uaccess.h +6 -6
@@ -267,12 +267,12 @@ extern long lstrnlen_user(const char __u#define clear_user lclear_user#define __clear_user lclear_user -#define copy_from_user lcopy_from_user -#define __copy_from_user lcopy_from_user -#define copy_to_user lcopy_to_user -#define __copy_to_user lcopy_to_user -#define copy_in_user lcopy_in_user -#define __copy_in_user lcopy_in_user +unsigned long copy_to_user (void __user *dst,const void *src,unsigned long len); +#define __copy_to_user copy_to_user +unsigned long copy_from_user (void *dst,const void __user *src,unsigned long len); +#define __copy_from_user copy_from_user +unsigned long copy_in_user (void __user *dst,const void __user *src,unsigned long len); +#define __copy_in_user copy_in_user#define __copy_to_user_inatomic __copy_to_user#define __copy_from_user_inatomic __copy_from_user
_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 3 lines
What tests have you run against this?
I think the original version was pretty stable on my 32-bit UP c3k. Included kernel compiles and some lws tests.
i've done kernel compiles at various -j levels, and run some lmbench benchmarks.
as you know, there's also a memcpy testsuite that verified the main copying algorithm for various block sizes and alignments.
i also have a small test program that is able to exercise the exception cases in the code, so i've tested the various load/store exception cases in the two functions (pa_memcpy and copy_dstaligned).
currently, the exception test program is a bit indirect (it requires a lot of manual work). i have an idea for a better automated testsuite which still needs to be implemented.
randolph
Signature · 7 lines
Randolph Chung Debian GNU/Linux Developer, hppa/ia64 ports http://www.tausq.org/ _______________________________________________ parisc-linux mailing list [email protected] http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
Randolph,
Quoted 2 lines
i've done kernel compiles at various -j levels, and run some lmbench benchmarks.
Good.
Quoted 2 lines
as you know, there's also a memcpy testsuite that verified the main copying algorithm for various block sizes and alignments.
Yup. I ran those first.
Quoted 3 lines
i also have a small test program that is able to exercise the exception cases in the code, so i've tested the various load/store exception cases in the two functions (
pa_memcpyandcopy_dstaligned).
Cool, that's the most important cases to test. My lws testcase exercises teh exception code, but obviously not the exception cases in pa_memcpy and copy_dstaligned.
Quoted 3 lines
currently, the exception test program is a bit indirect (it requires a lot of manual work). i have an idea for a better automated testsuite which still needs to be implemented.
Kernel module? :)
If you did it in C it could be used for a lot of arches.
c.
_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux