Messages in this thread | | | Date | Wed, 28 Apr 2010 23:52:16 +0200 | From | Frederic Weisbecker <> | Subject | Re: [PATCH 10/10] bkl: Fix-up compile problems as a result of the bkl-pushdown. |
| |
On Wed, Apr 28, 2010 at 07:55:02AM -0700, Linus Torvalds wrote: > > > 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
Hmm, there are 92 struct v4l2_file_operations::ioctl but actually a lot of duplicates ioctl. In fact there are just 26 ioctl functions.
It's probably worth the whole pushdown instead of the rename.
I'm going to do this.
| |