Re: epoll and vb2_poll: can't wake_up

[Yi Qingliang <niqingliang2003@gmail.com>]

the first patch can work on freescale's 4.1.2 kernel ! My case maybe
can't cover other changes.

in file drivers/media/v4l2-core/videobuf-core.c, the function
'videobuf_poll_stream' may return POLLERR without calling 'poll_wait',
it looks like different with 'vb2_poll''s process, maybe need further
check.

Thanks!

Yi Qingliang


On Mon, Jan 7, 2019 at 2:29 PM Yi Qingliang <niqingliang2003@gmail.com> wrote:
>
> Thanks! It should work now.
> BTW, I don't know if we should think about the error case before
> calling poll_wait, just like not streamon.
> if poll return error, does epoll framework need and how to remove
> waiter for client?
> for epoll framework, does it have some requirements or some tutorial
> for the implementation of client's poll?
>
> and I think it's better to split the two operation: adding waiter and
> polling, not only for epoll framework, and also for all clients.
>
> Yi Qingliang
>
> On Mon, Jan 7, 2019 at 1:45 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >
> > On 12/29/2018 03:10 AM, Yi Qingliang wrote:
> > > Hello, I encountered a "can't wake_up" problem when use camera on imx6.
> > >
> > > if delay some time after 'streamon' the /dev/video0, then add fd
> > > through epoll_ctl, then the process can't be waken_up after some time.
> > >
> > > I checked both the epoll / vb2_poll(videobuf2_core.c) code.
> > >
> > > epoll will pass 'poll_table' structure to vb2_poll, but it only
> > > contain valid function pointer when inserting fd.
> > >
> > > in vb2_poll, if found new data in done list, it will not call 'poll_wait'.
> > > after that, every call to vb2_poll will not contain valid poll_table,
> > > which will result in all calling to poll_wait will not work.
> > >
> > > so if app can process frames quickly, and found frame data when
> > > inserting fd (i.e. poll_wait will not be called or not contain valid
> > > function pointer), it will not found valid frame in 'vb2_poll' finally
> > > at some time, then call 'poll_wait' to expect be waken up at following
> > > vb2_buffer_done, but no good luck.
> > >
> > > I also checked the 'videobuf-core.c', there is no this problem.
> > >
> > > of course, both epoll and vb2_poll are right by itself side, but the
> > > result is we can't get new frames.
> > >
> > > I think by epoll's implementation, the user should always call poll_wait.
> > >
> > > and it's better to split the two actions: 'wait' and 'poll' both for
> > > epoll framework and all epoll users, for example, v4l2.
> > >
> > > am I right?
> > >
> > > Yi Qingliang
> > >
> >
> > Can you test this patch?
> >
> > Looking at what other drivers/frameworks do it seems that calling
> > poll_wait() at the start of the poll function is the right approach.
> >
> > Regards,
> >
> >         Hans
> >
> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > ---
> > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> > index 70e8c3366f9c..b1809628475d 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-core.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> > @@ -2273,6 +2273,8 @@ __poll_t vb2_core_poll(struct vb2_queue *q, struct file *file,
> >         struct vb2_buffer *vb = NULL;
> >         unsigned long flags;
> >
> > +       poll_wait(file, &q->done_wq, wait);
> > +
> >         if (!q->is_output && !(req_events & (EPOLLIN | EPOLLRDNORM)))
> >                 return 0;
> >         if (q->is_output && !(req_events & (EPOLLOUT | EPOLLWRNORM)))
> > @@ -2329,8 +2331,6 @@ __poll_t vb2_core_poll(struct vb2_queue *q, struct file *file,
> >                  */
> >                 if (q->last_buffer_dequeued)
> >                         return EPOLLIN | EPOLLRDNORM;
> > -
> > -               poll_wait(file, &q->done_wq, wait);
> >         }
> >
> >         /*