Re: ioctl cleanups: enable sg_io and serial stuff to be shared

[Pavel Machek <pavel@ucw.cz>]

Hi!

> > > BTW, need to extend this to netdev->do_ioctl as well for the
> > > handling of SIOCDEVPRIVATE based stuff.  Oh goody, we can finally
> > > fix up that crap :))))
> >
> > There's a *lot* of structs that contain *ioctl:
> 
> > atmdev.h:       int (*ioctl)(struct atm_dev *dev,unsigned int cmd,void *arg);
> > atmdev.h:       int (*ioctl)(struct atm_dev *dev,unsigned int cmd,void *arg);
> > fs.h:   int (*ioctl) (struct inode *, struct file *, unsigned, unsigned long);
> > fs.h:   int (*ioctl) (struct inode *, struct file *, unsigned int, unsigned long); 
> > hdlc.h: int (*ioctl)(struct net_device *dev, struct ifreq *ifr, int cmd); 
> > ...
> 
> There are even some more that your grep did not catch. However, only 
> few of them actually need to be changed if we add ->compat_ioctl(). 

Having few structs with ->ioctl() and few with both ->ioctl() and
->compat_ioctl() seems pretty ugly to me...

> For those subsystems that have a clearly defined set of ioctls that 
> are implemented by the specific drivers, the compatibility code can
> be put in a higher level ioctl handler, e.g. atm_ioctl() instead of
> each of the atm drivers.

If it is clearly defined, it should not be an ioctl() in the first
place. Yep, you are right ioctl is probably abused for that at few
points... And finding out where it ->compat_ioctl() is going to be
"interesting".

> Finding out exactly which interfaces need to be extended can be done
> in the process.

> > What about this one: redefine it to (*ioctl)( ...., unsigned *long*,
> > unsinged long). That means we can add
> >
> > #define IOCTL_COMPAT 0x1 0000 0000
> 
> I had the same idea before but in addition to the problem that davem
> mentioned, this would require changing every single ioctl handler
> in the kernel and in third party drivers to use 'long' numbers.

Oops; yep, that's a showstopper.

> Not really something we want to do during the current freeze.
> 
> The three options that currently make sense to me are:
> 
> a) keep using register_ioctl32_conversion() for everything

I believe this makes sense: its too close to 2.6 to do anything else.

> b) add a compat_task() function that handlers may use to decide
>    if they should use the compat data structures, list those ioctls
>    as COMPATIBLE_IOCTL()

Unfortunately missing compat handler silently does the wrong thing in
this case.

> c) add ->compat_ioctl() to some interfaces, with HANDLE_IOCTL() as
>    fallback

Seems like too much rewriting to me...
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]