Hi,
On 12 August 2015 at 13:42, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote:
> Hello Hans,
>
> I'm sorry for a delay. Once again I've been busy with some other internal
> stuff.
>
> On 2015-07-28 11:02, Hans Verkuil wrote:
>>
>> Kamil, Marek,
>>
>> Why does v4l2_m2m_poll unlock and lock in that function?
>
>
> I've checked the code and indeed the poll_wait() function doesn't do
> anything that
> should not be done with queue mutex being taken. I don't remember if it was
> always
> like that. You are right that the unlock&lock code should be removed.
>
>> Zahari is right that the locking is unbalanced, but I don't see the reason
>> for the unlock/lock sequence in the first place. I'm wondering if that
>> shouldn't just be removed.
>>
>> Am I missing something?
>>
>> Instead, I would expect to see a spin_lock_irqsave(&src/dst_q->done_lock,
>> flags)
>> around the list_empty(&src/dst_q->done_list) calls.
>
>
> Indeed, that's another thing that should be fixed in this function. I looks
> that
> commit c16218402a000bb25c1277c43ae98c11bcb59bd1 ("[media] videobuf2: return
> -EPIPE
> from DQBUF after the last buffer") is the root cause of both issues
> (unballanced
> locking and lack of spinlock protection), while the unnecessary queue
> unlock/lock
> sequence was there from the beginning.
>
I am all with Marek on this. Unlock/lock was there from the beginning,
it is not necessary. I agree also that spin_lock/unlock should be
added for the list_empty call.
[snip]
Best wishes,
Kamil
--
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
