All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv3 0/7] media: poll fixes
@ 2020-12-01 12:44 Hans Verkuil
  2020-12-01 12:44 ` [PATCHv3 1/7] vivid: fix 'disconnect' error injection Hans Verkuil
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Hans Verkuil @ 2020-12-01 12:44 UTC (permalink / raw)
  To: linux-media; +Cc: Alexandre Courbot, Tomasz Figa, Marek Szyprowski

This series fixes various poll bugs.

The first patch fixes a vivid bug w.r.t. the disconnect error injection.
It is identical to:

https://patchwork.linuxtv.org/project/linux-media/patch/74c0afae-3925-29c2-fa73-6c773c8d10c6@xs4all.nl/

This fix allows us to test disconnect using vivid in v4l2-compliance
(a patch for that will be posted separately).

The next two patches are from Alexandre and are unchanged from his
v2 series:

https://patchwork.linuxtv.org/project/linux-media/cover/20201123151843.798205-1-gnurou@gmail.com/

See that link for more info about these two patches.

The fourth patch introduces v4l2_event_wake_all(), which needs to
be called when unregistering a video device since otherwise any
processes that wait for an event won't wake up.

The fifth patch does the same for the vivid disconnect error
injection functionality. Again, this allows us to test that this
works using vivid and v4l2-compliance.

The last two patches add EPOLLPRI to the event mask when poll()
sees that the video or cec device is unregistered. This is needed
because if select() is called and it only checks for exceptions,
then it will only return if EPOLLPRI is set.

I'll post a patch for v4l2-compliance separately that will test
this special case using vivid.

Regards,

	Hans

Alexandre Courbot (2):
  media: videobuf2: always call poll_wait() on queues
  media: v4l2-mem2mem: always call poll_wait() on queues

Hans Verkuil (5):
  vivid: fix 'disconnect' error injection
  v4l2-dev/event: add v4l2_event_wake_all()
  vivid: call v4l2_event_wake_all() on disconnect
  v4l2-dev: add EPOLLPRI in v4l2_poll() when dev is unregistered
  cec: add EPOLLPRI in poll() when dev is unregistered

 drivers/media/cec/core/cec-api.c              |  2 +-
 .../media/common/videobuf2/videobuf2-core.c   | 11 +++-
 drivers/media/test-drivers/vivid/vivid-core.c | 65 ++++++++++++-------
 drivers/media/test-drivers/vivid/vivid-core.h |  1 +
 .../media/test-drivers/vivid/vivid-ctrls.c    | 27 +++++---
 drivers/media/v4l2-core/v4l2-dev.c            | 15 +++--
 drivers/media/v4l2-core/v4l2-event.c          | 17 +++++
 drivers/media/v4l2-core/v4l2-mem2mem.c        | 15 ++++-
 include/media/v4l2-event.h                    | 13 +++-
 9 files changed, 123 insertions(+), 43 deletions(-)

-- 
2.29.2


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

* [PATCHv3 1/7] vivid: fix 'disconnect' error injection
  2020-12-01 12:44 [PATCHv3 0/7] media: poll fixes Hans Verkuil
@ 2020-12-01 12:44 ` Hans Verkuil
  2020-12-01 12:44 ` [PATCHv3 2/7] media: videobuf2: always call poll_wait() on queues Hans Verkuil
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2020-12-01 12:44 UTC (permalink / raw)
  To: linux-media
  Cc: Alexandre Courbot, Tomasz Figa, Marek Szyprowski, Hans Verkuil

The 'disconnect' error injection functionality suffered from bit rot.

New device nodes were added without updating vivid_user_gen_s_ctrl(), so
that function had to be updated for the new device nodes.

Also, vivid didn't check if specific device nodes were actually ever created,
so the vivid_is_last_user() would fail on that (it would return true
instead of false in that case).

Finally, selecting Disconnect, then unbind the vivid driver would fail since
the remove() would think that the device nodes were already unregistered.
Keep track of whether disconnect was pressed and re-register the device nodes
in remove() before doing the real unregister.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/test-drivers/vivid/vivid-core.c | 65 ++++++++++++-------
 drivers/media/test-drivers/vivid/vivid-core.h |  1 +
 .../media/test-drivers/vivid/vivid-ctrls.c    | 29 ++++++---
 3 files changed, 65 insertions(+), 30 deletions(-)

diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c
index aa8d350fd682..5ddd31fdf102 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.c
+++ b/drivers/media/test-drivers/vivid/vivid-core.c
@@ -547,11 +547,13 @@ static int vivid_s_fmt_cap_mplane(struct file *file, void *priv,
 	return vidioc_s_fmt_vid_cap_mplane(file, priv, f);
 }
 
-static bool vivid_is_in_use(struct video_device *vdev)
+static bool vivid_is_in_use(bool valid, struct video_device *vdev)
 {
 	unsigned long flags;
 	bool res;
 
+	if (!valid)
+		return false;
 	spin_lock_irqsave(&vdev->fh_lock, flags);
 	res = !list_empty(&vdev->fh_list);
 	spin_unlock_irqrestore(&vdev->fh_lock, flags);
@@ -560,20 +562,45 @@ static bool vivid_is_in_use(struct video_device *vdev)
 
 static bool vivid_is_last_user(struct vivid_dev *dev)
 {
-	unsigned uses = vivid_is_in_use(&dev->vid_cap_dev) +
-			vivid_is_in_use(&dev->vid_out_dev) +
-			vivid_is_in_use(&dev->vbi_cap_dev) +
-			vivid_is_in_use(&dev->vbi_out_dev) +
-			vivid_is_in_use(&dev->sdr_cap_dev) +
-			vivid_is_in_use(&dev->radio_rx_dev) +
-			vivid_is_in_use(&dev->radio_tx_dev) +
-			vivid_is_in_use(&dev->meta_cap_dev) +
-			vivid_is_in_use(&dev->meta_out_dev) +
-			vivid_is_in_use(&dev->touch_cap_dev);
+	unsigned uses = vivid_is_in_use(dev->has_vid_cap, &dev->vid_cap_dev) +
+			vivid_is_in_use(dev->has_vid_out, &dev->vid_out_dev) +
+			vivid_is_in_use(dev->has_vbi_cap, &dev->vbi_cap_dev) +
+			vivid_is_in_use(dev->has_vbi_out, &dev->vbi_out_dev) +
+			vivid_is_in_use(dev->has_radio_rx, &dev->radio_rx_dev) +
+			vivid_is_in_use(dev->has_radio_tx, &dev->radio_tx_dev) +
+			vivid_is_in_use(dev->has_sdr_cap, &dev->sdr_cap_dev) +
+			vivid_is_in_use(dev->has_meta_cap, &dev->meta_cap_dev) +
+			vivid_is_in_use(dev->has_meta_out, &dev->meta_out_dev) +
+			vivid_is_in_use(dev->has_touch_cap, &dev->touch_cap_dev);
 
 	return uses == 1;
 }
 
+static void vivid_reconnect(struct vivid_dev *dev)
+{
+	if (dev->has_vid_cap)
+		set_bit(V4L2_FL_REGISTERED, &dev->vid_cap_dev.flags);
+	if (dev->has_vid_out)
+		set_bit(V4L2_FL_REGISTERED, &dev->vid_out_dev.flags);
+	if (dev->has_vbi_cap)
+		set_bit(V4L2_FL_REGISTERED, &dev->vbi_cap_dev.flags);
+	if (dev->has_vbi_out)
+		set_bit(V4L2_FL_REGISTERED, &dev->vbi_out_dev.flags);
+	if (dev->has_radio_rx)
+		set_bit(V4L2_FL_REGISTERED, &dev->radio_rx_dev.flags);
+	if (dev->has_radio_tx)
+		set_bit(V4L2_FL_REGISTERED, &dev->radio_tx_dev.flags);
+	if (dev->has_sdr_cap)
+		set_bit(V4L2_FL_REGISTERED, &dev->sdr_cap_dev.flags);
+	if (dev->has_meta_cap)
+		set_bit(V4L2_FL_REGISTERED, &dev->meta_cap_dev.flags);
+	if (dev->has_meta_out)
+		set_bit(V4L2_FL_REGISTERED, &dev->meta_out_dev.flags);
+	if (dev->has_touch_cap)
+		set_bit(V4L2_FL_REGISTERED, &dev->touch_cap_dev.flags);
+	dev->disconnect_error = false;
+}
+
 static int vivid_fop_release(struct file *file)
 {
 	struct vivid_dev *dev = video_drvdata(file);
@@ -581,23 +608,15 @@ static int vivid_fop_release(struct file *file)
 
 	mutex_lock(&dev->mutex);
 	if (!no_error_inj && v4l2_fh_is_singular_file(file) &&
-	    !video_is_registered(vdev) && vivid_is_last_user(dev)) {
+	    dev->disconnect_error && !video_is_registered(vdev) &&
+	    vivid_is_last_user(dev)) {
 		/*
 		 * I am the last user of this driver, and a disconnect
 		 * was forced (since this video_device is unregistered),
 		 * so re-register all video_device's again.
 		 */
 		v4l2_info(&dev->v4l2_dev, "reconnect\n");
-		set_bit(V4L2_FL_REGISTERED, &dev->vid_cap_dev.flags);
-		set_bit(V4L2_FL_REGISTERED, &dev->vid_out_dev.flags);
-		set_bit(V4L2_FL_REGISTERED, &dev->vbi_cap_dev.flags);
-		set_bit(V4L2_FL_REGISTERED, &dev->vbi_out_dev.flags);
-		set_bit(V4L2_FL_REGISTERED, &dev->sdr_cap_dev.flags);
-		set_bit(V4L2_FL_REGISTERED, &dev->radio_rx_dev.flags);
-		set_bit(V4L2_FL_REGISTERED, &dev->radio_tx_dev.flags);
-		set_bit(V4L2_FL_REGISTERED, &dev->meta_cap_dev.flags);
-		set_bit(V4L2_FL_REGISTERED, &dev->meta_out_dev.flags);
-		set_bit(V4L2_FL_REGISTERED, &dev->touch_cap_dev.flags);
+		vivid_reconnect(dev);
 	}
 	mutex_unlock(&dev->mutex);
 	if (file->private_data == dev->overlay_cap_owner)
@@ -1968,6 +1987,8 @@ static int vivid_remove(struct platform_device *pdev)
 		if (!dev)
 			continue;
 
+		if (dev->disconnect_error)
+			vivid_reconnect(dev);
 #ifdef CONFIG_MEDIA_CONTROLLER
 		media_device_unregister(&dev->mdev);
 #endif
diff --git a/drivers/media/test-drivers/vivid/vivid-core.h b/drivers/media/test-drivers/vivid/vivid-core.h
index 99e69b8f770f..9c2d1470b597 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.h
+++ b/drivers/media/test-drivers/vivid/vivid-core.h
@@ -303,6 +303,7 @@ struct vivid_dev {
 	struct fb_fix_screeninfo	fb_fix;
 
 	/* Error injection */
+	bool				disconnect_error;
 	bool				queue_setup_error;
 	bool				buf_prepare_error;
 	bool				start_streaming_error;
diff --git a/drivers/media/test-drivers/vivid/vivid-ctrls.c b/drivers/media/test-drivers/vivid/vivid-ctrls.c
index 334130568dcb..11e3b5617f52 100644
--- a/drivers/media/test-drivers/vivid/vivid-ctrls.c
+++ b/drivers/media/test-drivers/vivid/vivid-ctrls.c
@@ -107,14 +107,27 @@ static int vivid_user_gen_s_ctrl(struct v4l2_ctrl *ctrl)
 	switch (ctrl->id) {
 	case VIVID_CID_DISCONNECT:
 		v4l2_info(&dev->v4l2_dev, "disconnect\n");
-		clear_bit(V4L2_FL_REGISTERED, &dev->vid_cap_dev.flags);
-		clear_bit(V4L2_FL_REGISTERED, &dev->vid_out_dev.flags);
-		clear_bit(V4L2_FL_REGISTERED, &dev->vbi_cap_dev.flags);
-		clear_bit(V4L2_FL_REGISTERED, &dev->vbi_out_dev.flags);
-		clear_bit(V4L2_FL_REGISTERED, &dev->sdr_cap_dev.flags);
-		clear_bit(V4L2_FL_REGISTERED, &dev->radio_rx_dev.flags);
-		clear_bit(V4L2_FL_REGISTERED, &dev->radio_tx_dev.flags);
-		clear_bit(V4L2_FL_REGISTERED, &dev->meta_cap_dev.flags);
+		dev->disconnect_error = true;
+		if (dev->has_vid_cap)
+			clear_bit(V4L2_FL_REGISTERED, &dev->vid_cap_dev.flags);
+		if (dev->has_vid_out)
+			clear_bit(V4L2_FL_REGISTERED, &dev->vid_out_dev.flags);
+		if (dev->has_vbi_cap)
+			clear_bit(V4L2_FL_REGISTERED, &dev->vbi_cap_dev.flags);
+		if (dev->has_vbi_out)
+			clear_bit(V4L2_FL_REGISTERED, &dev->vbi_out_dev.flags);
+		if (dev->has_radio_rx)
+			clear_bit(V4L2_FL_REGISTERED, &dev->radio_rx_dev.flags);
+		if (dev->has_radio_tx)
+			clear_bit(V4L2_FL_REGISTERED, &dev->radio_tx_dev.flags);
+		if (dev->has_sdr_cap)
+			clear_bit(V4L2_FL_REGISTERED, &dev->sdr_cap_dev.flags);
+		if (dev->has_meta_cap)
+			clear_bit(V4L2_FL_REGISTERED, &dev->meta_cap_dev.flags);
+		if (dev->has_meta_out)
+			clear_bit(V4L2_FL_REGISTERED, &dev->meta_out_dev.flags);
+		if (dev->has_touch_cap)
+			clear_bit(V4L2_FL_REGISTERED, &dev->touch_cap_dev.flags);
 		break;
 	case VIVID_CID_BUTTON:
 		dev->button_pressed = 30;
-- 
2.29.2


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

* [PATCHv3 2/7] media: videobuf2: always call poll_wait() on queues
  2020-12-01 12:44 [PATCHv3 0/7] media: poll fixes Hans Verkuil
  2020-12-01 12:44 ` [PATCHv3 1/7] vivid: fix 'disconnect' error injection Hans Verkuil
