All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: "Mauro Carvalho Chehab" <mchehab@redhat.com>,
	linux-media@vger.kernel.org,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [RFC PATCH 0/8] V4L BKL removal: first round
Date: Tue, 16 Nov 2010 20:23:17 +0100	[thread overview]
Message-ID: <201011162023.17671.arnd@arndb.de> (raw)
In-Reply-To: <201011161938.11476.hverkuil@xs4all.nl>

On Tuesday 16 November 2010, Hans Verkuil wrote:
> I consider class 3 unacceptable for commonly seen devices. I did a quick scan
> of the v4l drivers and the only common driver that falls in that class is uvc.

If uvc is the only important one, that should be easy enough to fix by adding
a per-device mutex around uvc_v4l2_do_ioctl() or uvc_v4l2_ioctl().

> There is one other option, although it is very dirty: don't take the lock if
> the ioctl command is VIDIOC_DQBUF. It works and reliably as well for uvc and
> videobuf (I did a quick code analysis). But I don't know if it works everywhere.
> 
> I would like to get the opinion of others before I implement such a check. But
> frankly, I think this may be our best bet.
> 
> So the patch below would look like this if I add the check:
> 
> -               mutex_lock(&v4l2_ioctl_mutex);
> +               if (cmd != VIDIOC_DQBUF)
> +                       mutex_lock(m);
>                 if (video_is_registered(vdev))
>                         ret = vdev->fops->ioctl(filp, cmd, arg);
> -               mutex_unlock(&v4l2_ioctl_mutex);
> +               if (cmd != VIDIOC_DQBUF)
> +                       mutex_unlock(m);
> 

I was thinking of this as well, but didn't bring it up because I considered
it too hacky.

The patch you posted looks good, thanks for bringing up the problem with
my patch and the solution!

Acked-by: Arnd Bergmann <arnd@arndb.de>

  reply	other threads:[~2010-11-16 19:22 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-14 13:21 [RFC PATCH 0/8] V4L BKL removal: first round Hans Verkuil
2010-11-14 13:22 ` [RFC PATCH 1/8] v4l2-dev: use mutex_lock_interruptible instead of plain mutex_lock Hans Verkuil
2010-11-14 13:22 ` [RFC PATCH 2/8] BKL: trivial BKL removal from V4L2 radio drivers Hans Verkuil
2010-11-14 13:22 ` [RFC PATCH 3/8] cadet: use unlocked_ioctl Hans Verkuil
2010-11-14 13:22 ` [RFC PATCH 4/8] tea5764: convert to unlocked_ioctl Hans Verkuil
2010-11-14 13:22 ` [RFC PATCH 5/8] si4713: " Hans Verkuil
2010-11-14 13:22 ` [RFC PATCH 6/8] typhoon: " Hans Verkuil
2010-11-14 13:23 ` [RFC PATCH 7/8] dsbr100: " Hans Verkuil
2010-11-14 13:23 ` [RFC PATCH 8/8] BKL: trivial ioctl -> unlocked_ioctl video driver conversions Hans Verkuil
2010-11-14 21:53 ` [RFC PATCH 0/8] V4L BKL removal: first round Arnd Bergmann
2010-11-14 22:48   ` Hans Verkuil
2010-11-15  9:17     ` Arnd Bergmann
2010-11-15  9:49       ` Hans Verkuil
2010-11-16 12:19         ` Mauro Carvalho Chehab
2010-11-16 12:35           ` Hans Verkuil
2010-11-16 13:06             ` Mauro Carvalho Chehab
2010-11-16 13:20               ` Hans Verkuil
2010-11-16 14:22                 ` Arnd Bergmann
2010-11-16 14:50                   ` Hans Verkuil
2010-11-16 15:13                     ` Arnd Bergmann
2010-11-16 15:27                       ` Hans Verkuil
2010-11-16 15:30                         ` Hans Verkuil
2010-11-16 16:01                           ` Arnd Bergmann
2010-11-16 16:32                             ` Mauro Carvalho Chehab
2010-11-16 16:49                             ` Hans Verkuil
2010-11-16 18:38                               ` Hans Verkuil
2010-11-16 19:23                                 ` Arnd Bergmann [this message]
2010-11-16 19:59                                 ` Andy Walls
2010-11-16 20:29                                   ` Hans Verkuil
2010-11-16 21:10                                     ` Hans Verkuil
2010-11-16 21:32                                       ` David Ellingsworth
2010-11-16 21:42                                         ` Hans Verkuil
2010-11-17 15:36                                           ` David Ellingsworth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201011162023.17671.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.