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

[Mauro Carvalho Chehab <mchehab@redhat.com>]

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.

> For example, anyone who uses the davinci
> dm6467 driver for both input and output. And yes, that's what we use at
> work. And ship to thousands of customers. Or think about surveillance
> applications where you are capturing from many streams simultaneously.

Cheers,
Mauro