Re: [RFCv7 PATCH 2/4] poll: add poll_requested_events() andpoll_does_not_wait() functions

From: Andrew Morton
Date: Thu Feb 02 2012 - 17:48:36 EST


On Thu, 2 Feb 2012 11:26:55 +0100
Hans Verkuil <hverkuil@xxxxxxxxx> wrote:

> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> 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).
>
> Very rarely drivers might want to know whether poll_wait will actually wait.
> If another earlier file descriptor in the set already matched the events the
> caller wanted to wait for, then the kernel will return from the select() call
> without waiting.
>
> A new helper function poll_does_not_wait() is added that drivers can use to
> detect this situation.
>
> Drivers should no longer access any of the poll_table internals, but use the
> poll_requested_events() and poll_does_not_wait() access functions instead.

A way to communicate and enforce this is to rename the relevant fields. Prepend
a "_" to them and add a stern comment.


> Since the behavior of the qproc field changes with this patch (since this
> function pointer can now be NULL when that wasn't possible in the past) I
> have renamed that field from qproc to pq_proc. Any out-of-tree driver that
> uses it will now fail to compile.
>
> Some notes regarding the correctness of this patch: the driver's poll()
> function is called with a 'struct poll_table_struct *wait' argument. This
> pointer may or may not be NULL, drivers can never rely on it being one or
> the other as that depends on whether or not an earlier file descriptor in
> the select()'s fdset matched the requested events.
>
> ...
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/