Suspend block api (version 8)

The first goal can be achieved either by using device runtime PM and cpuidle to put all hardware into low-power states, transparently from the user space point of view, or by suspending the whole system. However, system suspend, in its current form, does not guarantee that the events of interest will always be responded to, since wakeup events (events that wake the CPU from idle and the system from suspend) that occur right after initiating suspend will not be processed until another possibly unrelated event wakes the system up again.

The first goal can be achieved either by using device runtime PM and cpuidle to put all hardware into low-power states, transparently from the user space point of view, or by suspending the whole system. However, system suspend, in its current form, does not guarantee that the events of interest will always be responded to, since wakeup events (events that wake the CPU from idle and the system from suspend) that occur right after initiating suspend will not be processed until another possibly unrelated event wakes the system up again.

This patch series adds a suspend-block api that provides the same functionality as the android wakelock api. This version adds a delay before suspending again if no suspend blockers were used during the last suspend attempt.

This patch series adds a suspend-block api that provides the same functionality as the android wakelock api. This version adds a delay before suspending again if no suspend blockers were used during the last suspend attempt.

Quoted 4 lines · Arve Hjønnevåg

This patch series adds a suspend-block api that provides the same functionality as the android wakelock api. This version adds a delay before suspending again if no suspend blockers were used during the last suspend attempt.

Patches [1-6/8] applied to suspend-2.6/linux-next

Quoted 6 lines · ext Rafael J. Wysocki

Quoted 4 lines · Arve Hjønnevåg

This patch series adds a suspend-block api that provides the same functionality as the android wakelock api. This version adds a delay before suspending again if no suspend blockers were used during the last suspend attempt.

Quoted 1 line

Patches [1-6/8] applied to suspend-2.6/linux-next

funny thing is that even without sorting out the concerns plenty of developers had on the other thread, this series is still taken. What's the point in dicussing/reviewing the patches then ?

Quoted 6 lines · ext Rafael J. Wysocki

Quoted 4 lines · Arve Hjønnevåg

This patch series adds a suspend-block api that provides the same functionality as the android wakelock api. This version adds a delay before suspending again if no suspend blockers were used during the last suspend attempt.

Quoted 1 line

Patches [1-6/8] applied to suspend-2.6/linux-next

funny thing is that even without sorting out the concerns plenty of developers had on the other thread, this series is still taken. What's the point in dicussing/reviewing the patches then ?

Quoted 7 lines · Felipe Balbi

Quoted 6 lines · ext Rafael J. Wysocki

Quoted 4 lines · Arve Hjønnevåg

This patch series adds a suspend-block api that provides the same functionality as the android wakelock api. This version adds a delay before suspending again if no suspend blockers were used during the last suspend attempt.

Quoted 5 lines

Quoted 1 line

Patches [1-6/8] applied to suspend-2.6/linux-next

funny thing is that even without sorting out the concerns plenty of developers had on the other thread, this series is still taken. What's the point in dicussing/reviewing the patches then ?

I don't think the concerns you're referring to can be solved out. Some people just don't like the whole idea and I don't think there's any way we can improve the patches to make them happy. The only "solution" they would be satisfied with would simply be rejecting the feature altogether, although there are no practically viable alternatives known to me.

Quoted 7 lines · Felipe Balbi

Quoted 6 lines · ext Rafael J. Wysocki

Quoted 4 lines · Arve Hjønnevåg

This patch series adds a suspend-block api that provides the same functionality as the android wakelock api. This version adds a delay before suspending again if no suspend blockers were used during the last suspend attempt.

Quoted 5 lines

Quoted 1 line

Patches [1-6/8] applied to suspend-2.6/linux-next

funny thing is that even without sorting out the concerns plenty of developers had on the other thread, this series is still taken. What's the point in dicussing/reviewing the patches then ?

I don't think the concerns you're referring to can be solved out. Some people just don't like the whole idea and I don't think there's any way we can improve the patches to make them happy. The only "solution" they would be satisfied with would simply be rejecting the feature altogether, although there are no practically viable alternatives known to me.

Quoted 14 lines · Rafael J. Wysocki

Quoted 7 lines · Felipe Balbi

Quoted 6 lines · ext Rafael J. Wysocki

Quoted 4 lines · Arve Hjønnevåg

This patch series adds a suspend-block api that provides the same functionality as the android wakelock api. This version adds a delay before suspending again if no suspend blockers were used during the last suspend attempt.

Quoted 5 lines

Quoted 1 line

Patches [1-6/8] applied to suspend-2.6/linux-next

funny thing is that even without sorting out the concerns plenty of developers had on the other thread, this series is still taken. What's the point in dicussing/reviewing the patches then ?

Quoted 6 lines

I don't think the concerns you're referring to can be solved out. Some people just don't like the whole idea and I don't think there's any way we can improve the patches to make them happy. The only "solution" they would be satisfied with would simply be rejecting the feature altogether, although there are no practically viable alternatives known to me.

I'm not sure who the "some people" you're referring to are, but I'll assume I'm included in that group.

I don't think this is a fair characterization of the objections, nor do I think "rejecting the feature altogether" is the only satisfactory answer. Speaking for myself, I find the idea of being able to suspend while idle a valid objective, and certainly see the usefulness of it for embedded systems. I'm also an owner and user of an Android phone, so I am certainly not out just to make life difficult for Android.

The primary objection is not the end goal, but rather the implementation. In particular, the problematic redefintion of what it means to be idle, or "not doing work that's immediately useful to the user" to use the phrase from the changelog (where "useful" is still not defined.)

This (re)definition completely bypasses all current idle infrastructure based on timers, scheduler, etc. and makes "usefulness" defined in terms of who holds suspend blockers.

This of course will lead to a scattering of suspend blockers into any drivers/subsystems considered "useful", which by looking through current Android kernels is many of them.

Quoted 14 lines · Rafael J. Wysocki

Quoted 7 lines · Felipe Balbi

Quoted 6 lines · ext Rafael J. Wysocki

Quoted 4 lines · Arve Hjønnevåg

This patch series adds a suspend-block api that provides the same functionality as the android wakelock api. This version adds a delay before suspending again if no suspend blockers were used during the last suspend attempt.

Quoted 5 lines

Quoted 1 line

Patches [1-6/8] applied to suspend-2.6/linux-next

funny thing is that even without sorting out the concerns plenty of developers had on the other thread, this series is still taken. What's the point in dicussing/reviewing the patches then ?

Quoted 6 lines

I don't think the concerns you're referring to can be solved out. Some people just don't like the whole idea and I don't think there's any way we can improve the patches to make them happy. The only "solution" they would be satisfied with would simply be rejecting the feature altogether, although there are no practically viable alternatives known to me.

I'm not sure who the "some people" you're referring to are, but I'll assume I'm included in that group.

I don't think this is a fair characterization of the objections, nor do I think "rejecting the feature altogether" is the only satisfactory answer. Speaking for myself, I find the idea of being able to suspend while idle a valid objective, and certainly see the usefulness of it for embedded systems. I'm also an owner and user of an Android phone, so I am certainly not out just to make life difficult for Android.

The primary objection is not the end goal, but rather the implementation. In particular, the problematic redefintion of what it means to be idle, or "not doing work that's immediately useful to the user" to use the phrase from the changelog (where "useful" is still not defined.)

This (re)definition completely bypasses all current idle infrastructure based on timers, scheduler, etc. and makes "usefulness" defined in terms of who holds suspend blockers.

This of course will lead to a scattering of suspend blockers into any drivers/subsystems considered "useful", which by looking through current Android kernels is many of them.

Quoted 10 lines

The first goal can be achieved either by using device runtime PM and cpuidle to put all hardware into low-power states, transparently from the user space point of view, or by suspending the whole system. However, system suspend, in its current form, does not guarantee that the events of interest will always be responded to, since wakeup events (events that wake the CPU from idle and the system from suspend) that occur right after initiating suspend will not be processed until another possibly unrelated event wakes the system up again.

Minor point of clarification here.  I'm not requesting that the patch description be rewritten.  But this issue of lost wakeup events is more subtle than it appears.

Wakeup events can be lost in at least three different ways:

    1. A hardware signal (such as an IRQ) gets ignored.

    2. The hardware event occurs, but without effect since the kernel thread that would handle the event has been frozen. The event just ends up sitting in a queue somewhere until something else wakes up the system.

    3. The hardware event occurs and the kernel handles it fully, but the event propagates to userspace for further handling and the user program is already frozen.

1 is a hardware configuration failure (for example, it might happen as a result of using edge-triggered IRQs instead of level-triggered) and is outside the scope of this discussion.

2 generally represents a failure of the core PM subsystem, or a failure of some other part of the kernel to use the PM core correctly.  In theory we should be able to fix such mistakes.  Right now I'm aware of at least one possible failure scenario that could be fixed fairly easily.

3 is the type of failure that suspend blockers were really meant to handle, particularly the userspace suspend-blocker API.

IMO, we should strive to fix the existing type-2 failure modes. However it is worth pointing out that they are basically separate from the suspend-blocker mechanism.

And it might be a good idea to point out somewhere in the patch descriptions that suspend blockers are really meant to handle type-3 wakeup losses.

Quoted 10 lines

The first goal can be achieved either by using device runtime PM and cpuidle to put all hardware into low-power states, transparently from the user space point of view, or by suspending the whole system. However, system suspend, in its current form, does not guarantee that the events of interest will always be responded to, since wakeup events (events that wake the CPU from idle and the system from suspend) that occur right after initiating suspend will not be processed until another possibly unrelated event wakes the system up again.

Minor point of clarification here.  I'm not requesting that the patch description be rewritten.  But this issue of lost wakeup events is more subtle than it appears.

Wakeup events can be lost in at least three different ways:

    1. A hardware signal (such as an IRQ) gets ignored.

    2. The hardware event occurs, but without effect since the kernel thread that would handle the event has been frozen. The event just ends up sitting in a queue somewhere until something else wakes up the system.

    3. The hardware event occurs and the kernel handles it fully, but the event propagates to userspace for further handling and the user program is already frozen.

1 is a hardware configuration failure (for example, it might happen as a result of using edge-triggered IRQs instead of level-triggered) and is outside the scope of this discussion.

2 generally represents a failure of the core PM subsystem, or a failure of some other part of the kernel to use the PM core correctly.  In theory we should be able to fix such mistakes.  Right now I'm aware of at least one possible failure scenario that could be fixed fairly easily.

3 is the type of failure that suspend blockers were really meant to handle, particularly the userspace suspend-blocker API.

IMO, we should strive to fix the existing type-2 failure modes. However it is worth pointing out that they are basically separate from the suspend-blocker mechanism.

And it might be a good idea to point out somewhere in the patch descriptions that suspend blockers are really meant to handle type-3 wakeup losses.

Quoted 23 lines

Wakeup events can be lost in at least three different ways:

    1. A hardware signal (such as an IRQ) gets ignored.

    2. The hardware event occurs, but without effect since the kernel thread that would handle the event has been frozen. The event just ends up sitting in a queue somewhere until something else wakes up the system.

    3. The hardware event occurs and the kernel handles it fully, but the event propagates to userspace for further handling and the user program is already frozen.

1 is a hardware configuration failure (for example, it might happen as a result of using edge-triggered IRQs instead of level-triggered) and is outside the scope of this discussion.

2 generally represents a failure of the core PM subsystem, or a failure of some other part of the kernel to use the PM core correctly.  In theory we should be able to fix such mistakes.  Right now I'm aware of at least one possible failure scenario that could be fixed fairly easily.

Quoted 2 lines

3 is the type of failure that suspend blockers were really meant to handle, particularly the userspace suspend-blocker API.

I don't see a big difference between 2 and 3. You can use suspend blockers to handle either.

Quoted 23 lines

Wakeup events can be lost in at least three different ways:

    1. A hardware signal (such as an IRQ) gets ignored.

    2. The hardware event occurs, but without effect since the kernel thread that would handle the event has been frozen. The event just ends up sitting in a queue somewhere until something else wakes up the system.

    3. The hardware event occurs and the kernel handles it fully, but the event propagates to userspace for further handling and the user program is already frozen.

1 is a hardware configuration failure (for example, it might happen as a result of using edge-triggered IRQs instead of level-triggered) and is outside the scope of this discussion.

2 generally represents a failure of the core PM subsystem, or a failure of some other part of the kernel to use the PM core correctly.  In theory we should be able to fix such mistakes.  Right now I'm aware of at least one possible failure scenario that could be fixed fairly easily.

Quoted 2 lines

3 is the type of failure that suspend blockers were really meant to handle, particularly the userspace suspend-blocker API.

I don't see a big difference between 2 and 3. You can use suspend blockers to handle either.

Quoted 23 lines · Arve Hjønnevåg

Quoted 23 lines

Wakeup events can be lost in at least three different ways:

    1. A hardware signal (such as an IRQ) gets ignored.

    2. The hardware event occurs, but without effect since the kernel thread that would handle the event has been frozen. The event just ends up sitting in a queue somewhere until something else wakes up the system.

    3. The hardware event occurs and the kernel handles it fully, but the event propagates to userspace for further handling and the user program is already frozen.

1 is a hardware configuration failure (for example, it might happen as a result of using edge-triggered IRQs instead of level-triggered) and is outside the scope of this discussion.

2 generally represents a failure of the core PM subsystem, or a failure of some other part of the kernel to use the PM core correctly.  In theory we should be able to fix such mistakes.  Right now I'm aware of at least one possible failure scenario that could be fixed fairly easily.

Quoted 5 lines

Quoted 2 lines

3 is the type of failure that suspend blockers were really meant to handle, particularly the userspace suspend-blocker API.

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.

Quoted 23 lines · Arve Hjønnevåg

Quoted 23 lines

Wakeup events can be lost in at least three different ways:

    1. A hardware signal (such as an IRQ) gets ignored.

    2. The hardware event occurs, but without effect since the kernel thread that would handle the event has been frozen. The event just ends up sitting in a queue somewhere until something else wakes up the system.

    3. The hardware event occurs and the kernel handles it fully, but the event propagates to userspace for further handling and the user program is already frozen.

1 is a hardware configuration failure (for example, it might happen as a result of using edge-triggered IRQs instead of level-triggered) and is outside the scope of this discussion.

2 generally represents a failure of the core PM subsystem, or a failure of some other part of the kernel to use the PM core correctly.  In theory we should be able to fix such mistakes.  Right now I'm aware of at least one possible failure scenario that could be fixed fairly easily.

Quoted 5 lines

Quoted 2 lines

3 is the type of failure that suspend blockers were really meant to handle, particularly the userspace suspend-blocker API.

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.

Quoted 10 lines

Quoted 2 lines

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.

Quoted 10 lines

Quoted 2 lines

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.

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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.

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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.

Quoted 15 lines

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:

Quoted 9 lines

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

Quoted 15 lines

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:

Quoted 9 lines

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

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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.

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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.

Quoted 21 lines

Quoted 6 lines

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

Quoted 3 lines

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.

Quoted 21 lines

Quoted 6 lines

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

Quoted 3 lines

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.

Quoted 32 lines · Alan Stern

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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:

Quoted 9 lines

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

Quoted 32 lines · Alan Stern

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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:

Quoted 9 lines

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

Quoted 43 lines · Dmitry Torokhov

Quoted 32 lines · Alan Stern

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 43 lines · Dmitry Torokhov

Quoted 32 lines · Alan Stern

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 51 lines · Rafael J. Wysocki

Quoted 43 lines · Dmitry Torokhov

Quoted 32 lines · Alan Stern

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 6 lines

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.

You do exactly the same as what Alan done, but in userspace - poll, post "busy" event to suspend manager, read, process, retract "busy". Basically you still have the suspend blocker, but it is confined to your userspace.

Quoted 3 lines

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.

Yes, all devices (real or virtual), not only input ones, holding the queues have to refuse suspending for this to work.

Quoted 51 lines · Rafael J. Wysocki

Quoted 43 lines · Dmitry Torokhov

Quoted 32 lines · Alan Stern

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 6 lines

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.

You do exactly the same as what Alan done, but in userspace - poll, post "busy" event to suspend manager, read, process, retract "busy". Basically you still have the suspend blocker, but it is confined to your userspace.

Quoted 3 lines

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.

Yes, all devices (real or virtual), not only input ones, holding the queues have to refuse suspending for this to work.

Quoted 59 lines · Dmitry Torokhov

Quoted 51 lines · Rafael J. Wysocki

Quoted 43 lines · Dmitry Torokhov

Quoted 32 lines · Alan Stern

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 6 lines

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.

Quoted 4 lines

You do exactly the same as what Alan done, but in userspace - poll, post "busy" event to suspend manager, read, process, retract "busy". Basically you still have the suspend blocker, but it is confined to your userspace.

OK, now the question is why this is actually better.

Quoted 6 lines

Quoted 3 lines

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.

Yes, all devices (real or virtual), not only input ones, holding the queues have to refuse suspending for this to work.

So, basically, you'd prefer to move the entire complexity to user space.

I'm not sure if that's a big win and I'm not sure anyone is actually going to implement it (and some drivers would still have to be modified to participate in this framework).

in a different way, but at this point we'd rather need a _working_ _solution_ (in the form of code one can build and actually use).

I don't think it's realistic to expect the Android people to go and redesign their stuff along the lines you've described, because they have a working implementation (in the kernel) that they are satisfied with.

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically. Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

And what really is the drawback if we merge the patches? Quite frankly, I don't see any.

Quoted 59 lines · Dmitry Torokhov

Quoted 51 lines · Rafael J. Wysocki

Quoted 43 lines · Dmitry Torokhov

Quoted 32 lines · Alan Stern

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 6 lines

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.

Quoted 4 lines

You do exactly the same as what Alan done, but in userspace - poll, post "busy" event to suspend manager, read, process, retract "busy". Basically you still have the suspend blocker, but it is confined to your userspace.

OK, now the question is why this is actually better.

Quoted 6 lines

Quoted 3 lines

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.

Yes, all devices (real or virtual), not only input ones, holding the queues have to refuse suspending for this to work.

So, basically, you'd prefer to move the entire complexity to user space.

I'm not sure if that's a big win and I'm not sure anyone is actually going to implement it (and some drivers would still have to be modified to participate in this framework).

in a different way, but at this point we'd rather need a _working_ _solution_ (in the form of code one can build and actually use).

I don't think it's realistic to expect the Android people to go and redesign their stuff along the lines you've described, because they have a working implementation (in the kernel) that they are satisfied with.

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically. Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

And what really is the drawback if we merge the patches? Quite frankly, I don't see any.

So, basically, you'd prefer to move the entire complexity to user space.

I'm not sure if that's a big win

and I'm not sure anyone is actually going to
implement it (and some drivers would still have to be modified to participate in this framework).

So again, we have a hunch that the goal may be achieved
in a different way, but at this point we'd rather need a _working_ _solution_ (in the form of code one can build and actually use).

I don't think it's realistic to expect the Android people to go and redesign their stuff along the lines you've described, because they have a working implementation (in the kernel) that they are satisfied with.

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically. Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

And what really is the drawback if we merge the patches? Quite frankly, I don't see any.

So, basically, you'd prefer to move the entire complexity to user space.

I'm not sure if that's a big win

and I'm not sure anyone is actually going to
implement it (and some drivers would still have to be modified to participate in this framework).

So again, we have a hunch that the goal may be achieved
in a different way, but at this point we'd rather need a _working_ _solution_ (in the form of code one can build and actually use).

I don't think it's realistic to expect the Android people to go and redesign their stuff along the lines you've described, because they have a working implementation (in the kernel) that they are satisfied with.

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically. Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

And what really is the drawback if we merge the patches? Quite frankly, I don't see any.

Quoted 8 lines

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically.  Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

And what really is the drawback if we merge the patches?  Quite frankly, I don't see any.

Adding stuff that is not beneficial to anyone but a particular platform? It is uncommon to say the least.

Quoted 8 lines

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically.  Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

And what really is the drawback if we merge the patches?  Quite frankly, I don't see any.

Adding stuff that is not beneficial to anyone but a particular platform? It is uncommon to say the least.

Quoted 1 line · Rafael J. Wysocki

On Tuesday 25 May 2010, Dmitry Torokhov wrote:

Quoted 8 lines

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically. Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

And what really is the drawback if we merge the patches? Quite frankly, I don't see any.

Adding stuff that is not beneficial to anyone but a particular platform? It is uncommon to say the least.

Quoted 1 line · Rafael J. Wysocki

On Tuesday 25 May 2010, Dmitry Torokhov wrote:

Quoted 8 lines

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically. Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

And what really is the drawback if we merge the patches? Quite frankly, I don't see any.

Adding stuff that is not beneficial to anyone but a particular platform? It is uncommon to say the least.

Quoted 1 line · Rafael J. Wysocki

So, basically, you'd prefer to move the entire complexity to user space.

No, just the complexity of the userspace suspend blockers. That was one of the parts of the interface that people objected to, after all.

Quoted 1 line

I'm not sure if that's a big win

It's not a _big_ win, but it is an improvement IMO.

Quoted 3 lines

and I'm not sure anyone is actually going to
implement it (and some drivers would still have to be modified to participate in this framework).

Of course drivers have to be modified. The kernel-layer suspend blockers aren't affected by this proposal, so they still have to be implemented.

Quoted 3 lines

So again, we have a hunch that the goal may be achieved
in a different way, but at this point we'd rather need a _working_ _solution_ (in the form of code one can build and actually use).

It's not very different from what has been submitted, and I think there's little doubt that it could be built and used fairly easily. All we're talking about is removing the userspace suspend-blocker API and the opportunistic workqueue, replacing them with an "opportunistic" entry in /sys/power/state, and setting up a userspace power manager process.

Quoted 3 lines

I don't think it's realistic to expect the Android people to go and redesign their stuff along the lines you've described, because they have a working implementation (in the kernel) that they are satisfied with.

The redesign would be pretty small. The kernel changes relative to what they have submitted are minimal, mostly just removing a few of their additions. Furthermore, we've been told that Android _already_ funnels all its userspace suspend-blocker work through a single process. All that would be needed would be to make that process initiate an opportunistic suspend whenever no userspace suspend blockers were active.

Quoted 6 lines

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically. Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

Quoted 2 lines

And what really is the drawback if we merge the patches? Quite frankly, I don't see any.

You don't seem to appreciate how small a change Dmitry has proposed. Almost all of the suspend-blocker work would remain as in the submitted patches. The only difference is that the userspace API and opportunistic-suspend implementation would be simplified, by moving some of the work out of the kernel.

Quoted 1 line · Rafael J. Wysocki

So, basically, you'd prefer to move the entire complexity to user space.

No, just the complexity of the userspace suspend blockers. That was one of the parts of the interface that people objected to, after all.

Quoted 1 line

I'm not sure if that's a big win

It's not a _big_ win, but it is an improvement IMO.

Quoted 3 lines

and I'm not sure anyone is actually going to
implement it (and some drivers would still have to be modified to participate in this framework).

Of course drivers have to be modified. The kernel-layer suspend blockers aren't affected by this proposal, so they still have to be implemented.

Quoted 3 lines

So again, we have a hunch that the goal may be achieved
in a different way, but at this point we'd rather need a _working_ _solution_ (in the form of code one can build and actually use).

It's not very different from what has been submitted, and I think there's little doubt that it could be built and used fairly easily. All we're talking about is removing the userspace suspend-blocker API and the opportunistic workqueue, replacing them with an "opportunistic" entry in /sys/power/state, and setting up a userspace power manager process.

Quoted 3 lines

I don't think it's realistic to expect the Android people to go and redesign their stuff along the lines you've described, because they have a working implementation (in the kernel) that they are satisfied with.

The redesign would be pretty small. The kernel changes relative to what they have submitted are minimal, mostly just removing a few of their additions. Furthermore, we've been told that Android _already_ funnels all its userspace suspend-blocker work through a single process. All that would be needed would be to make that process initiate an opportunistic suspend whenever no userspace suspend blockers were active.

Quoted 6 lines

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically. Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

Quoted 2 lines

And what really is the drawback if we merge the patches? Quite frankly, I don't see any.

You don't seem to appreciate how small a change Dmitry has proposed. Almost all of the suspend-blocker work would remain as in the submitted patches. The only difference is that the userspace API and opportunistic-suspend implementation would be simplified, by moving some of the work out of the kernel.

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

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.

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

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.

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 48 lines · Alan Stern

Quoted 1 line · Rafael J. Wysocki

So, basically, you'd prefer to move the entire complexity to user space.

No, just the complexity of the userspace suspend blockers.  That was one of the parts of the interface that people objected to, after all.

Quoted 1 line

I'm not sure if that's a big win

It's not a _big_ win, but it is an improvement IMO.

Quoted 3 lines

 and I'm not sure anyone is actually going to
implement it (and some drivers would still have to be modified to participate in this framework).

Of course drivers have to be modified.  The kernel-layer suspend blockers aren't affected by this proposal, so they still have to be implemented.

Quoted 3 lines

 So again, we have a hunch that the goal may be achieved
in a different way, but at this point we'd rather need a _working_ _solution_ (in the form of code one can build and actually use).

It's not very different from what has been submitted, and I think there's little doubt that it could be built and used fairly easily. All we're talking about is removing the userspace suspend-blocker API and the opportunistic workqueue, replacing them with an "opportunistic" entry in /sys/power/state, and setting up a userspace power manager process.

Quoted 3 lines

I don't think it's realistic to expect the Android people to go and redesign their stuff along the lines you've described, because they have a working implementation (in the kernel) that they are satisfied with.

