Linux-Media Archive on lore.kernel.org
 help / Atom feed
* [RFC PATCH 0/8] cec/mc/vb2/dvb: fix epoll support
@ 2019-02-07 11:49 hverkuil-cisco
  2019-02-07 11:49 ` [RFC PATCH 1/8] cec: fix epoll() by calling poll_wait first hverkuil-cisco
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: hverkuil-cisco @ 2019-02-07 11:49 UTC (permalink / raw)
  To: linux-media; +Cc: Michael Ira Krufky, Brad Love, Sakari Ailus

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

As was reported by Yi Qingliang (http://lkml.iu.edu/hypermail/linux/kernel/1812.3/02144.html)
the epoll support in v4l2 is broken.

After researching this some more it turns out that we never
really understood when poll_wait() should be called, and that in
fact it is broken in quite a few places in our media tree, and not
just v4l2.

The select() call is fairly simplistic: it calls the poll fop
first, then waits for an event if the poll fop returned 0.

The epoll() call is more complicated: epoll_ctl(EPOLL_CTL_ADD) will
call the poll fop which calls poll_wait in turn.

But epoll_wait() just waits for events to arrive on the registered
waitqueues, and does not call the poll fop until it is woken up.

So not calling poll_wait() in the poll fop will cause epoll_wait()
to wait forever (or until the timeout is reached).

This patch series just calls poll_wait() regardless of whether there
is an event pending.

It does this for all the various frameworks that did this wrong.

Note that there is also an extra mem2mem patch that adds a check for
q->error, which I noticed was missing.

I have not tested the videobuf and esp. the dvb-core changes. They
look sane, but it doesn't hurt to give those extra attention.

There are also older drivers that call poll_wait themselves. While
I have some patches for those (look in
https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=poll), they
need more review. I prefer to do the core frameworks first.

Several of the patches in this series should probably be CC-ed to
stable. I'll take a look at that once this RFC series gets the
green light.

Regards,

	Hans

Hans Verkuil (8):
  cec: fix epoll() by calling poll_wait first
  media-request: fix epoll() by calling poll_wait first
  vb2: fix epoll() by calling poll_wait first
  v4l2-ctrls.c: fix epoll() by calling poll_wait first
  v4l2-mem2mem: fix epoll() by calling poll_wait first
  v4l2-mem2mem: add q->error check to v4l2_m2m_poll()
  videobuf: fix epoll() by calling poll_wait first
  dvb-core: fix epoll() by calling poll_wait first

 drivers/media/cec/cec-api.c                   |  2 +-
 .../media/common/videobuf2/videobuf2-core.c   |  4 +--
 .../media/common/videobuf2/videobuf2-v4l2.c   |  4 +--
 drivers/media/dvb-core/dmxdev.c               |  8 +++---
 drivers/media/dvb-core/dvb_ca_en50221.c       |  5 ++--
 drivers/media/media-request.c                 |  3 +--
 drivers/media/v4l2-core/v4l2-ctrls.c          |  2 +-
 drivers/media/v4l2-core/v4l2-mem2mem.c        | 25 ++++++++-----------
 drivers/media/v4l2-core/videobuf-core.c       |  6 ++---
 9 files changed, 26 insertions(+), 33 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH 1/8] cec: fix epoll() by calling poll_wait first
  2019-02-07 11:49 [RFC PATCH 0/8] cec/mc/vb2/dvb: fix epoll support hverkuil-cisco
@ 2019-02-07 11:49 ` hverkuil-cisco
  2019-02-08 20:53   ` Sean Young
  2019-02-07 11:49 ` [RFC PATCH 2/8] media-request: " hverkuil-cisco
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: hverkuil-cisco @ 2019-02-07 11:49 UTC (permalink / raw)
  To: linux-media; +Cc: Michael Ira Krufky, Brad Love, Sakari Ailus, Hans Verkuil

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