@ 2020-12-01 12:44 ` Hans Verkuil
  2020-12-01 12:44 ` [PATCHv3 3/7] media: v4l2-mem2mem: " Hans Verkuil
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2020-12-01 12:44 UTC (permalink / raw)
  To: linux-media; +Cc: Alexandre Courbot, Tomasz Figa, Marek Szyprowski

From: Alexandre Courbot <gnurou@gmail.com>

do_poll()/do_select() seem to set the _qproc member of poll_table to
NULL the first time they are called on a given table, making subsequent
calls of poll_wait() on that table no-ops. This is a problem for vb2
which calls poll_wait() on the V4L2 queues' waitqueues only when a
queue-related event is requested, which may not necessarily be the case
during the first poll.

Fix this by making the call to poll_wait() happen first thing and
unconditionally in vb2_core_poll().

Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 4eab6d81cce1..ef06f90f5c6b 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -2363,13 +2363,20 @@ __poll_t vb2_core_poll(struct vb2_queue *q, struct file *file,
 	struct vb2_buffer *vb = NULL;
 	unsigned long flags;
 
+	/*
+	 * poll_wait() MUST be called on the first invocation on all the
+	 * potential queues of interest, even if we are not interested in their
+	 * events during this first call. Failure to do so will result in
+	 * queue's events to be ignored because the poll_table won't be capable
+	 * of adding new wait queues thereafter.
+	 */
+	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)))
 		return 0;
 
-	poll_wait(file, &q->done_wq, wait);
-
 	/*
 	 * Start file I/O emulator only if streaming API has not been used yet.
 	 */
-- 
2.29.2


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

* [PATCHv3 3/7] media: v4l2-mem2mem: always call poll_wait() on queues
  2020-12-01 12:44 [PATCHv3 0/7] media: poll fixes Hans Verkuil
  2020-12-01 12:44 ` [PATCHv3 1/7] vivid: fix 'disconnect' error injection Hans Verkuil
  2020-12-01 12:44 ` [PATCHv3 2/7] media: videobuf2: always call poll_wait() on queues Hans Verkuil
@ 2020-12-01 12:44 ` Hans Verkuil
  2020-12-01 12:44 ` [PATCHv3 4/7] v4l2-dev/event: add v4l2_event_wake_all() Hans Verkuil
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2020-12-01 12:44 UTC (permalink / raw)
  To: linux-media; +Cc: Alexandre Courbot, Tomasz Figa, Marek Szyprowski

From: Alexandre Courbot <gnurou@gmail.com>

do_poll()/do_select() seem to set the _qproc member of poll_table to
NULL the first time they are called on a given table, making subsequent
calls of poll_wait() on that table no-ops. This is a problem for mem2mem
which calls poll_wait() on the V4L2 queues' waitqueues only when a
queue-related event is requested, which may not necessarily be the case
during the first poll.

For instance, a stateful decoder is typically only interested in
EPOLLPRI events when it starts, and will switch to listening to both
EPOLLPRI and EPOLLIN after receiving the initial resolution change event
and configuring the CAPTURE queue. However by the time that switch
happens and v4l2_m2m_poll_for_data() is called for the first time,
poll_wait() has become a no-op and the V4L2 queues waitqueues thus
cannot be registered.

Fix this by moving the registration to v4l2_m2m_poll() and do it whether
or not one of the queue-related events are requested.

Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index b221b4e438a1..e7f4bf5bc8dd 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -887,9 +887,6 @@ static __poll_t v4l2_m2m_poll_for_data(struct file *file,
 	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);
-
 	/*
 	 * 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
@@ -922,9 +919,21 @@ __poll_t v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
 		       struct poll_table_struct *wait)
 {
 	struct video_device *vfd = video_devdata(file);
+	struct vb2_queue *src_q = v4l2_m2m_get_src_vq(m2m_ctx);
+	struct vb2_queue *dst_q = v4l2_m2m_get_dst_vq(m2m_ctx);
 	__poll_t req_events = poll_requested_events(wait);
 	__poll_t rc = 0;
 
+	/*
+	 * poll_wait() MUST be called on the first invocation on all the
+	 * potential queues of interest, even if we are not interested in their
+	 * events during this first call. Failure to do so will result in
+	 * queue's events to be ignored because the poll_table won't be capable
+	 * of adding new wait queues thereafter.
+	 */
+	poll_wait(file, &src_q->done_wq, wait);
+	poll_wait(file, &dst_q->done_wq, wait);
+
 	if (req_events & (EPOLLOUT | EPOLLWRNORM | EPOLLIN | EPOLLRDNORM))
 		rc = v4l2_m2m_poll_for_data(file, m2m_ctx, wait);
 
-- 
2.29.2


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

* [PATCHv3 4/7] v4l2-dev/event: add v4l2_event_wake_all()
  2020-12-01 12:44 [PATCHv3 0/7] media: poll fixes Hans Verkuil
                   ` (2 preceding siblings ...)
  2020-12-01 12:44 ` [PATCHv3 3/7] media: v4l2-mem2mem: " Hans Verkuil
@ 2020-12-01 12:44 ` Hans Verkuil
  2020-12-01 12:44 ` [PATCHv3 5/7] vivid: call v4l2_event_wake_all() on disconnect Hans Verkuil
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2020-12-01 12:44 UTC (permalink / raw)
  To: linux-media
  Cc: Alexandre Courbot, Tomasz Figa, Marek Szyprowski, Hans Verkuil

When unregistering a V4L2 device node, make sure any filehandles
that are waiting for an event are woken up.

Add v4l2_event_wake_all() to v4l2-event.c and call it from
video_unregister_device().

Otherwise userspace might never know that a device node was removed.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-dev.c   |  3 +++
 drivers/media/v4l2-core/v4l2-event.c | 17 +++++++++++++++++
 include/media/v4l2-event.h           | 13 +++++++++++--
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index a593ea0598b5..0ddc3554f1a4 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -28,6 +28,7 @@
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-event.h>
 
 #define VIDEO_NUM_DEVICES	256
 #define VIDEO_NAME              "video4linux"
@@ -1086,6 +1087,8 @@ void video_unregister_device(struct video_device *vdev)
 	 */
 	clear_bit(V4L2_FL_REGISTERED, &vdev->flags);
 	mutex_unlock(&videodev_lock);
+	if (test_bit(V4L2_FL_USES_V4L2_FH, &vdev->flags))
+		v4l2_event_wake_all(vdev);
 	device_unregister(&vdev->dev);
 }
 EXPORT_SYMBOL(video_unregister_device);
diff --git a/drivers/media/v4l2-core/v4l2-event.c b/drivers/media/v4l2-core/v4l2-event.c
index 290c6b213179..207a9ad80ea2 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -187,6 +187,23 @@ int v4l2_event_pending(struct v4l2_fh *fh)
 }
 EXPORT_SYMBOL_GPL(v4l2_event_pending);
 
+void v4l2_event_wake_all(struct video_device *vdev)
+{
+	struct v4l2_fh *fh;
+	unsigned long flags;
+
+	if (vdev == NULL)
+		return;
+
+	spin_lock_irqsave(&vdev->fh_lock, flags);
+
+	list_for_each_entry(fh, &vdev->fh_list, list)
+		wake_up_all(&fh->wait);
+
+	spin_unlock_irqrestore(&vdev->fh_lock, flags);
+}
+EXPORT_SYMBOL_GPL(v4l2_event_wake_all);
+
 static void __v4l2_event_unsubscribe(struct v4l2_subscribed_event *sev)
 {
 	struct v4l2_fh *fh = sev->fh;
diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
index 3f0281d06ec7..4ffa914ade3a 100644
--- a/include/media/v4l2-event.h
+++ b/include/media/v4l2-event.h
@@ -101,7 +101,7 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event,
  *
  * .. note::
  *    The driver's only responsibility is to fill in the type and the data
- *    fields.The other fields will be filled in by  V4L2.
+ *    fields. The other fields will be filled in by V4L2.
  */
 void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev);
 
@@ -116,10 +116,19 @@ void v4l2_event_queue(struct video_device *vdev, const struct v4l2_event *ev);
  *
  * .. note::
  *    The driver's only responsibility is to fill in the type and the data
- *    fields.The other fields will be filled in by  V4L2.
+ *    fields. The other fields will be filled in by V4L2.
  */
 void v4l2_event_queue_fh(struct v4l2_fh *fh, const struct v4l2_event *ev);
 
+/**
+ * v4l2_event_wake_all - Wake all filehandles.
+ *
+ * Used when unregistering a video device.
+ *
+ * @vdev: pointer to &struct video_device
+ */
+void v4l2_event_wake_all(struct video_device *vdev);
+
 /**
  * v4l2_event_pending - Check if an event is available
  *
-- 
2.29.2


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

* [PATCHv3 5/7] vivid: call v4l2_event_wake_all() on disconnect
  2020-12-01 12:44 [PATCHv3 0/7] media: poll fixes Hans Verkuil
                   ` (3 preceding siblings ...)
  2020-12-01 12:44 ` [PATCHv3 4/7] v4l2-dev/event: add v4l2_event_wake_all() Hans Verkuil
@ 2020-12-01 12:44 ` Hans Verkuil
  2020-12-01 12:44 ` [PATCHv3 6/7] v4l2-dev: add EPOLLPRI in v4l2_poll() when dev is unregistered Hans Verkuil
  2020-12-01 12:44 ` [PATCHv3 7/7] cec: add EPOLLPRI in poll() " Hans Verkuil
  6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2020-12-01 12:44 UTC (permalink / raw)
  To: linux-media
  Cc: Alexandre Courbot, Tomasz Figa, Marek Szyprowski, Hans Verkuil

When the disconnect error injection control is set, then besides
faking unregistering the device nodes, also call v4l2_event_wake_all()
to ensure any userspace applications will wake up as per a 'normal'
unregister.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 .../media/test-drivers/vivid/vivid-ctrls.c    | 38 +++++++++----------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/media/test-drivers/vivid/vivid-ctrls.c b/drivers/media/test-drivers/vivid/vivid-ctrls.c
index 11e3b5617f52..7957eadf3e2b 100644
--- a/drivers/media/test-drivers/vivid/vivid-ctrls.c
+++ b/drivers/media/test-drivers/vivid/vivid-ctrls.c
@@ -100,6 +100,14 @@
 
 /* General User Controls */
 
+static void vivid_unregister_dev(bool valid, struct video_device *vdev)
+{
+	if (!valid)
+		return;
+	clear_bit(V4L2_FL_REGISTERED, &vdev->flags);
+	v4l2_event_wake_all(vdev);
+}
+
 static int vivid_user_gen_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct vivid_dev *dev = container_of(ctrl->handler, struct vivid_dev, ctrl_hdl_user_gen);
@@ -108,26 +116,16 @@ static int vivid_user_gen_s_ctrl(struct v4l2_ctrl *ctrl)
 	case VIVID_CID_DISCONNECT:
 		v4l2_info(&dev->v4l2_dev, "disconnect\n");
 		dev->disconnect_error = true;
-		if (dev->has_vid_cap)
-			clear_bit(V4L2_FL_REGISTERED, &dev->vid_cap_dev.flags);
-		if (dev->has_vid_out)
-			clear_bit(V4L2_FL_REGISTERED, &dev->vid_out_dev.flags);
-		if (dev->has_vbi_cap)
-			clear_bit(V4L2_FL_REGISTERED, &dev->vbi_cap_dev.flags);
-		if (dev->has_vbi_out)
-			clear_bit(V4L2_FL_REGISTERED, &dev->vbi_out_dev.flags);
-		if (dev->has_radio_rx)
-			clear_bit(V4L2_FL_REGISTERED, &dev->radio_rx_dev.flags);
-		if (dev->has_radio_tx)
-			clear_bit(V4L2_FL_REGISTERED, &dev->radio_tx_dev.flags);
-		if (dev->has_sdr_cap)
-			clear_bit(V4L2_FL_REGISTERED, &dev->sdr_cap_dev.flags);
-		if (dev->has_meta_cap)
-			clear_bit(V4L2_FL_REGISTERED, &dev->meta_cap_dev.flags);
-		if (dev->has_meta_out)
-			clear_bit(V4L2_FL_REGISTERED, &dev->meta_out_dev.flags);
-		if (dev->has_touch_cap)
-			clear_bit(V4L2_FL_REGISTERED, &dev->touch_cap_dev.flags);
+		vivid_unregister_dev(dev->has_vid_cap, &dev->vid_cap_dev);
+		vivid_unregister_dev(dev->has_vid_out, &dev->vid_out_dev);
+		vivid_unregister_dev(dev->has_vbi_cap, &dev->vbi_cap_dev);
+		vivid_unregister_dev(dev->has_vbi_out, &dev->vbi_out_dev);
+		vivid_unregister_dev(dev->has_radio_rx, &dev->radio_rx_dev);
+		vivid_unregister_dev(dev->has_radio_tx, &dev->radio_tx_dev);
+		vivid_unregister_dev(dev->has_sdr_cap, &dev->sdr_cap_dev);
+		vivid_unregister_dev(dev->has_meta_cap, &dev->meta_cap_dev);
+		vivid_unregister_dev(dev->has_meta_out, &dev->meta_out_dev);
+		vivid_unregister_dev(dev->has_touch_cap, &dev->touch_cap_dev);
 		break;
 	case VIVID_CID_BUTTON:
 		dev->button_pressed = 30;
-- 
2.29.2


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

* [PATCHv3 6/7] v4l2-dev: add EPOLLPRI in v4l2_poll() when dev is unregistered
  2020-12-01 12:44 [PATCHv3 0/7] media: poll fixes Hans Verkuil
                   ` (4 preceding siblings ...)
  2020-12-01 12:44 ` [PATCHv3 5/7] vivid: call v4l2_event_wake_all() on disconnect Hans Verkuil
@ 2020-12-01 12:44 ` Hans Verkuil
  2020-12-01 12:44 ` [PATCHv3 7/7] cec: add EPOLLPRI in poll() " Hans Verkuil
  6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2020-12-01 12:44 UTC (permalink / raw)
  To: linux-media
  Cc: Alexandre Courbot, Tomasz Figa, Marek Szyprowski, Hans Verkuil

If the V4L2 device was unregistered, then add EPOLLPRI to
the poll mask. Otherwise a select() that only waits for
exceptions will not wake up. A select() that waits for
read and/or write events *will* wake up on an EPOLLERR, but
not (for some reason) if it just waits for exceptions.

Strangly the epoll functionality will wakeup on EPOLLERR if
you just wait for an exception, so in this respect select()
and epoll differ.

In the end it doesn't really matter, what matters is that
polling file handles are woken up on device unregistration.

