Re: epoll and vb2_poll: can't wake_up

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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);
>         }
>
>         /*



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux