Re: [PATCH 10/10] bkl: Fix-up compile problems as a result of the bkl-pushdown.

Linus Torvalds
2010-04-28T14:56:28Z

On Tue, 27 Apr 2010, Arnd Bergmann wrote:
> +static long v4l2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> {
> struct video_device *vdev = video_devdata(filp);
> + int ret;
>
> /* Allow ioctl to continue even if the device was unregistered.
> Things like dequeueing buffers might still be useful. */
> + if (vdev->fops->unlocked_ioctl) {
> + ret = vdev->fops->unlocked_ioctl(filp, cmd, arg);
> + } else if (vdev->fops->ioctl) {
> + /* TODO: convert all drivers to unlocked_ioctl */
> + lock_kernel();
> + ret = vdev->fops->ioctl(filp, cmd, arg);
> + unlock_kernel();
> + } else
> + ret = -ENOTTY;
>
> + return ret;

[ Removed the '-' lines so you can see what the end result ends up being ]

Please, if you do this for the V4L2 layer, then DO NOT make the same
mistake we did with the vasic VFS layer.

In other words, DO NOT keep the "bkl" version named just "ioctl". It was a
horrible horrible mistake, and it has resulted in problems years
afterwards.

I realize that it's so easy to just add a new ".unlocked_ioctl" member,
and then as people start using it, they get rid of the BKL. But it's a
mistake. It was a mistake for the VFS layer, it would be a mistake for the
V4L2 layer.

Instead, spend the 15 minutes just renaming every current 'ioctl' user in
the V4L2 layer. It's not that much work, the scripts I documented in my
renaming patch do 95% of the work (you just need to change
"file_operations" to "v4l2_file_operations"). It's not that painful. And
then you don't just push the BKL down, you actually annotate the remaining
users so that they can be grepped for.

So please please please, don't make the same mistake we did long ago.

Linus