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 -- 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
