Hi Hans, On Monday 07 May 2012 13:44:59 Hans Verkuil wrote: > On Monday 07 May 2012 13:06:01 Laurent Pinchart wrote: > > On Sunday 06 May 2012 16:54:40 Sander Eikelenboom wrote: > > > Hello Laurent / Mauro, > > > > > > I have updated to latest 3.4-rc5-tip, running multiple video grabbers. > > > I don't see anything specific to uvcvideo anymore, but i do get the > > > possible circular locking dependency below. > > > > Thanks for the report. > > > > We indeed have a serious issue there (CC'ing Hans Verkuil). > > > > Hans, serializing both ioctl handling an mmap with a single device lock as > > we currently do in V4L2 is prone to AB-BA deadlocks (uvcvideo shouldn't > > be affected as it has no device-wide lock). > > > > If we want to keep a device-wide lock we need to take it after the mm- > > > > >mmap_sem lock in all code paths, as there's no way we can change the lock > > > > ordering for mmap(). The copy_from_user/copy_to_user issue could be solved > > by moving locking from v4l2_ioctl to __video_do_ioctl (device-wide locks > > would then require using video_ioctl2), but I'm not sure whether that > > will play nicely with the QBUF implementation in videobuf2 (which already > > includes a workaround for this particular AB-BA deadlock issue). > > I've seen the same thing. It was on my TODO list of things to look into. I > think mmap shouldn't take the device wide lock at all. But it will mean > reviewing affected drivers before I can remove it. > > To be honest, I wasn't sure whether or not to take the device lock for mmap > when I first wrote that code. > > If you look at irc I had a discussion today with HdG about adding flags to > selectively disable locks for fops. It may be an idea to implement this soon > so we can start updating drivers one-by-one. > > Frankly, I do not believe this 'possible circular locking' thing to be a > real bug as I am not aware of drivers that use the device lock *and* lock > mmap_sem. If I understand Sander correctly 'motion' no longer locks up > after moving to 3.4-rc5-tip, right? copy_from_user()/copy_to_user() perform a down_read(&mm->mmap_sem) when a page fault occurs. All drivers thus potentially take mm->mmap_sem when an ioctl is performed. > I have to look into a bit more to see what the best approach it so we can > prevent this message from appearing. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
