Re: [PATCH] usb/serial: Add compat_ioctl pass-through

[Arnd Bergmann <arnd@arndb.de>]

On Thursday 27 November 2008, Keith Packard wrote:
> +static long serial_compat_ioctl(struct tty_struct *tty, struct file *file,
> +                              unsigned int cmd, unsigned long arg)
> +{
> +       struct usb_serial_port *port = tty->driver_data;
> +       long retval = -ENODEV;
> +
> +       dbg("%s - port %d, cmd 0x%.4x", __func__, port->number, cmd);
> +
> +       WARN_ON(!port->port.count);
> +
> +       /* pass on to the driver specific version of this function
> +          if it is available */
> +       if (port->serial->type->compat_ioctl) {
> +               lock_kernel();
> +               retval = port->serial->type->compat_ioctl(tty, file, cmd, arg);
> +               unlock_kernel(); 

By convention, compat_ioctl functions should be called without the big kernel
lock held.

The usb-serial driver all still need to be converted to have their
ioctl function called without bkl.

> --- a/include/linux/usb/serial.h
> +++ b/include/linux/usb/serial.h
> @@ -228,6 +228,8 @@ struct usb_serial_driver {
>         int  (*write_room)(struct tty_struct *tty);
>         int  (*ioctl)(struct tty_struct *tty, struct file *file,
>                       unsigned int cmd, unsigned long arg);
> +       long  (*compat_ioctl)(struct tty_struct *tty, struct file *file,
> +                             unsigned int cmd, unsigned long arg);
>         void (*set_termios)(struct tty_struct *tty,
>                         struct usb_serial_port *port, struct ktermios *old);
>         void (*break_ctl)(struct tty_struct *tty, int break_state);

You should probably define compat_ioctl to return an int as well
so that it becomes possible to use the same function for both eventually.
Then again, we can also drop the file argument, which is entirely unused
in all the usb-serial ioctls.
Maybe the best way for now is to make the new compat_ioctl be
int compat_ioctl(struct tty_struct, unsigned int cmd, unsigned long arg);
and leave the old ioctl as it is. When we get around to pushing the
BKL down into ->ioctl, we can change the prototype at the same time
to warn potential out-of-tree drivers.

	Arnd <><