From: Hans Verkuil <hverkuil@xs4all.nl>
To: Yi Qingliang <niqingliang2003@gmail.com>, linux-media@vger.kernel.org
Subject: Re: epoll and vb2_poll: can't wake_up
Date: Mon, 7 Jan 2019 14:45:32 +0100 [thread overview]
Message-ID: <3268a1a8-1712-52b2-e0e4-c6a98f003d75@xs4all.nl> (raw)
In-Reply-To: <CADwFkYdCXY5my5DW=qGJcJBDpjtZpRHXN6h4H2geneekiOzCgg@mail.gmail.com>
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);
}
/*
next prev parent reply other threads:[~2019-01-07 13:45 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-29 2:10 epoll and vb2_poll: can't wake_up Yi Qingliang
2019-01-07 13:45 ` Hans Verkuil [this message]
2019-01-07 14:15 ` Hans Verkuil
2019-01-07 14:29 ` Yi Qingliang
2019-01-09 1:30 ` Yi Qingliang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3268a1a8-1712-52b2-e0e4-c6a98f003d75@xs4all.nl \
--to=hverkuil@xs4all.nl \
--cc=linux-media@vger.kernel.org \
--cc=niqingliang2003@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).