Re: [RFC PATCH 0/8] V4L BKL removal: first round

["Hans Verkuil" <hverkuil@xs4all.nl>]

> Em 16-11-2010 10:35, Hans Verkuil escreveu:
>>
>>> Em 15-11-2010 07:49, Hans Verkuil escreveu:
>>>>
>>>>> On Sunday 14 November 2010 23:48:51 Hans Verkuil wrote:
>>>>>> On Sunday, November 14, 2010 22:53:29 Arnd Bergmann wrote:
>>>>>>> On Sunday 14 November 2010, Hans Verkuil wrote:
>>>>>>>> This patch series converts 24 v4l drivers to unlocked_ioctl. These
>>>>>> are low
>>>>>>>> hanging fruit but you have to start somewhere :-)
>>>>>>>>
>>>>>>>> The first patch replaces mutex_lock in the V4L2 core by
>>>>>> mutex_lock_interruptible
>>>>>>>> for most fops.
>>>>>>>
>>>>>>> The patches all look good as far as I can tell, but I suppose the
>>>>>> title is
>>>>>>> obsolete now that the BKL has been replaced with a v4l-wide mutex,
>>>>>> which
>>>>>>> is what you are removing in the series.
>>>>>>
>>>>>> I guess I have to rename it, even though strictly speaking the
>>>>>> branch
>>>>>> I'm
>>>>>> working in doesn't have your patch merged yet.
>>>>>>
>>>>>> BTW, replacing the BKL with a static mutex is rather scary: the BKL
>>>>>> gives up
>>>>>> the lock whenever you sleep, the mutex doesn't. Since sleeping is
>>>>>> very
>>>>>> common
>>>>>> in V4L (calling VIDIOC_DQBUF will typically sleep while waiting for
>>>>>> a
>>>>>> new frame
>>>>>> to arrive), this will make it impossible for another process to
>>>>>> access
>>>>>> any
>>>>>> v4l2 device node while the ioctl is sleeping.
>>>>>>
>>>>>> I am not sure whether that is what you intended. Or am I missing
>>>>>> something?
>>>>>
>>>>> I was aware that something like this could happen, but I apparently
>>>>> misjudged how big the impact is. The general pattern for ioctls is
>>>>> that
>>>>> those that get called frequently do not sleep, so it can almost
>>>>> always
>>>>> be
>>>>> called with a mutex held.
>>>>
>>>> True in general, but most definitely not true for V4L. The all
>>>> important
>>>> VIDIOC_DQBUF ioctl will almost always sleep.
>>>>
>>>> Mauro, I think this patch will have to be reverted and we just have to
>>>> do
>>>> the hard work ourselves.
>>>
>>> The VIDIOC_QBUF/VIDIOC_DQBUF ioctls are called after having the V4L
>>> device
>>> ready
>>> for stream. During the qbuf/dqbuf loop, the only other ioctls that may
>>> appear are
>>> the control change ioctl's, to adjust things like bright. I doubt that
>>> his
>>> will
>>> cause a really serious trouble.
>>
>> Yes, it does. Anyone who is using multiple capture/output devices at the
>> same time will be affected.
>
> One correction to your comment:
> 	"Anyone that uses multiple capture/output devices that were not converted
> to unlocked ioctl will be affected."
> This means that devices with multiple entries need to be fixed first.

No, it will also affect e.g. two bttv cards that you capture from in
parallel. Or two webcams, or...

We can't just ditch the BKL yet for 2.6.37 IMHO. Perhaps for 2.6.38 if we
all work really hard to convert everything.

I would prefer to have those drivers that depend on the BKL to have a
dependency on CONFIG_LOCK_KERNEL. Drivers should either work or not be
available at all, rather than working poorly (if at all).

Regards,

          Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco