diff mbox

[RFC,v2,3/4] v4l: vb2: Return POLLERR when polling for events and none are subscribed

Message ID 1380721516-488-4-git-send-email-sakari.ailus@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sakari Ailus Oct. 2, 2013, 1:45 p.m. UTC
The current implementation allowed polling for events even if none were
subscribed. This may be troublesome in multi-threaded applications where the
thread handling the subscription is different from the one that handles the
events.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Hans Verkuil Oct. 2, 2013, 1:59 p.m. UTC | #1
On 10/02/13 15:45, Sakari Ailus wrote:
> The current implementation allowed polling for events even if none were
> subscribed. This may be troublesome in multi-threaded applications where the
> thread handling the subscription is different from the one that handles the
> events.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>   drivers/media/v4l2-core/videobuf2-core.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 79acf5e..c5dc903 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -2011,6 +2011,9 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
>
>   		if (v4l2_event_pending(fh))
>   			res = POLLPRI;
> +
> +		if (!v4l2_event_has_subscribed(fh))
> +			return POLLERR | POLLPRI;

What should happen if you poll for both POLLPRI and POLLIN and one of 
the two would normally return POLLERR? Should that error condition be 
ignored?

I'm not sure, frankly.

Regards,

	Hans

>   	}
>
>   	if (!V4L2_TYPE_IS_OUTPUT(q->type) && !(req_events & (POLLIN | POLLRDNORM)))
>

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sakari Ailus Oct. 2, 2013, 2:21 p.m. UTC | #2
Hi Hans,

Thanks for your comments.

Hans Verkuil wrote:
> On 10/02/13 15:45, Sakari Ailus wrote:
>> The current implementation allowed polling for events even if none were
>> subscribed. This may be troublesome in multi-threaded applications
>> where the
>> thread handling the subscription is different from the one that
>> handles the
>> events.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>>   drivers/media/v4l2-core/videobuf2-core.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 79acf5e..c5dc903 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -2011,6 +2011,9 @@ unsigned int vb2_poll(struct vb2_queue *q,
>> struct file *file, poll_table *wait)
>>
>>           if (v4l2_event_pending(fh))
>>               res = POLLPRI;
>> +
>> +        if (!v4l2_event_has_subscribed(fh))
>> +            return POLLERR | POLLPRI;
>
> What should happen if you poll for both POLLPRI and POLLIN and one of
> the two would normally return POLLERR? Should that error condition be
> ignored?
>
> I'm not sure, frankly.

I think you just need to go to see what does VIDIOC_DQBUF / 
VIDIOC_DQEVENT return. If you're using select(2) you won't know about 
POLLERR explicitly anyway: there's a bit for read, write and exceptions 
but not for errors.
diff mbox

Patch

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 79acf5e..c5dc903 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -2011,6 +2011,9 @@  unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
 
 		if (v4l2_event_pending(fh))
 			res = POLLPRI;
+
+		if (!v4l2_event_has_subscribed(fh))
+			return POLLERR | POLLPRI;
 	}
 
 	if (!V4L2_TYPE_IS_OUTPUT(q->type) && !(req_events & (POLLIN | POLLRDNORM)))