On 01/18/18 12:21, Hans Verkuil wrote:
> When the video device is unregistered any filehandles waiting on
> an event (i.e. in VIDIOC_DQEVENT) will never wake up.
>
> Wake them up and detect that the video device is unregistered in
> __v4l2_event_dequeue() and return -ENODEV in that case.
>
> Signed-off-by: Hans Verkuil <[email protected]>
> ---
> Note: this is a quick hack, just for events. We have the same problem
> with vb2 (waiting for a buffer) and cec (waiting for an event). Not sure if rc
> or dvb are also suffering from the same problem.
>
> I wonder if there is an easier way to wake up all fhs that are waiting for
> some event.
>
> Michael: most applications use non-blocking mode and poll/select to wait
> for something to happen. In such applications the poll/select will return
> when the device is unregistered and this problem does not occur.
Forgot to mention: this has only been compile-tested.
Hans
> ---
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c
> b/drivers/media/v4l2-core/v4l2-dev.c
> index d5e0e536ef04..42574ccbd770 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -1017,6 +1017,9 @@ EXPORT_SYMBOL(__video_register_device);
> */
> void video_unregister_device(struct video_device *vdev)
> {
> + unsigned long flags;
> + struct v4l2_fh *fh;
> +
> /* Check if vdev was ever registered at all */
> if (!vdev || !video_is_registered(vdev))
> return;
> @@ -1027,6 +1030,12 @@ void video_unregister_device(struct video_device *vdev)
> */
> clear_bit(V4L2_FL_REGISTERED, &vdev->flags);
> mutex_unlock(&videodev_lock);
> +
> + 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);
> +
> 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 968c2eb08b5a..d04977cd1bfb 100644
> --- a/drivers/media/v4l2-core/v4l2-event.c
> +++ b/drivers/media/v4l2-core/v4l2-event.c
> @@ -36,12 +36,17 @@ static int __v4l2_event_dequeue(struct v4l2_fh *fh,
> struct v4l2_event *event)
> {
> struct v4l2_kevent *kev;
> unsigned long flags;
> + int ret = 0;
>
> spin_lock_irqsave(&fh->vdev->fh_lock, flags);
>
> + if (!video_is_registered(fh->vdev)) {
> + ret = -ENODEV;
> + goto err;
> + }
> if (list_empty(&fh->available)) {
> - spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
> - return -ENOENT;
> + ret = -ENOENT;
> + goto err;
> }
>
> WARN_ON(fh->navailable == 0);
> @@ -54,10 +59,10 @@ static int __v4l2_event_dequeue(struct v4l2_fh *fh,
> struct v4l2_event *event)
> *event = kev->event;
> kev->sev->first = sev_pos(kev->sev, 1);
> kev->sev->in_use--;
> -
> +err:
> spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
>
> - return 0;
> + return ret;
> }
>
> int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event,
>