Re: [PATCH] v4l: Deadlock in videobuf-core for DQBUF waiting on QBUF

[Mauro Carvalho Chehab <mchehab@infradead.org>]

On Mon, 3 Mar 2008 00:13:05 -0800
Brandon Philips <brandon@ifup.org> wrote:

> On 06:34 Fri 29 Feb 2008, Mauro Carvalho Chehab wrote:
> > On Wed, 27 Feb 2008 18:56:51 -0800
> > Brandon Philips <brandon@ifup.org> wrote:
> > 
> > > On 09:08 Wed 27 Feb 2008, Jonathan Corbet wrote:
> > > > Brandon Philips <brandon@ifup.org> wrote:
> > > > 
> > > > >  	buf = list_entry(q->stream.next, struct videobuf_buffer, stream);
> > > > > +	mutex_unlock(&q->vb_lock);
> > > > >  	retval = videobuf_waiton(buf, nonblocking, 1);
> > > > > +	mutex_lock(&q->vb_lock);
> > > > 
> > > > Are you sure that this doesn't create a race where two threads could end
> > > > up waiting on the same buf?  
> > > 
> > > You are right... I thought I had thought through this but a race can be
> > > created with two threads doing DQBUF.
> > > 
> > > > Actually, almost anything could happen to buf by the time you've
> > > > gotten the mutex back - it might not even exist anymore - but there
> > > > are no checks for that.  It seems like a better fix might be to set
> > > > nonblocking unconditionally to 1 for the videobuf_waiton() call, then
> > > > start over from the beginning on a -EAGAIN return (if the caller has
> > > > not requested nonblocking behavior).
> > > 
> > > Hrm, that is one solution.  I will think about it for a bit and submit a
> > > new patch.
> > > 
> > > Thanks for catching this, I was being stupid.
> > > 
> > > Mauro: Please don't push this patch out.  Thanks.
> > 
> > The patch were already applied at the staging tree. I'll keep it there until we
> > have a definitive solution. After that, the better would be to fold the both
> > patches and send to 2.6.25-rc.
> 
> Quick update:
> 
> Sorry for the delay I was mentoring my robotics team since Thursday.
> 
> I wrote a test program (attached, super hacky) and I have found that the
> current vivi can easily hang a kernel on a spinlock even before my
> patch...

Argh!
> 
> I will work on fixing this tomorrow before fixing the videobuf issue.  I
> would like to be able to test my fix before submitting it this time.
This would be nice.

About your testing program, it would be a good idea to add it at v4l2-apps/test
dir. The original non-threaded code from V4L2 specs is also there.

Cheers,
Mauro

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