public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Provide example copy_in_user implementation
@ 2003-06-24 10:06 Pavel Machek
  2003-06-24 10:18 ` Russell King
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Pavel Machek @ 2003-06-24 10:06 UTC (permalink / raw)
  To: torvalds, kernel list, Rusty trivial patch monkey Russell

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?]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Provide example copy_in_user implementation
  2003-06-24 10:06 Provide example copy_in_user implementation Pavel Machek
@ 2003-06-24 10:18 ` Russell King
  2003-06-24 10:25   ` Pavel Machek
  2003-06-24 10:29 ` Andrew Morton
  2003-06-24 18:51 ` Linus Torvalds
  2 siblings, 1 reply; 8+ messages in thread
From: Russell King @ 2003-06-24 10:18 UTC (permalink / raw)
  To: Pavel Machek; +Cc: torvalds, kernel list, Rusty trivial patch monkey Russell

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Provide example copy_in_user implementation
  2003-06-24 10:18 ` Russell King
@ 2003-06-24 10:25   ` Pavel Machek
  2003-06-24 11:01     ` Russell King
  2003-06-24 18:52     ` Linus Torvalds
  0 siblings, 2 replies; 8+ messages in thread
From: Pavel Machek @ 2003-06-24 10:25 UTC (permalink / raw)
  To: torvalds, kernel list, Rusty trivial patch monkey Russell

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?]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Provide example copy_in_user implementation
  2003-06-24 10:06 Provide example copy_in_user implementation Pavel Machek
  2003-06-24 10:18 ` Russell King
@ 2003-06-24 10:29 ` Andrew Morton
  2003-06-24 10:33   ` Pavel Machek
  2003-06-24 18:51 ` Linus Torvalds
  2 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2003-06-24 10:29 UTC (permalink / raw)
  To: Pavel Machek; +Cc: torvalds, linux-kernel, trivial

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Provide example copy_in_user implementation
  2003-06-24 10:29 ` Andrew Morton
@ 2003-06-24 10:33   ` Pavel Machek
  0 siblings, 0 replies; 8+ messages in thread
From: Pavel Machek @ 2003-06-24 10:33 UTC (permalink / raw)
  To: Andrew Morton; +Cc: torvalds, linux-kernel, trivial

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?]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Provide example copy_in_user implementation
  2003-06-24 10:25   ` Pavel Machek
@ 2003-06-24 11:01     ` Russell King
  2003-06-24 18:52     ` Linus Torvalds
  1 sibling, 0 replies; 8+ messages in thread
From: Russell King @ 2003-06-24 11:01 UTC (permalink / raw)
  To: Pavel Machek; +Cc: torvalds, kernel list, Rusty trivial patch monkey Russell

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Provide example copy_in_user implementation
  2003-06-24 10:06 Provide example copy_in_user implementation Pavel Machek
  2003-06-24 10:18 ` Russell King
  2003-06-24 10:29 ` Andrew Morton
@ 2003-06-24 18:51 ` Linus Torvalds
  2 siblings, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2003-06-24 18:51 UTC (permalink / raw)
  To: Pavel Machek; +Cc: kernel list, Rusty trivial patch monkey Russell


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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Provide example copy_in_user implementation
  2003-06-24 10:25   ` Pavel Machek
  2003-06-24 11:01     ` Russell King
@ 2003-06-24 18:52     ` Linus Torvalds
  1 sibling, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2003-06-24 18:52 UTC (permalink / raw)
  To: Pavel Machek; +Cc: kernel list, Rusty trivial patch monkey Russell


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


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2003-06-24 18:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-06-24 10:06 Provide example copy_in_user implementation Pavel Machek
2003-06-24 10:18 ` Russell King
2003-06-24 10:25   ` Pavel Machek
2003-06-24 11:01     ` Russell King
2003-06-24 18:52     ` Linus Torvalds
2003-06-24 10:29 ` Andrew Morton
2003-06-24 10:33   ` Pavel Machek
2003-06-24 18:51 ` Linus Torvalds

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox