On Tuesday, October 19, 2010 05:20:17 Jonathan Corbet wrote: > On Sat, 16 Oct 2010 10:44:56 -0300 > Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxx> wrote: > > > drivers/media/video/via-camera.c: In function ‘viacam_open’: > > drivers/media/video/via-camera.c:651: error: too few arguments to function ‘videobuf_queue_sg_init’ > > > The fix for this one is trivial: > > drivers/media/video/via-camera.c:651: error: too few arguments to function ‘videobuf_queue_sg_init’ > > > > Just add an extra NULL parameter to the function. > > So I'm looking into this stuff. The extra NULL parameter is a struct > mutex, which seems to be used in one place in videobuf_waiton(): > > is_ext_locked = q->ext_lock && mutex_is_locked(q->ext_lock); > > /* Release vdev lock to prevent this wait from blocking outside access to > the device. */ > if (is_ext_locked) > mutex_unlock(q->ext_lock); > > I'd be most curious to know what the reasoning behind this code is; to my > uneducated eye, it looks like a real hack. How does this function know who > locked ext_lock? Can it really just unlock it safely? It seems to me that > this is a sign of locking issues which should really be dealt with > elsewhere, but, as I said, I'm uneducated, and the changelogs don't help me > much. Can somebody educate me? We are working on removing the BKL. As part of that effort it is now possible for drivers to pass a serialization mutex to the v4l core (a mutex pointer was added to struct video_device). If the core sees that mutex then the core will serialize all open/ioctl/read/write/etc. file ops. So all file ops will in that case be called with that mutex held. Which is fine, but if the driver has to do a blocking wait, then you need to unlock the mutex first and lock it again afterwards. And since videobuf does a blocking wait it needs to know about that mutex. Right now we are in the middle of the transition from BKL to using core locks (and some drivers will do their own locking completely). During this transition period we have drivers that provide an external lock and drivers still relying on the BKL in which case videobuf needs to handle its own locking. Hopefully in 1-2 kernel cycles we will have abolished the BKL and we can remove the videobuf internal lock and use the external mutex only. So yes, it is a bit of a hack but there is actually a plan behind it :-) It's a common theme inside the v4l subsystem, I'm afraid. We are still in the process of creating a fully functional v4l core framework that all drivers can use. But for every new piece of core functionality it means a transition period where drivers are one by one converted to use it. That can take a very long time. Add to that mix all the new functionality that is being added to support embedded video hardware and it can get complex indeed. I have a very clear goal in mind, though, and I (and many others!) are steadily moving closer to that goal. Every kernel release has one or more essential building blocks in place. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- 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
