Messages in this thread | | | Date | Wed, 28 Apr 2010 07:55:02 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: [PATCH 10/10] bkl: Fix-up compile problems as a result of the bkl-pushdown. |
| |
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
| |