Re: [patch] Re: [discuss] f_ops flag to speed up compatible ioctls in linux kernel

Michael S. Tsirkin
2004-09-08T14:56:42Z

Hello!
Quoting r. Andi Kleen (ak@suse.de) "Re: [patch] Re: [discuss] f_ops flag to speed up compatible ioctls in linux kernel":
> On Wed, Sep 08, 2004 at 05:28:08PM +0300, Michael S. Tsirkin wrote:
> > --- linux-2.6.8.1/include/linux/fs.h 2004-09-07 19:33:43.000000000 +0300
> > +++ linux-2.6.8.1-new/include/linux/fs.h 2004-09-08 07:18:20.000000000 +0300
> > @@ -879,6 +879,8 @@ struct file_operations {
> > int (*readdir) (struct file *, void *, filldir_t);
> > unsigned int (*poll) (struct file *, struct poll_table_struct *);
> > int (*ioctl) (struct inode *, struct file *, unsigned int, unsigned long);
> > + int (*ioctl_native) (struct inode *, struct file *, unsigned int, unsigned long);
> > + int (*ioctl_compat) (struct inode *, struct file *, unsigned int, unsigned long);
>
> Define these as long, not int. No need to waste 32 perfectly good bits on
> 64bit platforms.

I was just following ioctl.

And ioctl is the way it is because man IOCTL(2) defines ioctl as

int ioctl(int d, int request, ...);

So I wander what goes on here- the syscall returns a long but
libc cuts the high 32 bit?

Now that I think about it,for compat if you start returning 0 in low
32 bits you are unlike to get the effect you wanted ...
The ioctl_native could be changed but that would make it impossible
for compatible ioctls to just use the same pointer in both.

So what do you think - should I make just the native ioctl a long,
or both, and document that the high 32 bit are cut in the compat call?

> The main thing missing is documentation. You need clear comments what
> the locking rules are and what compat is good for.

Would these be best fit in the header file itself, or in a new
Documentation/ file?

> And you should change the code style to follow Documentation/CodingStyle
I'll go over it again. Something specific that I missed?

> Other than that it looks ok to me.
>
> -Andi