Provide example copy_in_user implementation
Hi!
This patch adds example copy_in_user implementation (copy_in_user is needed for new ioctl32 implementation, all 64bit archs will need it)... Please apply,
Pavel
include/asm-generic/uaccess.h +13 -0
@@ -0,0 +1,13 @@ +static inline unsigned long copy_in_user (void *dst,const void *src,unsigned size) +{ +unsigned i, ret; +unsigned char c; +for (i=0 ; i<size; i++) { +if (copy_from_user (&c, src+i,1 )) +return size-i; +if (copy_to_user (dst+i, &c,1 )) +return size-i; + } +return 0 ; +} +
Signature · 2 lines
When do you have a heart between your knees? [Johanka's followup: and *two* hearts?]
Quoted 3 lines · Pavel Machek
This patch adds example
copy_in_userimplementation (copy_in_useris needed for new ioctl32 implementation, all 64bit archs will need it)... Please apply,
get_user / put_user on byte quantities may be faster than using copy_from_user/copy_to_user on byte quantities. Yes, it may be a generic implementation, but there's no point in purposely making it inefficient.
Signature · 2 lines
Russell King ([email protected]) The developer of ARM Linux http://www.arm.linux.org.uk/personal/aboutme.html
Quoted 9 lines · Russell King
Quoted 3 lines · Pavel Machek
This patch adds example
copy_in_userimplementation (copy_in_useris needed for new ioctl32 implementation, all 64bit archs will need it)... Please apply,
get_user/put_useron byte quantities may be faster than using copy_from_user/copy_to_user on byte quantities. Yes, it may be a generic implementation, but there's no point in purposely making it inefficient.
Actually, it seems that most architectures do...
static inline unsigned long__copy_from_user(void *to, const void __user *from, unsigned long n)
{
if (__builtin_constant_p(n)) {
unsigned long ret;
switch (n) {
case 1:...so it should be exactly as fast. Ahha, not arm.
If I wanted to optimize it, first step would be to copy over something else than bytes. I'm afraid I do not want to optimize it.
Pavel
Signature · 2 lines
When do you have a heart between your knees? [Johanka's followup: and *two* hearts?]
Quoted 23 lines · Pavel Machek
Quoted 9 lines · Russell King
Quoted 3 lines · Pavel Machek
This patch adds example
copy_in_userimplementation (copy_in_useris needed for new ioctl32 implementation, all 64bit archs will need it)... Please apply,
get_user/put_useron byte quantities may be faster than using copy_from_user/copy_to_user on byte quantities. Yes, it may be a generic implementation, but there's no point in purposely making it inefficient.Actually, it seems that most architectures do...
static inline unsigned long
__copy_from_user(void *to, const void __user *from, unsigned long n){ if (__builtin_constant_p(n)) { unsigned long ret; switch (n) { case 1:
Quoted 1 line
...so it should be exactly as fast. Ahha, not arm.
Note that ARM isn't 64 bit, I'm not worried about it. Given that code does get cut'n'pasted about the place, I still think its a good idea. But its your code...
Signature · 2 lines
Russell King ([email protected]) The developer of ARM Linux http://www.arm.linux.org.uk/personal/aboutme.html
Quoted 2 lines · Pavel Machek
...so it should be exactly as fast.
No it shouldn't.
You should do the access_ok() only _once_ (well, twice: once for source and once for dest).
Also, anything that copies memory one byte at a time is just asking to be shot.
Linus
Quoted 14 lines · Pavel Machek
+static inline unsigned long
copy_in_user(void *dst, const void *src, unsigned size)
+{
+ unsigned i, ret;
+ unsigned char c;
+ for (i=0; i<size; i++) {
+ if (copy_from_user(&c, src+i, 1))
+ return size-i;
+ if (copy_to_user(dst+i, &c, 1))
+ return size-i;
+ }
+ return 0;
+}
+
I know that this is usually not performance critical, but by gawd that code is inefficient and bloaty.
It has 18 callsites; it can be put in lib/lib.a:copy_in_user.o. The access_ok() checks only need to be run once. It can copy a cacheline at a time.
Hi!
Quoted 14 lines
Quoted 5 lines
+static inline unsigned long
copy_in_user(void *dst, const void *src, unsigned size)
+{
+ unsigned i, ret;
+ unsigned char c;
+ for (i=0; i<size; i++) {Quoted 8 lines
+ if (
copy_from_user(&c, src+i, 1))
+ return size-i;
+ if (copy_to_user(dst+i, &c, 1))
+ return size-i;
+ }
+ return 0;
+}
+
Quoted 6 lines
I know that this is usually not performance critical, but by gawd that code is inefficient and bloaty.
It has 18 callsites; it can be put in lib/lib.a:copy_in_user.o. The
access_ok()checks only need to be run once. It can copy a cacheline at a time.
It was meant to demonstrate calling convention of copy_in_user(); architectures probably want to modify their copy_from_user to be able to copy into user, too; usually that can be done at 0 overhead.
Pavel
Signature · 2 lines
When do you have a heart between your knees? [Johanka's followup: and *two* hearts?]
Quoted 4 lines · Pavel Machek
This patch adds example
copy_in_userimplementation (copy_in_useris needed for new ioctl32 implementation, all 64bit archs will need it)... Please apply,
Hell no.
This is an example of how to do things WRONG. In short, it's not an example we want to have in the kernel, or anywhere _near_ the kernel.
Do it right, or don't do it at all. Code this bad should not be allowed to exist, please remove it from your harddisk immediately. I will go and wash my eyes from just having looked at it.
Linus