lkml.org 
[lkml]   [2010]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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.



    \
     
     \ /
      Last update: 2010-04-28 23:55    [from the cache]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean