On Tuesday, June 28, 2011 14:43:22 Mauro Carvalho Chehab wrote: > Em 28-06-2011 09:21, Andy Walls escreveu: > > Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> wrote: > > >> I'm not very comfortable with vb2 returning unexpected errors there. > >> Also, > >> for me it is clear that, if read will fail, POLLERR should be rised. > >> > >> Mauro. > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-media" > >> in > >> the body of a message to majordomo@xxxxxxxxxxxxxxx > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > It is also the case that a driver's poll method should never sleep. > > True. Actually, it is allowed, but only since kernel 2.6.29 (before that it could apparently give rise to busy looping if you were unlucky). But the main use case is userspace file systems like fuse. Not so much in regular drivers. Since drivers can sleep when starting streaming (ivtv will do that, in any case), we were in violation of the poll kernel API for a long time :-) > > I will try to find the conversation I had with laurent on interpreting the POSIX spec on error returns from select() and poll(). I will also try to find links to previos discussion with Hans on this. > > > > One issue is how to start streaming with apps that: > > - Open /dev/video/ in a nonblocking mode, and > > - Use the read() method > > > > while doing it in a way that is POSIX compliant and doesn't break existing apps. > > Well, a first call for poll() may rise a thread that will prepare the buffers, and > return with 0 while there's no data available. There is actually no guarantee whatsoever that if poll says you can read(), that that read also has to succeed. Other threads can have read the data already, and errors may have occured. And in fact, just starting streaming gives no guarantee that there is anything to read. For example, starting the DMA engine when there is no valid input signal. Many drivers (certainly those dealing with digital interfaces as opposed to analog) will just sit and wait. A non-blocking read will just return 0 without reading anything. So the current poll implementation (and that includes the one in videobuf-core.c as well) actually does *not* give any guarantee about whether data will be available in read(). And from the same POSIX link you posted: "The poll() function shall support regular files, terminal and pseudo-terminal devices, FIFOs, pipes, sockets and [XSR] [Option Start] STREAMS-based files. [Option End] The behavior of poll() on elements of fds that refer to other types of file is unspecified." Note the last line: we do not fall under this posix document. > > The other constraint is to ensure when only poll()-ing for exception conditions, not having significant IO side effects. > > > > I'm pretty sure sleeping in a driver's poll() method, or having significant side effects, is not ine the spirit of the POSIX select() and poll(), even if the letter of POSIX says nothing about it. > > > > The method I suggested to Hans is completely POSIX compliant for apps using read() and select() and was checked against MythTV as having no bad side effects. (And by thought experiment doesn't break any sensible app using nonblocking IO with select() and read().) > > > > I did not do analysis for apps that use mmap(), which I guess is the current concern. There isn't a problem with mmap(). For the stream I/O API you have to call STREAMON explicitly in order to start streaming. poll() will not do that for you. I was thinking that one improvement that could be realized is that vb2_poll could do some basic checks, such as checking whether streaming was already in progress (EBUSY), but then I realized that it already does that: this code is only active if there is no streaming in progress anyway. Regards, Hans > > The concern is that it is pointing that there are available data, even when there is an error. > This looks like a POSIX violation for me. > > Cheers, > Mauro. > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
