From: Kevin Hilman on
Arve Hj�nnev�g <arve(a)android.com> writes:

> 2010/5/25 Dmitry Torokhov <dmitry.torokhov(a)gmail.com>:
>> On Tue, May 25, 2010 at 03:23:23PM -0700, Arve Hj�nnev�g wrote:
>>> On Tue, May 25, 2010 at 11:47 AM, Dmitry Torokhov
>>> <dmitry.torokhov(a)gmail.com> wrote:
>>> > On Tue, May 25, 2010 at 02:35:17PM -0400, Alan Stern wrote:
>>> >> On Tue, 25 May 2010, Dmitry Torokhov wrote:
>>> >>
>>> >> > > Here's the scenario:
>>> >> > >
>>> >> > > The system is awake, and the user presses a key. The keyboard driver
>>> >> > > processes the keystroke and puts it in an input queue. �A user process
>>> >> > > reads it from the event queue, thereby emptying the queue.
>>> >> > >
>>> >> > > At that moment, the system decides to go into opportunistic suspend.
>>> >> > > Since the input queue is empty, there's nothing to stop it. �As the
>>> >> > > first step, userspace is frozen -- before the process has a chance to
>>> >> > > do anything with the keystroke it just read. �As a result, the system
>>> >> > > stays asleep until something else wakes it up, even though the
>>> >> > > keystroke was important and should have prevented it from sleeping.
>>> >> > >
>>> >> > > Suspend blockers protect against this scenario. �Here's how:
>>> >> > >
>>> >> > > The user process doesn't read the input queue directly; instead it
>>> >> > > does a select or poll. �When it sees there is data in the queue, it
>>> >> > > first acquires a suspend blocker and then reads the data.
>>> >> > >
>>> >> > > Now the system _can't_ go into opportunistic suspend, because a suspend
>>> >> > > blocker is active. �The user process can do whatever it wants with the
>>> >> > > keystroke. �When it is finished, it releases the suspend blocker and
>>> >> > > loops back to the select/poll call.
>>> >> > >
>>> >> >
>>> >> > What you describe can be done in userspace though, via a "suspend manager"
>>> >> > process. Tasks reading input events will post "busy" events to stop the
>>> >> > manager process from sending system into suspend. But this can be confined to
>>> >> > Android userspace, leaving the kernel as is (well, kernel needs to be modified
>>> >> > to not go into suspend with full queues, but that is using existing kernel
>>> >> > APIs).
>>> >>
>>> >> I think that could be made to work. �And it might remove the need for
>>> >> the userspace suspend-blocker API, which would be an advantage. �It
>>> >> could even remove the need for the opportunistic-suspend workqueue --
>>> >> opportunistic suspends would be initiated by the "suspend manager"
>>> >> process instead of by the kernel.
>>> >>
>>> >> However you still have the issue of modifying the kernel drivers to
>>> >> disallow opportunistic suspend if their queues are non-empty. �Doing
>>> >> that is more or less equivalent to implementing kernel-level suspend
>>> >> blockers. �(The suspend blocker approach is slightly more efficient,
>>> >> because it will prevent a suspend from starting if a queue is
>>> >> non-empty, instead of allowing the suspend to start and then aborting
>>> >> it partway through.)
>>> >>
>>> >> Maybe I'm missing something here... �No doubt someone will point it out
>>> >> if I am.
>>> >>
>>> >
>>> > Well, from my perspective that would limit changes to the evdev driver
>>> > (well, limited input core plumbing will be needed) but that is using the
>>> > current PM infrastructure. The HW driver changes will be limited to what
>>> > you described "type 2" in your other e-mail.
>>> >
>>> > Also, not suspending while events are in progress) is probably
>>> > beneficial for platforms other than Android as well. So unless I am
>>> > missing something this sounds like a win.
>>> >
>>>
>>> How would this limit the changes you need in the evdev driver? It need
>>> to block suspend when there are unprocessed events in some queues.
>>> Suspend blockers gives you an api to do this, without it, you check
>>> the queues in your suspend hook and abort suspend if they are not
>>> empty. Without suspend blockers you have no api to signal that it is
>>> OK to suspend again, so you are forcing the thread that tried to
>>> suspend to poll until you stop aborting suspend.
>>
>> No, you do not need to poll. You just set a timeout (short or long,
>> depending on your needs) and if no userspace task blocked suspend
>> durng that time you attempt to initiate suspend from your manager
>> process. If it succeeds - good, if not that means that more events came
>> your way and you have to do it later.
>>
>
> How is that not polling? If the user is holding down a key, the keypad
> driver has to block suspend, and user space will try to suspend again
> and again and again...

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

Kevin

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Dmitry Torokhov on
On Tue, May 25, 2010 at 03:37:48PM -0700, Arve Hj�nnev�g wrote:
> 2010/5/25 Dmitry Torokhov <dmitry.torokhov(a)gmail.com>:
> > On Tue, May 25, 2010 at 03:23:23PM -0700, Arve Hj�nnev�g wrote:
> >> On Tue, May 25, 2010 at 11:47 AM, Dmitry Torokhov
> >> <dmitry.torokhov(a)gmail.com> wrote:
> >> > On Tue, May 25, 2010 at 02:35:17PM -0400, Alan Stern wrote:
> >> >> On Tue, 25 May 2010, Dmitry Torokhov wrote:
> >> >>
> >> >> > > Here's the scenario:
> >> >> > >
> >> >> > > The system is awake, and the user presses a key. The keyboard driver
> >> >> > > processes the keystroke and puts it in an input queue. �A user process
> >> >> > > reads it from the event queue, thereby emptying the queue.
> >> >> > >
> >> >> > > At that moment, the system decides to go into opportunistic suspend.
> >> >> > > Since the input queue is empty, there's nothing to stop it. �As the
> >> >> > > first step, userspace is frozen -- before the process has a chance to
> >> >> > > do anything with the keystroke it just read. �As a result, the system
> >> >> > > stays asleep until something else wakes it up, even though the
> >> >> > > keystroke was important and should have prevented it from sleeping.
> >> >> > >
> >> >> > > Suspend blockers protect against this scenario. �Here's how:
> >> >> > >
> >> >> > > The user process doesn't read the input queue directly; instead it
> >> >> > > does a select or poll. �When it sees there is data in the queue, it
> >> >> > > first acquires a suspend blocker and then reads the data.
> >> >> > >
> >> >> > > Now the system _can't_ go into opportunistic suspend, because a suspend
> >> >> > > blocker is active. �The user process can do whatever it wants with the
> >> >> > > keystroke. �When it is finished, it releases the suspend blocker and
> >> >> > > loops back to the select/poll call.
> >> >> > >
> >> >> >
> >> >> > What you describe can be done in userspace though, via a "suspend manager"
> >> >> > process. Tasks reading input events will post "busy" events to stop the
> >> >> > manager process from sending system into suspend. But this can be confined to
> >> >> > Android userspace, leaving the kernel as is (well, kernel needs to be modified
> >> >> > to not go into suspend with full queues, but that is using existing kernel
> >> >> > APIs).
> >> >>
> >> >> I think that could be made to work. �And it might remove the need for
> >> >> the userspace suspend-blocker API, which would be an advantage. �It
> >> >> could even remove the need for the opportunistic-suspend workqueue --
> >> >> opportunistic suspends would be initiated by the "suspend manager"
> >> >> process instead of by the kernel.
> >> >>
> >> >> However you still have the issue of modifying the kernel drivers to
> >> >> disallow opportunistic suspend if their queues are non-empty. �Doing
> >> >> that is more or less equivalent to implementing kernel-level suspend
> >> >> blockers. �(The suspend blocker approach is slightly more efficient,
> >> >> because it will prevent a suspend from starting if a queue is
> >> >> non-empty, instead of allowing the suspend to start and then aborting
> >> >> it partway through.)
> >> >>
> >> >> Maybe I'm missing something here... �No doubt someone will point it out
> >> >> if I am.
> >> >>
> >> >
> >> > Well, from my perspective that would limit changes to the evdev driver
> >> > (well, limited input core plumbing will be needed) but that is using the
> >> > current PM infrastructure. The HW driver changes will be limited to what
> >> > you described "type 2" in your other e-mail.
> >> >
> >> > Also, not suspending while events are in progress) is probably
> >> > beneficial for platforms other than Android as well. So unless I am
> >> > missing something this sounds like a win.
> >> >
> >>
> >> How would this limit the changes you need in the evdev driver? It need
> >> to block suspend when there are unprocessed events in some queues.
> >> Suspend blockers gives you an api to do this, without it, you check
> >> the queues in your suspend hook and abort suspend if they are not
> >> empty. Without suspend blockers you have no api to signal that it is
> >> OK to suspend again, so you are forcing the thread that tried to
> >> suspend to poll until you stop aborting suspend.
> >
> > No, you do not need to poll. You just set a timeout (short or long,
> > depending on your needs) and if no userspace task blocked suspend
> > durng that time you attempt to initiate suspend from your manager
> > process. If it succeeds - good, if not that means that more events came
> > your way and you have to do it later.
> >
>
> How is that not polling? If the user is holding down a key, the keypad
> driver has to block suspend, and user space will try to suspend again
> and again and again...
>

If your userpsace is that stupid - sure. However, you can:

1. Notify the suspend manager process that he rest of your userspace is
busy handling keystrokes so that it does not try to suspend while there
are events pending.

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.

--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Kevin Hilman on
"Rafael J. Wysocki" <rjw(a)sisk.pl> writes:

> On Tuesday 25 May 2010, Dmitry Torokhov wrote:
>> On Tue, May 25, 2010 at 09:47:22PM +0200, Rafael J. Wysocki wrote:
>> > On Tuesday 25 May 2010, Dmitry Torokhov wrote:
>> > > On Tuesday 25 May 2010 11:08:03 am Alan Stern wrote:
>> > > > On Tue, 25 May 2010, Dmitry Torokhov wrote:
>> > > > > > > I don't see a big difference between 2 and 3. You can use suspend
>> > > > > > > blockers to handle either.
>> > > > > >
>> > > > > > You can, but they aren't necessary. If 2 were the only reason for
>> > > > > > suspend blockers, I would say they shouldn't be merged.
>> > > > > >
>> > > > > > Whereas 3, on the other hand, can _not_ be handled by any existing
>> > > > > > mechanism. 3 is perhaps the most important reason for using suspend
>> > > > > > blockers.
>> > > > >
>> > > > > I do not see why 3 has to be implemented using suspend blockers either.
>> > > > > If you are concerned that event gets stuck somewhere in the stack make
>> > > > > sure that devices in the stack do not suspend while their queue is not
>> > > > > empty. This way if you try opportunistic suspend it will keep failing
>> > > > > until you drained all important queues.
>> > > >
>> > > > Here's the scenario:
>> > > >
>> > > > The system is awake, and the user presses a key. The keyboard driver
>> > > > processes the keystroke and puts it in an input queue. A user process
>> > > > reads it from the event queue, thereby emptying the queue.
>> > > >
>> > > > At that moment, the system decides to go into opportunistic suspend.
>> > > > Since the input queue is empty, there's nothing to stop it. As the
>> > > > first step, userspace is frozen -- before the process has a chance to
>> > > > do anything with the keystroke it just read. As a result, the system
>> > > > stays asleep until something else wakes it up, even though the
>> > > > keystroke was important and should have prevented it from sleeping.
>> > > >
>> > > > Suspend blockers protect against this scenario. Here's how:
>> > > >
>> > > > The user process doesn't read the input queue directly; instead it
>> > > > does a select or poll. When it sees there is data in the queue, it
>> > > > first acquires a suspend blocker and then reads the data.
>> > > >
>> > > > Now the system _can't_ go into opportunistic suspend, because a suspend
>> > > > blocker is active. The user process can do whatever it wants with the
>> > > > keystroke. When it is finished, it releases the suspend blocker and
>> > > > loops back to the select/poll call.
>> > > >
>> > >
>> > > What you describe can be done in userspace though, via a "suspend manager"
>> > > process. Tasks reading input events will post "busy" events to stop the
>> > > manager process from sending system into suspend. But this can be confined to
>> > > Android userspace, leaving the kernel as is (well, kernel needs to be modified
>> > > to not go into suspend with full queues, but that is using existing kernel
>> > > APIs).
>> >
>> > For that to work, you'd have to make the user space suspend manager prevent
>> > key-reading processes from emptying the queue before it orders the kernel to
>> > put the system to sleep. Otherwise it still is possible that the queue will be
>> > emptied right at the moment it writes to /sys/power/state and the scenario
>> > described by Alan is going to happen.
>> >
>>
>> 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.

A couple things come to mind...

1. Fixes problems for *all* kernel users, not just Android.

The kernel changes (refuse to suspend) would be done in a way that
would fix problems in the traditional suspend path as well as the
opportunistic suspend path, thus benefiting everyone.

2. Keep policy out of the kernel

A userspace suspend manager could implement _policy_ decisions in a
platform specific way, rather than having policy hard-coded into the
kernel.

Keeping the policy/governor in userspace would also allow various
governor techniques to be experimented with (polling/timeout
intervals, etc.) without having to patch the kernel.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Arve Hjønnevåg on
2010/5/25 Dmitry Torokhov <dmitry.torokhov(a)gmail.com>:
> On Tue, May 25, 2010 at 03:37:48PM -0700, Arve Hj�nnev�g wrote:
>> 2010/5/25 Dmitry Torokhov <dmitry.torokhov(a)gmail.com>:
>> > On Tue, May 25, 2010 at 03:23:23PM -0700, Arve Hj�nnev�g wrote:
>> >> On Tue, May 25, 2010 at 11:47 AM, Dmitry Torokhov
>> >> <dmitry.torokhov(a)gmail.com> wrote:
>> >> > On Tue, May 25, 2010 at 02:35:17PM -0400, Alan Stern wrote:
>> >> >> On Tue, 25 May 2010, Dmitry Torokhov wrote:
>> >> >>
>> >> >> > > Here's the scenario:
>> >> >> > >
>> >> >> > > The system is awake, and the user presses a key. The keyboard driver
>> >> >> > > processes the keystroke and puts it in an input queue. �A user process
>> >> >> > > reads it from the event queue, thereby emptying the queue.
>> >> >> > >
>> >> >> > > At that moment, the system decides to go into opportunistic suspend.
>> >> >> > > Since the input queue is empty, there's nothing to stop it. �As the
>> >> >> > > first step, userspace is frozen -- before the process has a chance to
>> >> >> > > do anything with the keystroke it just read. �As a result, the system
>> >> >> > > stays asleep until something else wakes it up, even though the
>> >> >> > > keystroke was important and should have prevented it from sleeping.
>> >> >> > >
>> >> >> > > Suspend blockers protect against this scenario. �Here's how:
>> >> >> > >
>> >> >> > > The user process doesn't read the input queue directly; instead it
>> >> >> > > does a select or poll. �When it sees there is data in the queue, it
>> >> >> > > first acquires a suspend blocker and then reads the data.
>> >> >> > >
>> >> >> > > Now the system _can't_ go into opportunistic suspend, because a suspend
>> >> >> > > blocker is active. �The user process can do whatever it wants with the
>> >> >> > > keystroke. �When it is finished, it releases the suspend blocker and
>> >> >> > > loops back to the select/poll call.
>> >> >> > >
>> >> >> >
>> >> >> > What you describe can be done in userspace though, via a "suspend manager"
>> >> >> > process. Tasks reading input events will post "busy" events to stop the
>> >> >> > manager process from sending system into suspend. But this can be confined to
>> >> >> > Android userspace, leaving the kernel as is (well, kernel needs to be modified
>> >> >> > to not go into suspend with full queues, but that is using existing kernel
>> >> >> > APIs).
>> >> >>
>> >> >> I think that could be made to work. �And it might remove the need for
>> >> >> the userspace suspend-blocker API, which would be an advantage. �It
>> >> >> could even remove the need for the opportunistic-suspend workqueue --
>> >> >> opportunistic suspends would be initiated by the "suspend manager"
>> >> >> process instead of by the kernel.
>> >> >>
>> >> >> However you still have the issue of modifying the kernel drivers to
>> >> >> disallow opportunistic suspend if their queues are non-empty. �Doing
>> >> >> that is more or less equivalent to implementing kernel-level suspend
>> >> >> blockers. �(The suspend blocker approach is slightly more efficient,
>> >> >> because it will prevent a suspend from starting if a queue is
>> >> >> non-empty, instead of allowing the suspend to start and then aborting
>> >> >> it partway through.)
>> >> >>
>> >> >> Maybe I'm missing something here... �No doubt someone will point it out
>> >> >> if I am.
>> >> >>
>> >> >
>> >> > Well, from my perspective that would limit changes to the evdev driver
>> >> > (well, limited input core plumbing will be needed) but that is using the
>> >> > current PM infrastructure. The HW driver changes will be limited to what
>> >> > you described "type 2" in your other e-mail.
>> >> >
>> >> > Also, not suspending while events are in progress) is probably
>> >> > beneficial for platforms other than Android as well. So unless I am
>> >> > missing something this sounds like a win.
>> >> >
>> >>
>> >> How would this limit the changes you need in the evdev driver? It need
>> >> to block suspend when there are unprocessed events in some queues.
>> >> Suspend blockers gives you an api to do this, without it, you check
>> >> the queues in your suspend hook and abort suspend if they are not
>> >> empty. Without suspend blockers you have no api to signal that it is
>> >> OK to suspend again, so you are forcing the thread that tried to
>> >> suspend to poll until you stop aborting suspend.
>> >
>> > No, you do not need to poll. You just set a timeout (short or long,
>> > depending on your needs) and if no userspace task blocked suspend
>> > durng that time you attempt to initiate suspend from your manager
>> > process. If it succeeds - good, if not that means that more events came
>> > your way and you have to do it later.
>> >
>>
>> How is that not polling? If the user is holding down a key, the keypad
>> driver has to block suspend, and user space will try to suspend again
>> and again and again...
>>
>
> If your userpsace is that stupid - sure. However, you can:
>
> 1. Notify the suspend manager process that he rest of your userspace is
> busy handling keystrokes so that it does not try to suspend while there
> are events pending.