It also improves the code a bit if vdev->fops->poll is NULL:
this didn't check for device unregistration.

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

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 0ddc3554f1a4..f9cff033d0dc 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -339,12 +339,14 @@ static ssize_t v4l2_write(struct file *filp, const char __user *buf,
 static __poll_t v4l2_poll(struct file *filp, struct poll_table_struct *poll)
 {
 	struct video_device *vdev = video_devdata(filp);
-	__poll_t res = EPOLLERR | EPOLLHUP;
+	__poll_t res = EPOLLERR | EPOLLHUP | EPOLLPRI;
 
-	if (!vdev->fops->poll)
-		return DEFAULT_POLLMASK;
-	if (video_is_registered(vdev))
-		res = vdev->fops->poll(filp, poll);
+	if (video_is_registered(vdev)) {
+		if (!vdev->fops->poll)
+			res = DEFAULT_POLLMASK;
+		else
+			res = vdev->fops->poll(filp, poll);
+	}
 	if (vdev->dev_debug & V4L2_DEV_DEBUG_POLL)
 		dprintk("%s: poll: %08x\n",
 			video_device_node_name(vdev), res);
-- 
2.29.2


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

* [PATCHv3 7/7] cec: add EPOLLPRI in poll() when dev is unregistered
  2020-12-01 12:44 [PATCHv3 0/7] media: poll fixes Hans Verkuil
                   ` (5 preceding siblings ...)
  2020-12-01 12:44 ` [PATCHv3 6/7] v4l2-dev: add EPOLLPRI in v4l2_poll() when dev is unregistered Hans Verkuil
@ 2020-12-01 12:44 ` Hans Verkuil
  6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2020-12-01 12:44 UTC (permalink / raw)
  To: linux-media
  Cc: Alexandre Courbot, Tomasz Figa, Marek Szyprowski, Hans Verkuil

If the CEC device was unregistered, then add EPOLLPRI to
the poll() mask. Otherwise a select() that only waits for
exceptions will not wake up. A select() that waits for
read and/or write events *will* wake up on an EPOLLERR, but
not (for some reason) if it just waits for exceptions.

Strangly the epoll functionality will wakeup on EPOLLERR if
you just wait for an exception, so in this respect select()
and epoll differ.

In the end it doesn't really matter, what matters is that
polling file handles are woken up on device unregistration.

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

diff --git a/drivers/media/cec/core/cec-api.c b/drivers/media/cec/core/cec-api.c
index f922a2196b2b..769e6b4cddce 100644
--- a/drivers/media/cec/core/cec-api.c
+++ b/drivers/media/cec/core/cec-api.c
@@ -40,7 +40,7 @@ static __poll_t cec_poll(struct file *filp,
 
 	poll_wait(filp, &fh->wait, poll);
 	if (!cec_is_registered(adap))
-		return EPOLLERR | EPOLLHUP;
+		return EPOLLERR | EPOLLHUP | EPOLLPRI;
 	mutex_lock(&adap->lock);
 	if (adap->is_configured &&
 	    adap->transmit_queue_sz < CEC_MAX_MSG_TX_QUEUE_SZ)
-- 
2.29.2


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

end of thread, other threads:[~2020-12-01 12:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-01 12:44 [PATCHv3 0/7] media: poll fixes Hans Verkuil
2020-12-01 12:44 ` [PATCHv3 1/7] vivid: fix 'disconnect' error injection Hans Verkuil
2020-12-01 12:44 ` [PATCHv3 2/7] media: videobuf2: always call poll_wait() on queues Hans Verkuil
2020-12-01 12:44 ` [PATCHv3 3/7] media: v4l2-mem2mem: " Hans Verkuil
2020-12-01 12:44 ` [PATCHv3 4/7] v4l2-dev/event: add v4l2_event_wake_all() Hans Verkuil
2020-12-01 12:44 ` [PATCHv3 5/7] vivid: call v4l2_event_wake_all() on disconnect Hans Verkuil
2020-12-01 12:44 ` [PATCHv3 6/7] v4l2-dev: add EPOLLPRI in v4l2_poll() when dev is unregistered Hans Verkuil
2020-12-01 12:44 ` [PATCHv3 7/7] cec: add EPOLLPRI in poll() " Hans Verkuil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.