The epoll function expects that whenever the poll file op is
called, the poll_wait function is also called. That didn't
always happen in cec_poll(). Fix this, otherwise epoll()
would timeout when it shouldn't.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/cec-api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/cec/cec-api.c b/drivers/media/cec/cec-api.c
index 391b6fd483e1..156a0d76ab2a 100644
--- a/drivers/media/cec/cec-api.c
+++ b/drivers/media/cec/cec-api.c
@@ -38,6 +38,7 @@ static __poll_t cec_poll(struct file *filp,
 	struct cec_adapter *adap = fh->adap;
 	__poll_t res = 0;
 
+	poll_wait(filp, &fh->wait, poll);
 	if (!cec_is_registered(adap))
 		return EPOLLERR | EPOLLHUP;
 	mutex_lock(&adap->lock);
@@ -48,7 +49,6 @@ static __poll_t cec_poll(struct file *filp,
 		res |= EPOLLIN | EPOLLRDNORM;
 	if (fh->total_queued_events)
 		res |= EPOLLPRI;
-	poll_wait(filp, &fh->wait, poll);
 	mutex_unlock(&adap->lock);
 	return res;
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH 2/8] media-request: fix epoll() by calling poll_wait first
  2019-02-07 11:49 [RFC PATCH 0/8] cec/mc/vb2/dvb: fix epoll support hverkuil-cisco
  2019-02-07 11:49 ` [RFC PATCH 1/8] cec: fix epoll() by calling poll_wait first hverkuil-cisco
@ 2019-02-07 11:49 ` " hverkuil-cisco
  2019-02-07 11:49 ` [RFC PATCH 3/8] vb2: " hverkuil-cisco
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: hverkuil-cisco @ 2019-02-07 11:49 UTC (permalink / raw)
  To: linux-media; +Cc: Michael Ira Krufky, Brad Love, Sakari Ailus, Hans Verkuil

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

The epoll function expects that whenever the poll file op is
called, the poll_wait function is also called. That didn't
always happen in media_request_poll(). Fix this, otherwise
epoll() would timeout when it shouldn't.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/media-request.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
index c71a34ae6383..eec2e2b2f6ec 100644
--- a/drivers/media/media-request.c
+++ b/drivers/media/media-request.c
@@ -100,6 +100,7 @@ static __poll_t media_request_poll(struct file *filp,
 	if (!(poll_requested_events(wait) & EPOLLPRI))
 		return 0;
 
+	poll_wait(filp, &req->poll_wait, wait);
 	spin_lock_irqsave(&req->lock, flags);
 	if (req->state == MEDIA_REQUEST_STATE_COMPLETE) {
 		ret = EPOLLPRI;
@@ -110,8 +111,6 @@ static __poll_t media_request_poll(struct file *filp,
 		goto unlock;
 	}
 
-	poll_wait(filp, &req->poll_wait, wait);
-
 unlock:
 	spin_unlock_irqrestore(&req->lock, flags);
 	return ret;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH 3/8] vb2: fix epoll() by calling poll_wait first
  2019-02-07 11:49 [RFC PATCH 0/8] cec/mc/vb2/dvb: fix epoll support hverkuil-cisco
  2019-02-07 11:49 ` [RFC PATCH 1/8] cec: fix epoll() by calling poll_wait first hverkuil-cisco
  2019-02-07 11:49 ` [RFC PATCH 2/8] media-request: " hverkuil-cisco
@ 2019-02-07 11:49 ` " hverkuil-cisco
  2019-02-07 11:49 ` [RFC PATCH 4/8] v4l2-ctrls.c: " hverkuil-cisco
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: hverkuil-cisco @ 2019-02-07 11:49 UTC (permalink / raw)
  To: linux-media
  Cc: Michael Ira Krufky, Brad Love, Sakari Ailus, Hans Verkuil, Yi Qingliang

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

The epoll function expects that whenever the poll file op is
called, the poll_wait function is also called. That didn't
always happen in vb2_core_poll() and vb2_poll(). Fix this,
otherwise epoll() would timeout when it shouldn't.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reported-by: Yi Qingliang <niqingliang2003@gmail.com>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 4 ++--
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 4 +---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index e07b6bdb6982..b40e9f48a943 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -2289,6 +2289,8 @@ __poll_t vb2_core_poll(struct vb2_queue *q, struct file *file,
 	if (q->is_output && !(req_events & (EPOLLOUT | EPOLLWRNORM)))
 		return 0;
 
+	poll_wait(file, &q->done_wq, wait);
+
 	/*
 	 * Start file I/O emulator only if streaming API has not been used yet.
 	 */
@@ -2340,8 +2342,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);
 	}
 
 	/*
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 3aeaea3af42a..101479768f91 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -867,16 +867,14 @@ EXPORT_SYMBOL_GPL(vb2_queue_release);
 __poll_t vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
 {
 	struct video_device *vfd = video_devdata(file);
-	__poll_t req_events = poll_requested_events(wait);
 	__poll_t res = 0;
 
 	if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)) {
 		struct v4l2_fh *fh = file->private_data;
 
+		poll_wait(file, &fh->wait, wait);
 		if (v4l2_event_pending(fh))
 			res = EPOLLPRI;
-		else if (req_events & EPOLLPRI)
-			poll_wait(file, &fh->wait, wait);
 	}
 
 	return res | vb2_core_poll(q, file, wait);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH 4/8] v4l2-ctrls.c: fix epoll() by calling poll_wait first
  2019-02-07 11:49 [RFC PATCH 0/8] cec/mc/vb2/dvb: fix epoll support hverkuil-cisco
                   ` (2 preceding siblings ...)
  2019-02-07 11:49 ` [RFC PATCH 3/8] vb2: " hverkuil-cisco
@ 2019-02-07 11:49 ` " hverkuil-cisco
  2019-02-07 11:49 ` [RFC PATCH 5/8] v4l2-mem2mem: " hverkuil-cisco
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: hverkuil-cisco @ 2019-02-07 11:49 UTC (permalink / raw)
  To: linux-media; +Cc: Michael Ira Krufky, Brad Love, Sakari Ailus, Hans Verkuil

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

The epoll function expects that whenever the poll file op is
called, the poll_wait function is also called. That didn't
always happen in v4l2_ctrl_poll(). Fix this, otherwise epoll()
would timeout when it shouldn't.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 99308dac2daa..b79d3bbd8350 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -4166,9 +4166,9 @@ __poll_t v4l2_ctrl_poll(struct file *file, struct poll_table_struct *wait)
 {
 	struct v4l2_fh *fh = file->private_data;
 
+	poll_wait(file, &fh->wait, wait);
 	if (v4l2_event_pending(fh))
 		return EPOLLPRI;
-	poll_wait(file, &fh->wait, wait);
 	return 0;
 }
 EXPORT_SYMBOL(v4l2_ctrl_poll);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH 5/8] v4l2-mem2mem: fix epoll() by calling poll_wait first
  2019-02-07 11:49 [RFC PATCH 0/8] cec/mc/vb2/dvb: fix epoll support hverkuil-cisco
                   ` (3 preceding siblings ...)
  2019-02-07 11:49 ` [RFC PATCH 4/8] v4l2-ctrls.c: " hverkuil-cisco
@ 2019-02-07 11:49 ` " hverkuil-cisco
  2019-02-07 11:49 ` [RFC PATCH 6/8] v4l2-mem2mem: add q->error check to v4l2_m2m_poll() hverkuil-cisco
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: hverkuil-cisco @ 2019-02-07 11:49 UTC (permalink / raw)
  To: linux-media; +Cc: Michael Ira Krufky, Brad Love, Sakari Ailus, Hans Verkuil

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

The epoll function expects that whenever the poll file op is
called, the poll_wait function is also called. That didn't
always happen in v4l2_m2m_poll(). Fix this, otherwise
epoll() would timeout when it shouldn't.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 631f4e2aa942..d97781b8ff88 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -617,20 +617,22 @@ __poll_t v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
 	__poll_t rc = 0;
 	unsigned long flags;
 
+	src_q = v4l2_m2m_get_src_vq(m2m_ctx);
+	dst_q = v4l2_m2m_get_dst_vq(m2m_ctx);
+
+	poll_wait(file, &src_q->done_wq, wait);
+	poll_wait(file, &dst_q->done_wq, wait);
+
 	if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)) {
 		struct v4l2_fh *fh = file->private_data;
 
+		poll_wait(file, &fh->wait, wait);
 		if (v4l2_event_pending(fh))
 			rc = EPOLLPRI;
-		else if (req_events & EPOLLPRI)
-			poll_wait(file, &fh->wait, wait);
 		if (!(req_events & (EPOLLOUT | EPOLLWRNORM | EPOLLIN | EPOLLRDNORM)))
 			return rc;
 	}
 
-	src_q = v4l2_m2m_get_src_vq(m2m_ctx);
-	dst_q = v4l2_m2m_get_dst_vq(m2m_ctx);
-
 	/*
 	 * There has to be at least one buffer queued on each queued_list, which
 	 * means either in driver already or waiting for driver to claim it
@@ -642,11 +644,6 @@ __poll_t v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
 		goto end;
 	}
 
-	spin_lock_irqsave(&src_q->done_lock, flags);
-	if (list_empty(&src_q->done_list))
-		poll_wait(file, &src_q->done_wq, wait);
-	spin_unlock_irqrestore(&src_q->done_lock, flags);
-
 	spin_lock_irqsave(&dst_q->done_lock, flags);
 	if (list_empty(&dst_q->done_list)) {
 		/*
@@ -657,8 +654,6 @@ __poll_t v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
 			spin_unlock_irqrestore(&dst_q->done_lock, flags);
 			return rc | EPOLLIN | EPOLLRDNORM;
 		}
-
-		poll_wait(file, &dst_q->done_wq, wait);
 	}
 	spin_unlock_irqrestore(&dst_q->done_lock, flags);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH 6/8] v4l2-mem2mem: add q->error check to v4l2_m2m_poll()
  2019-02-07 11:49 [RFC PATCH 0/8] cec/mc/vb2/dvb: fix epoll support hverkuil-cisco
                   ` (4 preceding siblings ...)
  2019-02-07 11:49 ` [RFC PATCH 5/8] v4l2-mem2mem: " hverkuil-cisco
@ 2019-02-07 11:49 ` hverkuil-cisco
  2019-02-07 11:49 ` [RFC PATCH 7/8] videobuf: fix epoll() by calling poll_wait first hverkuil-cisco
  2019-02-07 11:49 ` [RFC PATCH 8/8] dvb-core: " hverkuil-cisco
  7 siblings, 0 replies; 11+ messages in thread
From: hverkuil-cisco @ 2019-02-07 11:49 UTC (permalink / raw)
  To: linux-media; +Cc: Michael Ira Krufky, Brad Love, Sakari Ailus, Hans Verkuil

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

The v4l2_m2m_poll function didn't check whether q->error
was set for either of the two queues. Add support for this.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index d97781b8ff88..d2da5249b61b 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -638,8 +638,10 @@ __poll_t v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
 	 * means either in driver already or waiting for driver to claim it
 	 * and start processing.
 	 */
-	if ((!src_q->streaming || list_empty(&src_q->queued_list))
-		&& (!dst_q->streaming || list_empty(&dst_q->queued_list))) {
+	if ((!src_q->streaming || src_q->error ||
+	     list_empty(&src_q->queued_list)) &&
+	    (!dst_q->streaming || dst_q->error ||
+	     list_empty(&dst_q->queued_list))) {
 		rc |= EPOLLERR;
 		goto end;
 	}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH 7/8] videobuf: fix epoll() by calling poll_wait first
  2019-02-07 11:49 [RFC PATCH 0/8] cec/mc/vb2/dvb: fix epoll support hverkuil-cisco
                   ` (5 preceding siblings ...)
  2019-02-07 11:49 ` [RFC PATCH 6/8] v4l2-mem2mem: add q->error check to v4l2_m2m_poll() hverkuil-cisco
@ 2019-02-07 11:49 ` hverkuil-cisco
  2019-02-07 11:49 ` [RFC PATCH 8/8] dvb-core: " hverkuil-cisco
  7 siblings, 0 replies; 11+ messages in thread
From: hverkuil-cisco @ 2019-02-07 11:49 UTC (permalink / raw)
  To: linux-media; +Cc: Michael Ira Krufky, Brad Love, Sakari Ailus, Hans Verkuil

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

The epoll function expects that whenever the poll file op is
called, the poll_wait function is also called. That didn't
always happen in videobuf_poll_stream(). Fix this, otherwise
epoll() would timeout when it shouldn't.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/videobuf-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf-core.c b/drivers/media/v4l2-core/videobuf-core.c
index 7491b337002c..bbef899f0bf7 100644
--- a/drivers/media/v4l2-core/videobuf-core.c
+++ b/drivers/media/v4l2-core/videobuf-core.c
@@ -1119,13 +1119,14 @@ ssize_t videobuf_read_stream(struct videobuf_queue *q,
 EXPORT_SYMBOL_GPL(videobuf_read_stream);
 
 __poll_t videobuf_poll_stream(struct file *file,
-				  struct videobuf_queue *q,
-				  poll_table *wait)
+			      struct videobuf_queue *q,
+			      poll_table *wait)
 {
 	__poll_t req_events = poll_requested_events(wait);
 	struct videobuf_buffer *buf = NULL;
 	__poll_t rc = 0;
 
+	poll_wait(file, &buf->done, wait);
 	videobuf_queue_lock(q);
 	if (q->streaming) {
 		if (!list_empty(&q->stream))
@@ -1149,7 +1150,6 @@ __poll_t videobuf_poll_stream(struct file *file,
 		rc = EPOLLERR;
 
 	if (0 == rc) {
-		poll_wait(file, &buf->done, wait);
 		if (buf->state == VIDEOBUF_DONE ||
 		    buf->state == VIDEOBUF_ERROR) {
 			switch (q->type) {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH 8/8] dvb-core: fix epoll() by calling poll_wait first
  2019-02-07 11:49 [RFC PATCH 0/8] cec/mc/vb2/dvb: fix epoll support hverkuil-cisco
                   ` (6 preceding siblings ...)
  2019-02-07 11:49 ` [RFC PATCH 7/8] videobuf: fix epoll() by calling poll_wait first hverkuil-cisco
@ 2019-02-07 11:49 ` " hverkuil-cisco
  2019-02-08 21:12   ` Sean Young
  7 siblings, 1 reply; 11+ messages in thread
From: hverkuil-cisco @ 2019-02-07 11:49 UTC (permalink / raw)
  To: linux-media; +Cc: Michael Ira Krufky, Brad Love, Sakari Ailus, Hans Verkuil

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

The epoll function expects that whenever the poll file op is
called, the poll_wait function is also called. That didn't
always happen in dvb_demux_poll(), dvb_dvr_poll() and
dvb_ca_en50221_io_poll(). Fix this, otherwise epoll()
can timeout when it shouldn't.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/dvb-core/dmxdev.c         | 8 ++++----
 drivers/media/dvb-core/dvb_ca_en50221.c | 5 ++---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 1544e8cef564..f14a872d1268 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -1195,13 +1195,13 @@ static __poll_t dvb_demux_poll(struct file *file, poll_table *wait)
 	struct dmxdev_filter *dmxdevfilter = file->private_data;
 	__poll_t mask = 0;
 
+	poll_wait(file, &dmxdevfilter->buffer.queue, wait);
+
 	if ((!dmxdevfilter) || dmxdevfilter->dev->exit)
 		return EPOLLERR;
 	if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx))
 		return dvb_vb2_poll(&dmxdevfilter->vb2_ctx, file, wait);
 
-	poll_wait(file, &dmxdevfilter->buffer.queue, wait);
-
 	if (dmxdevfilter->state != DMXDEV_STATE_GO &&
 	    dmxdevfilter->state != DMXDEV_STATE_DONE &&
 	    dmxdevfilter->state != DMXDEV_STATE_TIMEDOUT)
@@ -1346,13 +1346,13 @@ static __poll_t dvb_dvr_poll(struct file *file, poll_table *wait)
 
 	dprintk("%s\n", __func__);
 
+	poll_wait(file, &dmxdev->dvr_buffer.queue, wait);
+
 	if (dmxdev->exit)
 		return EPOLLERR;
 	if (dvb_vb2_is_streaming(&dmxdev->dvr_vb2_ctx))
 		return dvb_vb2_poll(&dmxdev->dvr_vb2_ctx, file, wait);
 
-	poll_wait(file, &dmxdev->dvr_buffer.queue, wait);
-
 	if (((file->f_flags & O_ACCMODE) == O_RDONLY) ||
 	    dmxdev->may_do_mmap) {
 		if (dmxdev->dvr_buffer.error)
diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index 4d371cea0d5d..ebf1e3b03819 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1797,6 +1797,8 @@ static __poll_t dvb_ca_en50221_io_poll(struct file *file, poll_table *wait)
 
 	dprintk("%s\n", __func__);
 
+	poll_wait(file, &ca->wait_queue, wait);
+
 	if (dvb_ca_en50221_io_read_condition(ca, &result, &slot) == 1)
 		mask |= EPOLLIN;
 
@@ -1804,9 +1806,6 @@ static __poll_t dvb_ca_en50221_io_poll(struct file *file, poll_table *wait)
 	if (mask)
 		return mask;
 
-	/* wait for something to happen */
-	poll_wait(file, &ca->wait_queue, wait);
-
 	if (dvb_ca_en50221_io_read_condition(ca, &result, &slot) == 1)
 		mask |= EPOLLIN;
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 1/8] cec: fix epoll() by calling poll_wait first
  2019-02-07 11:49 ` [RFC PATCH 1/8] cec: fix epoll() by calling poll_wait first hverkuil-cisco
@ 2019-02-08 20:53   ` Sean Young
  0 siblings, 0 replies; 11+ messages in thread
From: Sean Young @ 2019-02-08 20:53 UTC (permalink / raw)
  To: hverkuil-cisco; +Cc: linux-media, Michael Ira Krufky, Brad Love, Sakari Ailus

On Thu, Feb 07, 2019 at 12:49:41PM +0100, hverkuil-cisco@xs4all.nl wrote:
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> The epoll function expects that whenever the poll file op is
> called, the poll_wait function is also called. That didn't
> always happen in cec_poll(). Fix this, otherwise epoll()
> would timeout when it shouldn't.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/cec/cec-api.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/cec/cec-api.c b/drivers/media/cec/cec-api.c
> index 391b6fd483e1..156a0d76ab2a 100644
> --- a/drivers/media/cec/cec-api.c
> +++ b/drivers/media/cec/cec-api.c
> @@ -38,6 +38,7 @@ static __poll_t cec_poll(struct file *filp,
>  	struct cec_adapter *adap = fh->adap;
>  	__poll_t res = 0;
>  
> +	poll_wait(filp, &fh->wait, poll);
>  	if (!cec_is_registered(adap))
>  		return EPOLLERR | EPOLLHUP;
>  	mutex_lock(&adap->lock);
> @@ -48,7 +49,6 @@ static __poll_t cec_poll(struct file *filp,
>  		res |= EPOLLIN | EPOLLRDNORM;
>  	if (fh->total_queued_events)
>  		res |= EPOLLPRI;
> -	poll_wait(filp, &fh->wait, poll);
>  	mutex_unlock(&adap->lock);
>  	return res;
>  }
> -- 
> 2.20.1

Reviewed-by: Sean Young <sean@mess.org>


Sean

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH 8/8] dvb-core: fix epoll() by calling poll_wait first
  2019-02-07 11:49 ` [RFC PATCH 8/8] dvb-core: " hverkuil-cisco
@ 2019-02-08 21:12   ` Sean Young
  0 siblings, 0 replies; 11+ messages in thread
From: Sean Young @ 2019-02-08 21:12 UTC (permalink / raw)
  To: hverkuil-cisco; +Cc: linux-media, Michael Ira Krufky, Brad Love, Sakari Ailus

On Thu, Feb 07, 2019 at 12:49:48PM +0100, hverkuil-cisco@xs4all.nl wrote:
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> The epoll function expects that whenever the poll file op is
> called, the poll_wait function is also called. That didn't
> always happen in dvb_demux_poll(), dvb_dvr_poll() and
> dvb_ca_en50221_io_poll(). Fix this, otherwise epoll()
> can timeout when it shouldn't.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Reviewed-by: Sean Young <sean@mess.org>

> ---
>  drivers/media/dvb-core/dmxdev.c         | 8 ++++----
>  drivers/media/dvb-core/dvb_ca_en50221.c | 5 ++---
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
> index 1544e8cef564..f14a872d1268 100644
> --- a/drivers/media/dvb-core/dmxdev.c
> +++ b/drivers/media/dvb-core/dmxdev.c
> @@ -1195,13 +1195,13 @@ static __poll_t dvb_demux_poll(struct file *file, poll_table *wait)
>  	struct dmxdev_filter *dmxdevfilter = file->private_data;
>  	__poll_t mask = 0;
>  
> +	poll_wait(file, &dmxdevfilter->buffer.queue, wait);
> +
>  	if ((!dmxdevfilter) || dmxdevfilter->dev->exit)
>  		return EPOLLERR;
>  	if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx))
>  		return dvb_vb2_poll(&dmxdevfilter->vb2_ctx, file, wait);
>  
> -	poll_wait(file, &dmxdevfilter->buffer.queue, wait);
> -
>  	if (dmxdevfilter->state != DMXDEV_STATE_GO &&
>  	    dmxdevfilter->state != DMXDEV_STATE_DONE &&
>  	    dmxdevfilter->state != DMXDEV_STATE_TIMEDOUT)
> @@ -1346,13 +1346,13 @@ static __poll_t dvb_dvr_poll(struct file *file, poll_table *wait)
>  
>  	dprintk("%s\n", __func__);
>  
> +	poll_wait(file, &dmxdev->dvr_buffer.queue, wait);
> +
>  	if (dmxdev->exit)
>  		return EPOLLERR;
>  	if (dvb_vb2_is_streaming(&dmxdev->dvr_vb2_ctx))
>  		return dvb_vb2_poll(&dmxdev->dvr_vb2_ctx, file, wait);
>  
> -	poll_wait(file, &dmxdev->dvr_buffer.queue, wait);
> -
>  	if (((file->f_flags & O_ACCMODE) == O_RDONLY) ||
>  	    dmxdev->may_do_mmap) {
>  		if (dmxdev->dvr_buffer.error)
> diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
> index 4d371cea0d5d..ebf1e3b03819 100644
> --- a/drivers/media/dvb-core/dvb_ca_en50221.c
> +++ b/drivers/media/dvb-core/dvb_ca_en50221.c
> @@ -1797,6 +1797,8 @@ static __poll_t dvb_ca_en50221_io_poll(struct file *file, poll_table *wait)
>  
>  	dprintk("%s\n", __func__);
>  
> +	poll_wait(file, &ca->wait_queue, wait);
> +
>  	if (dvb_ca_en50221_io_read_condition(ca, &result, &slot) == 1)
>  		mask |= EPOLLIN;
>  
> @@ -1804,9 +1806,6 @@ static __poll_t dvb_ca_en50221_io_poll(struct file *file, poll_table *wait)
>  	if (mask)
>  		return mask;
>  
> -	/* wait for something to happen */
> -	poll_wait(file, &ca->wait_queue, wait);
> -
>  	if (dvb_ca_en50221_io_read_condition(ca, &result, &slot) == 1)
>  		mask |= EPOLLIN;
>  
> -- 
> 2.20.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-07 11:49 [RFC PATCH 0/8] cec/mc/vb2/dvb: fix epoll support hverkuil-cisco
2019-02-07 11:49 ` [RFC PATCH 1/8] cec: fix epoll() by calling poll_wait first hverkuil-cisco
2019-02-08 20:53   ` Sean Young
2019-02-07 11:49 ` [RFC PATCH 2/8] media-request: " hverkuil-cisco
2019-02-07 11:49 ` [RFC PATCH 3/8] vb2: " hverkuil-cisco
2019-02-07 11:49 ` [RFC PATCH 4/8] v4l2-ctrls.c: " hverkuil-cisco
2019-02-07 11:49 ` [RFC PATCH 5/8] v4l2-mem2mem: " hverkuil-cisco
2019-02-07 11:49 ` [RFC PATCH 6/8] v4l2-mem2mem: add q->error check to v4l2_m2m_poll() hverkuil-cisco
2019-02-07 11:49 ` [RFC PATCH 7/8] videobuf: fix epoll() by calling poll_wait first hverkuil-cisco
2019-02-07 11:49 ` [RFC PATCH 8/8] dvb-core: " hverkuil-cisco
2019-02-08 21:12   ` Sean Young

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org linux-media@archiver.kernel.org
	public-inbox-index linux-media


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/ public-inbox