Re: [RFC][PATCH 5/8] epoll: Add a flag, EPOLLWAKEUP, to prevent suspend while epoll events are ready

[Rafael J. Wysocki]

On Friday, April 27, 2012, Arve Hjønnevåg wrote:
> 2012/4/26 Rafael J. Wysocki <rjw@xxxxxxx>:
> > On Thursday, April 26, 2012, NeilBrown wrote:
> >> On Sun, 22 Apr 2012 23:22:43 +0200 "Rafael J. Wysocki" <rjw@xxxxxxx> wrote:
> >>
> >> > From: Arve Hjønnevåg <arve@xxxxxxxxxxx>
> >> >
> >> > When an epoll_event, that has the EPOLLWAKEUP flag set, is ready, a
> >> > wakeup_source will be active to prevent suspend. This can be used to
> >> > handle wakeup events from a driver that support poll, e.g. input, if
> >> > that driver wakes up the waitqueue passed to epoll before allowing
> >> > suspend.
> >> >
> >> > The current implementation uses an extra wakeup_source when
> >> > ep_scan_ready_list runs. This can cause problems if a single thread
> >> > is polling on wakeup events and frequent non-wakeup events (events
> >> > usually arrive during thread freezing) using the same epoll file.
> >>
> >> This is quite neat.
> >>
> >> If I understand it correctly, you register file descriptors with epoll_ctl()
> >> on an fd created with epoll_create(), and set the new EPOLLWAKEUP flag.
> >> Then when a regular 'poll' or 'select' on the epoll fd reports that it is
> >> readable you:
> 
> I think it makes more sense to use epoll_wait than mixing this with
> select or poll.
> 
> >>   - get a wakelock
> This may not be needed, since epoll does not reevaluate its state
> until you call into it again (at least using epoll_wait).
> 
> >>   - use epoll_wait to collect the events
> >>   - process the events
> >>   - release your wakelock
> >>   - go back to poll() or select() on the epoll fd.
> >> Correct?  As long as there are ready events with EPOLLWAKEUP set a
> >> wakeup_source is held active and the system won't go to sleep.
> >>
> >> My concern with this is about permissions.  It appears that any process could
> >> wait of some fd (maybe a pipe they created themselves) with EPOLLWAKEUP, and
> >> then simply never epoll_wait() for the event.  Then they would be keeping
> >> the system awake.  I don't think that is acceptable.
> >
> > I wonder what Arve has to say to that, but let me say that on systems without
> > autosleep every process can go into an infinite busy loop which is going to
> > drain battery relatively quickly just as well and I don't see why that's so
> > much different.
> >
> 
> I still think is useful to limit access to this feature. On a phone, a
> process stuck in an infinite loop will increase battery drain, but if
> this process does not have permission to prevent suspend, then this is
> only catastrophic if another process that have that permission is
> preventing suspend. I think we should add a capability for this.
> Assuming you agree,

I do.

> do want me to create a separate patch for that
> adds a capability, or roll it into this one.

Please roll it into this one, if that's not a problem.

Thanks,
Rafael
--
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/