The redesign would be pretty small.  The kernel changes relative to what they have submitted are minimal, mostly just removing a few of their additions.  Furthermore, we've been told that Android _already_ funnels all its userspace suspend-blocker work through a single process.  All that would be needed would be to make that process initiate an opportunistic suspend whenever no userspace suspend blockers were active.

Quoted 6 lines

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically.  Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

Quoted 8 lines

Quoted 2 lines

And what really is the drawback if we merge the patches?  Quite frankly, I don't see any.

You don't seem to appreciate how small a change Dmitry has proposed. Almost all of the suspend-blocker work would remain as in the submitted patches.  The only difference is that the userspace API and opportunistic-suspend implementation would be simplified, by moving some of the work out of the kernel.

No, I don't really think it's going to be a small change.  The problem is that for the Android people changing user space is very hard, so I don't think this realy is an option, given that they would have to implement it themselves, test it, validate it on multiple different hardware platforms etc. and _then_ resubmit the feature without any guarantee that it will be merged.

So, my opinion is that we only have a choice to either take the feature as is now, or reject it altogether and live with the consequeces in each case.  And quite frankly I don't feel like I'm in position to make that decision.

Quoted 48 lines · Alan Stern

Quoted 1 line · Rafael J. Wysocki

So, basically, you'd prefer to move the entire complexity to user space.

No, just the complexity of the userspace suspend blockers.  That was one of the parts of the interface that people objected to, after all.

Quoted 1 line

I'm not sure if that's a big win

It's not a _big_ win, but it is an improvement IMO.

Quoted 3 lines

 and I'm not sure anyone is actually going to
implement it (and some drivers would still have to be modified to participate in this framework).

Of course drivers have to be modified.  The kernel-layer suspend blockers aren't affected by this proposal, so they still have to be implemented.

Quoted 3 lines

 So again, we have a hunch that the goal may be achieved
in a different way, but at this point we'd rather need a _working_ _solution_ (in the form of code one can build and actually use).

It's not very different from what has been submitted, and I think there's little doubt that it could be built and used fairly easily. All we're talking about is removing the userspace suspend-blocker API and the opportunistic workqueue, replacing them with an "opportunistic" entry in /sys/power/state, and setting up a userspace power manager process.

Quoted 3 lines

I don't think it's realistic to expect the Android people to go and redesign their stuff along the lines you've described, because they have a working implementation (in the kernel) that they are satisfied with.

The redesign would be pretty small.  The kernel changes relative to what they have submitted are minimal, mostly just removing a few of their additions.  Furthermore, we've been told that Android _already_ funnels all its userspace suspend-blocker work through a single process.  All that would be needed would be to make that process initiate an opportunistic suspend whenever no userspace suspend blockers were active.

Quoted 6 lines

Now, we can reject their patches, but that's not going to cause any progress to happen, realistically.  Quite on the contrary, Android will continue to use wakelocks and Android driver writers will continue to ignore the mainline and the gap between the two kernel lines will only get wider and wider over time.

Quoted 8 lines

Quoted 2 lines

And what really is the drawback if we merge the patches?  Quite frankly, I don't see any.

You don't seem to appreciate how small a change Dmitry has proposed. Almost all of the suspend-blocker work would remain as in the submitted patches.  The only difference is that the userspace API and opportunistic-suspend implementation would be simplified, by moving some of the work out of the kernel.

No, I don't really think it's going to be a small change.  The problem is that for the Android people changing user space is very hard, so I don't think this realy is an option, given that they would have to implement it themselves, test it, validate it on multiple different hardware platforms etc. and _then_ resubmit the feature without any guarantee that it will be merged.

So, my opinion is that we only have a choice to either take the feature as is now, or reject it altogether and live with the consequeces in each case.  And quite frankly I don't feel like I'm in position to make that decision.

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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

Quoted 59 lines · Dmitry Torokhov

Quoted 57 lines · Rafael J. Wysocki

Quoted 43 lines · Dmitry Torokhov

Quoted 32 lines · Alan Stern

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

You do exactly the same as what Alan done, but in userspace - poll, post "busy" event to suspend manager, read, process, retract "busy". Basically you still have the suspend blocker, but it is confined to your userspace.

OK, now the question is why this is actually better.

Quoted 59 lines · Dmitry Torokhov

Quoted 57 lines · Rafael J. Wysocki

Quoted 43 lines · Dmitry Torokhov

Quoted 32 lines · Alan Stern

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines

Quoted 2 lines

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.

Quoted 5 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

