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?]
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 ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
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?]
Pavel Machek <[email protected]> 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.
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?]
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 ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
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
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