From: Dmitry Torokhov on
On Tuesday 25 May 2010 11:08:03 am Alan Stern wrote:
> On Tue, 25 May 2010, Dmitry Torokhov wrote:
> > > > I don't see a big difference between 2 and 3. You can use suspend
> > > > blockers to handle either.
> > >
> > > You can, but they aren't necessary. If 2 were the only reason for
> > > suspend blockers, I would say they shouldn't be merged.
> > >
> > > Whereas 3, on the other hand, can _not_ be handled by any existing
> > > mechanism. 3 is perhaps the most important reason for using suspend
> > > blockers.
> >
> > I do not see why 3 has to be implemented using suspend blockers either.
> > If you are concerned that event gets stuck somewhere in the stack make
> > sure that devices in the stack do not suspend while their queue is not
> > empty. This way if you try opportunistic suspend it will keep failing
> > until you drained all important queues.
>
> 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).

--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on
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.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Dmitry Torokhov on
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.

--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on
On Tue, 25 May 2010, Dmitry Torokhov wrote:

> > > 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.

I agree that simplifying the user API would be an advantage. Instead
of the full-blown suspend-blocker interface, we would need only a way
to initiate an opportunistic suspend. For example:

echo opportunistic >/sys/power/state

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Rafael J. Wysocki on
On Tuesday 25 May 2010, Dmitry Torokhov wrote:
> On Tuesday 25 May 2010 11:08:03 am Alan Stern wrote:
> > On Tue, 25 May 2010, Dmitry Torokhov wrote:
> > > > > I don't see a big difference between 2 and 3. You can use suspend
> > > > > blockers to handle either.
> > > >
> > > > You can, but they aren't necessary. If 2 were the only reason for
> > > > suspend blockers, I would say they shouldn't be merged.
> > > >
> > > > Whereas 3, on the other hand, can _not_ be handled by any existing
> > > > mechanism. 3 is perhaps the most important reason for using suspend
> > > > blockers.
> > >
> > > I do not see why 3 has to be implemented using suspend blockers either.
> > > If you are concerned that event gets stuck somewhere in the stack make
> > > sure that devices in the stack do not suspend while their queue is not
> > > empty. This way if you try opportunistic suspend it will keep failing
> > > until you drained all important queues.
> >
> > 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).

For that to work, you'd have to make the user space suspend manager prevent
key-reading processes from emptying the queue before it orders the kernel to
put the system to sleep. Otherwise it still is possible that the queue will be
emptied right at the moment it writes to /sys/power/state and the scenario
described by Alan is going to happen.

Moreover, I don't think it's limited to the input subsystem, because the wakeup
events may originate from the network or some other sources and all of them
would require similar handling.

The problem here is that user space can't do anything to stop the freezing of
tasks without suspend blockers (or something more-or-less equivalent).

Now, you can argue that in theory we can avoid that if tasks are not frozen,
but quite frankly that's not a very realistic way to go.

Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/