You do exactly the same as what Alan done, but in userspace - poll, post "busy" event to suspend manager, read, process, retract "busy". Basically you still have the suspend blocker, but it is confined to your userspace.

OK, now the question is why this is actually better.

Quoted 82 lines

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

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.

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.

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.

Quoted 82 lines

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

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.

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.

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.

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 90 lines · Arve Hjønnevåg

Quoted 82 lines

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

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:

Quoted 3 lines

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,

or releases the
currently held key.

Quoted 4 lines

2. Wait a tiny bit after last application notified you that it finished processing events.

Quoted 4 lines

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.

Quoted 6 lines

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.

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 90 lines · Arve Hjønnevåg

Quoted 82 lines

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

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:

Quoted 3 lines

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,

or releases the
currently held key.

Quoted 4 lines

2. Wait a tiny bit after last application notified you that it finished processing events.

Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 95 lines

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 93 lines · Arve Hjønnevåg

Quoted 82 lines

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 4 lines

or releases the
currently held key.

Userspace consumer should wait for the key release and retract "busy" once event is received and handled.

Quoted 9 lines

Quoted 8 lines

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.

Quoted 3 lines

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.

Quoted 7 lines

Quoted 6 lines

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.

Quoted 1 line

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.

Quoted 95 lines

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 93 lines · Arve Hjønnevåg

Quoted 82 lines

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 4 lines

or releases the
currently held key.

Userspace consumer should wait for the key release and retract "busy" once event is received and handled.

Quoted 9 lines

Quoted 8 lines

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.

Quoted 3 lines

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.

Quoted 7 lines

Quoted 6 lines

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.

Quoted 1 line

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.

Quoted 82 lines · Arve Hjønnevåg

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

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

Then the userspace suspend manager should be a little more clever and should not blindly retry continuously.

It should be more like a governor which makes some simple decisions based on previous events, simple heuristics, uses timeouts etc.,

Quoted 82 lines · Arve Hjønnevåg

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 74 lines · Arve Hjønnevåg

Quoted 65 lines

Quoted 53 lines · Dmitry Torokhov

Quoted 48 lines · Alan Stern

Quoted 25 lines · Dmitry Torokhov

Quoted 15 lines

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:

Quoted 9 lines

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.

Quoted 6 lines

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

Quoted 3 lines

Maybe I'm missing something here...  No doubt someone will point it out if I am.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

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

Then the userspace suspend manager should be a little more clever and should not blindly retry continuously.

It should be more like a governor which makes some simple decisions based on previous events, simple heuristics, uses timeouts etc.,

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 7 lines · Arve Hjønnevåg

Quoted 4 lines

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?

Quoted 6 lines

Quoted 5 lines

or releases the
currently held key.

Quoted 3 lines

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.

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

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.

Quoted 8 lines

Quoted 3 lines

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.

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

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.

Quoted 5 lines

Quoted 1 line

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.

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 7 lines · Arve Hjønnevåg

Quoted 4 lines

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?

Quoted 6 lines

Quoted 5 lines

or releases the
currently held key.

Quoted 3 lines

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.

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

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.

Quoted 8 lines

Quoted 3 lines

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.

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

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.

Quoted 5 lines

Quoted 1 line

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.

Quoted 7 lines

2010/5/25 Dmitry Torokhov <[email protected]>:

Quoted 6 lines · Arve Hjønnevåg

Quoted 4 lines

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,

Quoted 4 lines

Quoted 1 line

Employ reasonable timeout.

Timeout for what? Stop trying to suspend altogether, stop scanning for key changes, or a more "reasonable" poll interval?

Stop trying to suspend for a fraction of a second to see if user wants to press another key.

Quoted 10 lines

Quoted 9 lines

Quoted 5 lines

or releases the
currently held key.

Userspace consumer should wait for the key release and retract "busy" once event is received and handled.

Quoted 3 lines

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.

OTOH nor does kernel driver know whether system suspend should be blocked while it is scanning.

What should happen in I press KEY_SUSPEND?
Do we always want to wait till user releases it?

Quoted 39 lines

Quoted 11 lines

Quoted 11 lines

Quoted 10 lines

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.

Quoted 8 lines

Quoted 3 lines

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.

Quoted 12 lines

Quoted 8 lines

Quoted 6 lines

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.

Quoted 2 lines

If the device implements runtime PM it should already be powered-down if it is not in use.

It could have been in use but userspace-initiated suspend accelerated it entering low power state.