From: Arnd Bergmann <arnd@arndb.de>
To: Keith Packard <keithp@keithp.com>
Subject: Re: [PATCH] usb/serial: Add compat_ioctl pass-through
Date: Thu, 27 Nov 2008 15:31:18 +0100
User-Agent: KMail/1.9.9
Cc: linux-kernel@vger.kernel.org, Greg Kroah-Hartman <gregkh@suse.de>
References: <1227774576-5503-1-git-send-email-keithp@keithp.com>
In-Reply-To: <1227774576-5503-1-git-send-email-keithp@keithp.com>
MIME-Version: 1.0
Content-Disposition: inline
Message-Id: <200811271531.18498.arnd@arndb.de>
Content-Type: text/plain;
  charset="iso-8859-15"
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2377
Lines: 52

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 <><
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/