Provide example copy_in_user implementation

Provide example copy_in_user implementation

[Pavel Machek]

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

Index: linux/include/asm-generic/uaccess.h
===================================================================
--- linux.orig/include/asm-generic/uaccess.h	2003-06-17 17:10:44.000000000 +0200
+++ linux/include/asm-generic/uaccess.h	2003-06-16 16:09:52.000000000 +0200
@@ -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;
+}	
+


-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: Provide example copy_in_user implementation

[Russell King]

On Tue, Jun 24, 2003 at 12:06:10PM +0200, Pavel Machek wrote:
> 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,

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.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Provide example copy_in_user implementation

[Pavel Machek]

On Út 24-06-03 11:18:20, Russell King wrote:
> On Tue, Jun 24, 2003 at 12:06:10PM +0200, Pavel Machek wrote:
> > 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,
> 
> 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.

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

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: Provide example copy_in_user implementation

[Russell King]

On Tue, Jun 24, 2003 at 12:25:51PM +0200, Pavel Machek wrote:
> On Út 24-06-03 11:18:20, Russell King wrote:
> > On Tue, Jun 24, 2003 at 12:06:10PM +0200, Pavel Machek wrote:
> > > 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,
> > 
> > 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.
> 
> 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.

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

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Provide example copy_in_user implementation

[Linus Torvalds]

On Tue, 24 Jun 2003, Pavel Machek wrote:
> 
> ...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

Re: Provide example copy_in_user implementation

[Andrew Morton]

Pavel Machek <pavel@ucw.cz> wrote:
>
> +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.

Re: Provide example copy_in_user implementation

[Pavel Machek]

Hi!

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

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
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: Provide example copy_in_user implementation

[Linus Torvalds]

On Tue, 24 Jun 2003, Pavel Machek wrote:
> 
> 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,

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