Re: Can you review or ack this patch?

From: Hans Verkuil <hidden>
Date: 2011-09-08 07:49:14
Also in: linux-fsdevel, lkml

Hi Andrew,

Sorry for top-posting, I only have a crappy Android mailer at the moment.

Thank you for looking at this. Unfortunately I am dealing with a family emergency at the moment (and for the next 1-2 weeks), so I don't have the time or motivation to follow up on this. So if you or Mauro (or someone else for that matter) can fix the two issues you found, then that would be helpful.

Alternatively, I can also make a follow-on patch later once things have settled down for me.

I guess that multiple-assignment thing is something that anyone can correct of course.

Regards,

     Hans

Andrew Morton [off-list ref] wrote:
On Tue, 23 Aug 2011 08:33:25 +0200
Hans Verkuil [off-list ref] wrote:
quoted
(and resent again, this time with the correct linux-fsdevel mail address)
(Resent as requested by Andrew Morton since this is still stuck)

Hi Al, Andrew,

Can you take a look at this patch and send an Ack or review comments?

It's already been reviewed by Jon Corbet and we really need this functionality
for v3.1. You were in the CC list in earlier postings:

Here: http://www.spinics.net/lists/linux-fsdevel/msg46753.html
and here: http://www.mail-archive.com/linux-media@vger.kernel.org/msg34546.html

The patch also featured on LWN: http://lwn.net/Articles/450658/

Without your ack Mauro can't upstream this and we have a number of other
patches that depend on this and are currently blocked.

We would prefer to upstream this patch through the linux-media git tree
due to these dependencies.

My git branch containing this and the dependent patches is here:

http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/poll

Your help would be greatly appreciated (and your ack even more :-) )!
I'll grab the patch to get it a bit of testing while Al cogitates.
quoted

[PATCH] poll: add poll_requested_events() function

In some cases the poll() implementation in a driver has to do different
things depending on the events the caller wants to poll for. An example is
when a driver needs to start a DMA engine if the caller polls for POLLIN,
but doesn't want to do that if POLLIN is not requested but instead only
POLLOUT or POLLPRI is requested. This is something that can happen in the
video4linux subsystem.

Unfortunately, the current epoll/poll/select implementation doesn't provide
that information reliably. The poll_table_struct does have it: it has a key
field with the event mask. But once a poll() call matches one or more bits
of that mask any following poll() calls are passed a NULL poll_table_struct
pointer.

The solution is to set the qproc field to NULL in poll_table_struct once
poll() matches the events, not the poll_table_struct pointer itself. That
way drivers can obtain the mask through a new poll_requested_events inline.

The poll_table_struct can still be NULL since some kernel code calls it
internally (netfs_state_poll() in ./drivers/staging/pohmelfs/netfs.h). In
that case poll_requested_events() returns ~0 (i.e. all events).

Since eventpoll always leaves the key field at ~0 instead of using the
requested events mask, that source was changed as well to properly fill in
the key field.
It would be nice to find some suitable place in the code where we can
explain all this to other potential users of the capability.  Perhaps
the implementation site for the currently undocumented
poll_requested_events() would be a suitable place.
quoted
...

-	epi->event.events = event->events;
+	epi->event.events = pt.key = event->events;
coding style nit: we generally try to avoid multiple assignemnts like this.
quoted
...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help