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

[Linus Torvalds]

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