Re: videobuf lock problem

["Hans Verkuil" <hverkuil@xxxxxxxxx>]

> Hi Hans,
>
> On Wed, 30 Jan 2008 11:52:40 +0100 (CET)
> "Hans Verkuil" <hverkuil@xxxxxxxxx> wrote:
>
>> Hi all,
>>
>> Am I missing something, or is there indeed a deadlock bug in
>> videobuf-core?
>>
>> At work we are using one thread that dequeues buffers from a v4l output
>> device and another thread that queues the filled buffers into the same
>> output device. However, videobuf_dqbuf() locks q->lock at the start and
>> never releases it until it gets the buffer. If there are no buffers
>> available, then it will wait without releasing the mutex! But
>> videobuf_qbuf() also tries to lock q->lock. So videobuf_dqbuf is waiting
>> for a new buffer to become available, but videobuf_qbuf cannot queue a
>> buffer become videobuf_dqbuf still has the mutex locked.
>>
>> I'm no videobuf expert, but I'd say that videobuf_dqbuf should release
>> the
>> q->lock mutex when calling videobuf_waiton(). This is probably true for
>> all calls to videobuf_waiton().
>>
>> I just hacked that in and it seems to work now (I just added an unlock
>> before the waiton() and a lock afterwards). But I don't know if there
>> are
>> any subtleties that I'm missing.
>>
>> Mauro, can you take a look at this?
>
> Videobuf is a very complex piece of software. It supports several
> different
> operation modes. Also, some boards touch on videobuf priv locks.
>
> I can't foresee any troubles with your approach, except for the fact that
> waiton() may be called externally. In fact, bttv, cx23885, saa7134,
> saa7146 and
> vivi calls waiton. This happens at interrupt time, when a buffer is
> filled.

waiton in interrupt time?! Yuck.

> So, if you simply do mutex_unlock() inside waiton(), you're doing
> something bad.
>
> I'll take a look for a proper solution. What's the driver you're using?
> What
> mode (read, userspace memory allocated mmap, kernelspace memory allocated
> mmap)?

It's using kernelspace mmap. The driver is for the TI DaVinciHD
System-on-a-Chip videoports. This driver is not in the kernel, but it is
GPL. It's not really relevant to this problem. For the time being I'll add
the unlock/locks in our kernel.

Thanks for the info!

Regards,

        Hans

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@xxxxxxxxxx?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list