Re: [RFC v2 4/4] v4l: events: Don't sleep in dequeue if none are subscribed

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Hans,

Thanks for the comments!

Hans Verkuil wrote:
> On 10/02/13 15:45, Sakari Ailus wrote:
>> Dequeueing events was is entirely possible even if none are subscribed,
>> leading to sleeping indefinitely. Fix this by returning -ENOENT when no
>> events are subscribed.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>>   drivers/media/v4l2-core/v4l2-event.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-event.c
>> b/drivers/media/v4l2-core/v4l2-event.c
>> index b53897e..553a800 100644
>> --- a/drivers/media/v4l2-core/v4l2-event.c
>> +++ b/drivers/media/v4l2-core/v4l2-event.c
>> @@ -77,10 +77,17 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct
>> v4l2_event *event,
>>           mutex_unlock(fh->vdev->lock);
>>
>>       do {
>> -        ret = wait_event_interruptible(fh->wait,
>> -                           fh->navailable != 0);
>> +        bool subscribed;
>
> Can you add an empty line here?

Sure.

>> +        ret = wait_event_interruptible(
>> +            fh->wait,
>> +            fh->navailable != 0 ||
>> +            !(subscribed = v4l2_event_has_subscribed(fh)));
>>           if (ret < 0)
>>               break;
>> +        if (!subscribed) {
>> +            ret = -EIO;
>
> Shouldn't this be -ENOENT?

If I use -ENOENT, having no events subscribed is indistinguishable form 
no events pending condition. Combine that with using select(2), and you 
can no longer distinguish having no events subscribed from the case 
where you got an event but someone else (another thread or process) 
dequeued it.

-EIO makes that explicit --- this also mirrors the behaviour of 
VIDIOC_DQBUF. (And it must be documented as well, which is missing from 
the patch currently.)

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com