Prev: [GIT PULL] x86 fixes for 2.6.34-rc6
Next: [PATCH] Staging: hv: Fix coding style errors in NetVsc.c
From: Frederic Weisbecker on 28 Apr 2010 18:00
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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: [GIT PULL] x86 fixes for 2.6.34-rc6
Next: [PATCH] Staging: hv: Fix coding style errors in NetVsc.c