public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: linux-media@vger.kernel.org, laurent.pinchart@ideasonboard.com,
	teemux.tuominen@intel.com
Subject: Re: [RFC v2 4/4] v4l: events: Don't sleep in dequeue if none are subscribed
Date: Wed, 02 Oct 2013 17:49:36 +0300	[thread overview]
Message-ID: <524C3280.5030406@linux.intel.com> (raw)
In-Reply-To: <524C2F9A.80806@xs4all.nl>

Hans Verkuil wrote:
...
>>>> +        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.
>
> OK, but then your commit message is out of sync with the actual patch since
> the commit log says ENOENT.
>
>> -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.)
>
> I don't like using EIO for this. EIO generally is returned if a hardware
> error or an unexpected hardware condition occurs. How about -ENOMSG? Or
> perhaps EPIPE? (As in: "the pipe containing events is gone").

Thinking about this some more, -ENOENT is probably what we should 
return. *But* when there are no events to dequeue, we should instead 
return -EAGAIN (i.e. EWOULDBLOCK) which VIDIOC_DQBUF also uses.

However I'm not sure if anything depends on -ENOENT currently (probably 
not really) so changing this might require some consideration. No error 
codes are currently defined for VIDIOC_DQEVENT; was planning to fix that 
while we're at this.

-- 
Cheers,

Sakari Ailus
sakari.ailus@linux.intel.com

  parent reply	other threads:[~2013-10-02 14:49 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-02 13:45 [RFC v2 0/4] Sakari Ailus
2013-10-02 13:45 ` [RFC v2 1/4] v4l: return POLLERR on V4L2 sub-devices if no events are subscribed Sakari Ailus
2013-10-02 14:05   ` Hans Verkuil
2013-10-02 13:45 ` [RFC v2 2/4] v4l: vb2: Only poll for events if the user is interested in them Sakari Ailus
2013-10-02 14:07   ` Hans Verkuil
2013-10-02 13:45 ` [RFC v2 3/4] v4l: vb2: Return POLLERR when polling for events and none are subscribed Sakari Ailus
2013-10-02 13:59   ` Hans Verkuil
2013-10-02 14:21     ` Sakari Ailus
2013-10-02 13:45 ` [RFC v2 4/4] v4l: events: Don't sleep in dequeue if " Sakari Ailus
2013-10-02 14:04   ` Hans Verkuil
2013-10-02 14:18     ` Sakari Ailus
2013-10-02 14:37       ` Hans Verkuil
2013-10-02 14:45         ` Sakari Ailus
2013-10-03  9:29           ` Hans Verkuil
2013-10-02 14:49         ` Sakari Ailus [this message]
2013-10-03  9:49           ` Hans Verkuil
2013-10-02 18:12   ` Laurent Pinchart
2013-10-02 20:23     ` Sakari Ailus
2013-10-02 14:00 ` [RFC v2 0/4] Hans Verkuil
2013-10-02 18:15 ` Laurent Pinchart
2013-10-02 20:22   ` Sakari Ailus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=524C3280.5030406@linux.intel.com \
    --to=sakari.ailus@linux.intel.com \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=teemux.tuominen@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox