On Fri, Aug 14, 2015 at 01:57:51PM +0200, Hans Verkuil wrote:
> On 08/12/2015 05:50 PM, Kamil Debski wrote:
> > 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.
>
> Zahari, will you make a new version of this patch with the suggested changes?
yes I will do it.
Regards
Zahari
>
> Regards,
>
> Hans
> --
> 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
--
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
