2010/5/25 Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>: > On Tue, May 25, 2010 at 04:13:35PM -0700, Arve Hjønnevåg wrote: >> 2010/5/25 Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>: >> > On Tue, May 25, 2010 at 03:37:48PM -0700, Arve Hjønnevåg wrote: >> >> 2010/5/25 Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>: >> >> > On Tue, May 25, 2010 at 03:23:23PM -0700, Arve Hjønnevåg wrote: >> >> >> On Tue, May 25, 2010 at 11:47 AM, Dmitry Torokhov >> >> >> <dmitry.torokhov@xxxxxxxxx> wrote: >> >> >> > On Tue, May 25, 2010 at 02:35:17PM -0400, Alan Stern wrote: >> >> >> >> On Tue, 25 May 2010, Dmitry Torokhov wrote: >> >> >> >> >> >> >> >> > > Here's the scenario: >> >> >> >> > > >> >> >> >> > > The system is awake, and the user presses a key. The keyboard driver >> >> >> >> > > processes the keystroke and puts it in an input queue. A user process >> >> >> >> > > reads it from the event queue, thereby emptying the queue. >> >> >> >> > > >> >> >> >> > > At that moment, the system decides to go into opportunistic suspend. >> >> >> >> > > Since the input queue is empty, there's nothing to stop it. As the >> >> >> >> > > first step, userspace is frozen -- before the process has a chance to >> >> >> >> > > do anything with the keystroke it just read. As a result, the system >> >> >> >> > > stays asleep until something else wakes it up, even though the >> >> >> >> > > keystroke was important and should have prevented it from sleeping. >> >> >> >> > > >> >> >> >> > > Suspend blockers protect against this scenario. Here's how: >> >> >> >> > > >> >> >> >> > > The user process doesn't read the input queue directly; instead it >> >> >> >> > > does a select or poll. When it sees there is data in the queue, it >> >> >> >> > > first acquires a suspend blocker and then reads the data. >> >> >> >> > > >> >> >> >> > > Now the system _can't_ go into opportunistic suspend, because a suspend >> >> >> >> > > blocker is active. The user process can do whatever it wants with the >> >> >> >> > > keystroke. When it is finished, it releases the suspend blocker and >> >> >> >> > > loops back to the select/poll call. >> >> >> >> > > >> >> >> >> > >> >> >> >> > What you describe can be done in userspace though, via a "suspend manager" >> >> >> >> > process. Tasks reading input events will post "busy" events to stop the >> >> >> >> > manager process from sending system into suspend. But this can be confined to >> >> >> >> > Android userspace, leaving the kernel as is (well, kernel needs to be modified >> >> >> >> > to not go into suspend with full queues, but that is using existing kernel >> >> >> >> > APIs). >> >> >> >> >> >> >> >> I think that could be made to work. And it might remove the need for >> >> >> >> the userspace suspend-blocker API, which would be an advantage. It >> >> >> >> could even remove the need for the opportunistic-suspend workqueue -- >> >> >> >> opportunistic suspends would be initiated by the "suspend manager" >> >> >> >> process instead of by the kernel. >> >> >> >> >> >> >> >> However you still have the issue of modifying the kernel drivers to >> >> >> >> disallow opportunistic suspend if their queues are non-empty. Doing >> >> >> >> that is more or less equivalent to implementing kernel-level suspend >> >> >> >> blockers. (The suspend blocker approach is slightly more efficient, >> >> >> >> because it will prevent a suspend from starting if a queue is >> >> >> >> non-empty, instead of allowing the suspend to start and then aborting >> >> >> >> it partway through.) >> >> >> >> >> >> >> >> Maybe I'm missing something here... No doubt someone will point it out >> >> >> >> if I am. >> >> >> >> >> >> >> > >> >> >> > Well, from my perspective that would limit changes to the evdev driver >> >> >> > (well, limited input core plumbing will be needed) but that is using the >> >> >> > current PM infrastructure. The HW driver changes will be limited to what >> >> >> > you described "type 2" in your other e-mail. >> >> >> > >> >> >> > Also, not suspending while events are in progress) is probably >> >> >> > beneficial for platforms other than Android as well. So unless I am >> >> >> > missing something this sounds like a win. >> >> >> > >> >> >> >> >> >> How would this limit the changes you need in the evdev driver? It need >> >> >> to block suspend when there are unprocessed events in some queues. >> >> >> Suspend blockers gives you an api to do this, without it, you check >> >> >> the queues in your suspend hook and abort suspend if they are not >> >> >> empty. Without suspend blockers you have no api to signal that it is >> >> >> OK to suspend again, so you are forcing the thread that tried to >> >> >> suspend to poll until you stop aborting suspend. >> >> > >> >> > No, you do not need to poll. You just set a timeout (short or long, >> >> > depending on your needs) and if no userspace task blocked suspend >> >> > durng that time you attempt to initiate suspend from your manager >> >> > process. If it succeeds - good, if not that means that more events came >> >> > your way and you have to do it later. >> >> > >> >> >> >> How is that not polling? If the user is holding down a key, the keypad >> >> driver has to block suspend, and user space will try to suspend again >> >> and again and again... >> >> >> > >> > If your userpsace is that stupid - sure. However, you can: >> > >> > 1. Notify the suspend manager process that he rest of your userspace is >> > busy handling keystrokes so that it does not try to suspend while there >> > are events pending. >> >> You are missing the point. There are no event pending. The kernel >> reported the key down event, it was handled, but the keypad driver is >> still scanning to see if the user presses another key, > > Employ reasonable timeout. Timeout for what? Stop trying to suspend altogether, stop scanning for key changes, or a more "reasonable" poll interval? > >> or releases the >> currently held key. >> > > Userspace consumer should wait for the key release and retract "busy" > once event is received and handled. > No it should not. User-space does not know if the key is coming from a keypad driver that needs to actively scan the matrix while keys are pressed. >> > >> > 2. Wait a tiny bit after last application notified you that it finished >> > processing events. >> > >> > So basically the difference is that with in-kernel suspend blockers, >> > there is a tiny window where we haven't started the suspend yet but are >> > about to the driver has a chance to prevent entire system from starting >> > sleep. >> >> No, the difference is that if a driver needs to prevent suspend for an >> extended period of time, you don't have user space continuously >> polling to see if it can suspend. > > Why would a driver, on its own, prevent suspend for extended periods of > time? I think that the decision should originate from userspace, kernel > is here just to serve the requests. > A driver prevents suspend if suspend would prevent it from working. For instance, the gpio keypad matrix code prevents suspend when a key is help down, since it has to activly scan the keypad for changes. Only no-keys-pressed versus one-or-more-keys-pressed can be detected with an interrupt. >> >> > >> > Without the blocker we may start suspending and will stop midcycle. We >> > may be even better off in the end since we could leave some devices >> > still powered down after aborting system-wide suspend. >> > >> >> That does not sound right. > > Why doesn't it? If a device implements runtime PM it may chose remain in > powered-down mode even if system is awake. > If the device implements runtime PM it should already be powered-down if it is not in use. -- Arve Hjønnevåg _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