You are missing the point. There are no event pending. The kernel
reported the key down event, it was handled, but the keypad driver is
still scanning to see if the user presses another key, or releases the
currently held key.

>
> 2. Wait a tiny bit after last application notified you that it finished
> processing events.
>
> So basically the difference is that with in-kernel suspend blockers,
> there is a tiny window where we haven't started the suspend yet but are
> about to the driver has a chance to prevent entire system from starting
> sleep.

No, the difference is that if a driver needs to prevent suspend for an
extended period of time, you don't have user space continuously
polling to see if it can suspend.

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

--
Arve Hj�nnev�g
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Dmitry Torokhov on
On Tue, May 25, 2010 at 04:13:35PM -0700, Arve Hj�nnev�g wrote:
> 2010/5/25 Dmitry Torokhov <dmitry.torokhov(a)gmail.com>:
> > On Tue, May 25, 2010 at 03:37:48PM -0700, Arve Hj�nnev�g wrote:
> >> 2010/5/25 Dmitry Torokhov <dmitry.torokhov(a)gmail.com>:
> >> > On Tue, May 25, 2010 at 03:23:23PM -0700, Arve Hj�nnev�g wrote:
> >> >> On Tue, May 25, 2010 at 11:47 AM, Dmitry Torokhov
> >> >> <dmitry.torokhov(a)gmail.com> wrote:
> >> >> > On Tue, May 25, 2010 at 02:35:17PM -0400, Alan Stern wrote:
> >> >> >> On Tue, 25 May 2010, Dmitry Torokhov wrote:
> >> >> >>
> >> >> >> > > Here's the scenario:
> >> >> >> > >
> >> >> >> > > The system is awake, and the user presses a key. The keyboard driver
> >> >> >> > > processes the keystroke and puts it in an input queue. �A user process
> >> >> >> > > reads it from the event queue, thereby emptying the queue.
> >> >> >> > >
> >> >> >> > > At that moment, the system decides to go into opportunistic suspend.
> >> >> >> > > Since the input queue is empty, there's nothing to stop it. �As the
> >> >> >> > > first step, userspace is frozen -- before the process has a chance to
> >> >> >> > > do anything with the keystroke it just read. �As a result, the system
> >> >> >> > > stays asleep until something else wakes it up, even though the
> >> >> >> > > keystroke was important and should have prevented it from sleeping.
> >> >> >> > >
> >> >> >> > > Suspend blockers protect against this scenario. �Here's how:
> >> >> >> > >
> >> >> >> > > The user process doesn't read the input queue directly; instead it
> >> >> >> > > does a select or poll. �When it sees there is data in the queue, it
> >> >> >> > > first acquires a suspend blocker and then reads the data.
> >> >> >> > >
> >> >> >> > > Now the system _can't_ go into opportunistic suspend, because a suspend
> >> >> >> > > blocker is active. �The user process can do whatever it wants with the
> >> >> >> > > keystroke. �When it is finished, it releases the suspend blocker and
> >> >> >> > > loops back to the select/poll call.
> >> >> >> > >
> >> >> >> >
> >> >> >> > What you describe can be done in userspace though, via a "suspend manager"
> >> >> >> > process. Tasks reading input events will post "busy" events to stop the
> >> >> >> > manager process from sending system into suspend. But this can be confined to
> >> >> >> > Android userspace, leaving the kernel as is (well, kernel needs to be modified
> >> >> >> > to not go into suspend with full queues, but that is using existing kernel
> >> >> >> > APIs).
> >> >> >>
> >> >> >> I think that could be made to work. �And it might remove the need for
> >> >> >> the userspace suspend-blocker API, which would be an advantage. �It
> >> >> >> could even remove the need for the opportunistic-suspend workqueue --
> >> >> >> opportunistic suspends would be initiated by the "suspend manager"
> >> >> >> process instead of by the kernel.
> >> >> >>
> >> >> >> However you still have the issue of modifying the kernel drivers to
> >> >> >> disallow opportunistic suspend if their queues are non-empty. �Doing
> >> >> >> that is more or less equivalent to implementing kernel-level suspend
> >> >> >> blockers. �(The suspend blocker approach is slightly more efficient,
> >> >> >> because it will prevent a suspend from starting if a queue is
> >> >> >> non-empty, instead of allowing the suspend to start and then aborting
> >> >> >> it partway through.)
> >> >> >>
> >> >> >> Maybe I'm missing something here... �No doubt someone will point it out
> >> >> >> if I am.
> >> >> >>
> >> >> >
> >> >> > Well, from my perspective that would limit changes to the evdev driver
> >> >> > (well, limited input core plumbing will be needed) but that is using the
> >> >> > current PM infrastructure. The HW driver changes will be limited to what
> >> >> > you described "type 2" in your other e-mail.
> >> >> >
> >> >> > Also, not suspending while events are in progress) is probably
> >> >> > beneficial for platforms other than Android as well. So unless I am
> >> >> > missing something this sounds like a win.
> >> >> >
> >> >>
> >> >> How would this limit the changes you need in the evdev driver? It need
> >> >> to block suspend when there are unprocessed events in some queues.
> >> >> Suspend blockers gives you an api to do this, without it, you check
> >> >> the queues in your suspend hook and abort suspend if they are not
> >> >> empty. Without suspend blockers you have no api to signal that it is
> >> >> OK to suspend again, so you are forcing the thread that tried to
> >> >> suspend to poll until you stop aborting suspend.
> >> >
> >> > No, you do not need to poll. You just set a timeout (short or long,
> >> > depending on your needs) and if no userspace task blocked suspend
> >> > durng that time you attempt to initiate suspend from your manager
> >> > process. If it succeeds - good, if not that means that more events came
> >> > your way and you have to do it later.
> >> >
> >>
> >> How is that not polling? If the user is holding down a key, the keypad
> >> driver has to block suspend, and user space will try to suspend again
> >> and again and again...
> >>
> >
> > If your userpsace is that stupid - sure. However, you can:
> >
> > 1. Notify the suspend manager process that he rest of your userspace is
> > busy handling keystrokes so that it does not try to suspend while there
> > are events pending.
>
> You are missing the point. There are no event pending. The kernel
> reported the key down event, it was handled, but the keypad driver is
> still scanning to see if the user presses another key,

Employ reasonable timeout.

> or releases the
> currently held key.
>

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

> >
> > 2. Wait a tiny bit after last application notified you that it finished
> > processing events.
> >
> > So basically the difference is that with in-kernel suspend blockers,
> > there is a tiny window where we haven't started the suspend yet but are
> > about to the driver has a chance to prevent entire system from starting
> > sleep.
>
> No, the difference is that if a driver needs to prevent suspend for an
> extended period of time, you don't have user space continuously
> polling to see if it can suspend.

Why would a driver, on its own, prevent suspend for extended periods of
time? I think that the decision should originate from userspace, kernel
is here just to serve the requests.

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

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