linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Arve Hjønnevåg" <arve@android.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>,
	Linux-pm mailing list <linux-pm@lists.linux-foundation.org>,
	Kernel development list <linux-kernel@vger.kernel.org>,
	Tejun Heo <tj@kernel.org>, Oleg Nesterov <oleg@redhat.com>,
	Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
	Randy Dunlap <rdunlap@xenotime.net>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	Nigel Cunningham <nigel@tuxonice.net>,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	Ming Lei <tom.leiming@gmail.com>,
	Wu Fengguang <fengguang.wu@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Maxim Levitsky <maximlevitsky@gmail.com>,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH 1/8] PM: Add suspend block api.
Date: Thu, 29 Apr 2010 16:39:29 -0700	[thread overview]
Message-ID: <p2qd6200be21004291639qd65002a8l4cfbf635962615ba@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1004291124580.1697-100000@iolanthe.rowland.org>

2010/4/29 Alan Stern <stern@rowland.harvard.edu>:
> On Wed, 28 Apr 2010, Arve Hjønnevåg wrote:
>
>> >> >  suspend blockers can be used to allow
>> >> > +user-space to decide whether a keystroke received while the system is suspended
>> >> > +should cause the screen to be turned back on or allow the system to go back into
>> >> > +suspend.
>> >>
>> >> That's not right.  Handling the screen doesn't need suspend blockers:
>> >> The program decides what to do and then either turns on the screen or
>> >> else writes "mem" to /sys/power/state.
>>
>> That does not work though. Unless every key turns the screen on you
>> will have a race every time the user presses a key you want to ignore.
>
> Of course.  You are confirming what I wrote immediately below: Suspend
Yet you offered it as an example of why "Handling the screen doesn't
need suspend blockers".

> blockers help resolve races.  Note that this race has nothing to do
> with the _screen_ in particular -- exactly the same race occurs if you
> decide to turn on the audio speaker or some other piece of hardware.
>
I agree with this, but that does not mean that describing how you can
handle the screen with suspend blockers is a bad example.

>> >>  What suspend blockers add is
>> >> the ability to resolve races and satisfy multiple constraints when
>> >> going into suspend -- which has nothing to do with operating the
>> >> screen.
>>
>> I'm not sure I agree with this. You cannot reliably turn the screen on
>> from user space when the user presses a wakeup-key without suspend
>> blockers.
>
> Let's say that it has nothing to do _specifically_ with the screen.
> _Any_ action you want to take in userspace is difficult to coordinate
> with system suspends if you don't have suspend blockers.
>
>> >>
>> >> I _think_ what you're trying to get at can be expressed this way:
>> >>
>> >>       Here's an example showing how a cell phone or other embedded
>> >>       system can handle keystrokes (or other input events) in the
>> >>       presence of suspend blockers.  Use set_irq_wake...
>>
>> OK, but the last version was what you (Alan) suggested last year.
>
> So at least my mental processes have remained consistent over the span
> of a year.  Nice to know I haven't undergone a complete personality
> change...  :-)
>
>> >>       ...
>> >>
>> >>       - The user-space input-event thread returns from read.  It
>> >>       carries out whatever activities are appropriate (for example,
>> >>       powering up the display screen, running other programs, and so
>> >>       on).  When it is finished, it calls suspend_unblock on the
>> >>       process_input_events suspend_blocker and then calls select or
>> >>       poll.  The system will automatically suspend again when it is
>> >>       idle and no suspend blockers remain active.
>> >
>> > Yeah, that sounds better.  Arve, what do you think?
>> >
>>
>> Idle is irrelevant and needs to be removed. This new last step is also
>> no longer a concrete example, but if you really think is it better I
>> can change it.
>
> Perhaps you would prefer to change this completely.  Write up a
> description of what can go wrong when suspend blockers _aren't_ used,
> and show how suspend blockers can prevent the problem from occurring.
>
> But whatever you do, don't make it appear that suspend blockers allow
> user programs to make decisions (which is what you wrote before).  They
> don't -- programs can make whatever decisions they want.  Suspend
> blockers merely help them carry out the actions they have decided upon
> in a safe manner.

I think suspend blockers do allow user programs to make decisions.
Without suspend blockers some decisions can only be safely be made in
the kernel/drivers.

>
> And don't make it appear that suspend blockers can only be used for
> solving screen-related problems.

How about:

- The user-space input-event thread returns from read. If it
determines that the key should be ignored, it calls suspend_unblock on
the process_input_events suspend_blocker and then calls select or
poll. The system will automatically suspend again, since now no
suspend blockers are active.

If the key that was pressed instead should preform a simple action
(for example, adjusting the volume), this action can be performed
right before calling suspend_unblock on the process_input_events
suspend_blocker. However, if the key triggers a longer-running action,
that action needs its own suspend_blocker and suspend_block must be
called on that suspend blocker before calling suspend_unblock on the
process_input_events suspend_blocker.

-- 
Arve Hjønnevåg

  reply	other threads:[~2010-04-30 18:06 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-28  4:31 [PATCH 0/9] Suspend block api (version 5) Arve Hjønnevåg
2010-04-28  4:31 ` [PATCH 1/8] PM: Add suspend block api Arve Hjønnevåg
2010-04-28  4:31   ` [PATCH 2/8] PM: suspend_block: Add driver to access suspend blockers from user-space Arve Hjønnevåg
2010-04-28  4:31     ` [PATCH 3/8] PM: suspend_block: Abort task freezing if a suspend_blocker is active Arve Hjønnevåg
2010-04-28  4:31       ` [PATCH 4/8] PM: suspend_block: Add debugfs file Arve Hjønnevåg
2010-04-28  4:31         ` [PATCH 5/8] PM: suspend_block: Add suspend_blocker stats Arve Hjønnevåg
2010-04-28  4:31           ` [PATCH 6/8] PM: Add suspend blocking work Arve Hjønnevåg
2010-04-28  4:31             ` [PATCH 7/8] Input: Block suspend while event queue is not empty Arve Hjønnevåg
2010-04-28  4:31               ` [PATCH 8/8] power_supply: Block suspend while power supply change notifications are pending Arve Hjønnevåg
2010-04-28  6:06             ` [PATCH 6/8] PM: Add suspend blocking work Pavel Machek
2010-04-28  6:44             ` Tejun Heo
2010-04-28  7:02               ` Arve Hjønnevåg
2010-04-28  7:18                 ` Tejun Heo
2010-04-28 19:40             ` Oleg Nesterov
2010-04-28 20:22               ` Tejun Heo
2010-04-28 21:08                 ` Rafael J. Wysocki
2010-04-29 18:58                   ` Oleg Nesterov
2010-04-29 19:44                     ` [PATCH 0/2] workqueue fixlets (Was: PM: Add suspend blocking work.) Oleg Nesterov
2010-04-29 19:45                       ` [PATCH 1/2] workqueues: flush_delayed_work: keep the original workqueue for re-queueing Oleg Nesterov
2010-04-30  5:15                         ` Tejun Heo
2010-04-29 19:45                       ` [PATCH 2/2] workqueues: export keventd_wq Oleg Nesterov
2010-04-30  5:16                         ` Tejun Heo
2010-04-30  5:39                           ` Arve Hjønnevåg
2010-04-30  5:52                             ` Tejun Heo
2010-04-30 18:05                               ` Oleg Nesterov
2010-04-30 18:11                                 ` Tejun Heo
2010-04-29 21:08                     ` [PATCH 6/8] PM: Add suspend blocking work Rafael J. Wysocki
2010-04-28 21:09               ` Rafael J. Wysocki
2010-04-28 22:09                 ` Arve Hjønnevåg
2010-04-28 22:19                   ` Rafael J. Wysocki
2010-04-29  3:47                     ` Arve Hjønnevåg
2010-04-29 21:09                       ` Rafael J. Wysocki
2010-04-28  5:07       ` [PATCH 3/8] PM: suspend_block: Abort task freezing if a suspend_blocker is active Pavel Machek
2010-04-28 20:58     ` [PATCH 2/8] PM: suspend_block: Add driver to access suspend blockers from user-space Rafael J. Wysocki
2010-04-28 22:31       ` Arve Hjønnevåg
2010-04-28 23:05         ` Rafael J. Wysocki
2010-04-28 23:38           ` Arve Hjønnevåg
2010-04-29 21:11             ` Rafael J. Wysocki
2010-04-29 23:41               ` Arve Hjønnevåg
2010-04-28  6:07   ` [PATCH 1/8] PM: Add suspend block api Pavel Machek
2010-04-28 19:13   ` Alan Stern
2010-04-28 21:13     ` Rafael J. Wysocki
2010-04-28 23:35       ` Arve Hjønnevåg
2010-04-29 15:41         ` Alan Stern
2010-04-29 23:39           ` Arve Hjønnevåg [this message]
2010-04-30 14:41             ` Alan Stern
2010-04-28 20:50   ` Rafael J. Wysocki
2010-04-29  3:37     ` Arve Hjønnevåg
2010-04-29 21:16       ` Rafael J. Wysocki
2010-04-30  4:24         ` Tejun Heo
2010-04-30 17:26           ` Oleg Nesterov
2010-05-20  8:30             ` Tejun Heo
2010-05-20 22:27               ` Rafael J. Wysocki
2010-05-21  6:35                 ` Tejun Heo
2010-05-06 15:18   ` Alan Stern
2010-05-06 19:28     ` Rafael J. Wysocki
2010-05-06 19:40       ` Alan Stern
2010-05-06 23:48         ` Arve Hjønnevåg
2010-05-07 14:22           ` Alan Stern
  -- strict thread matches above, loose matches on Subject: below --
2010-04-30 22:36 [PATCH 0/8] Suspend block api (version 6) Arve Hjønnevåg
2010-04-30 22:36 ` [PATCH 1/8] PM: Add suspend block api Arve Hjønnevåg
2010-05-02  6:56   ` Pavel Machek
2010-05-02 20:10     ` Rafael J. Wysocki
2010-05-02 20:52       ` Pavel Machek
2010-05-02 21:29         ` Rafael J. Wysocki
2010-05-03 19:01           ` Pavel Machek
2010-05-03 21:38             ` Rafael J. Wysocki
2010-05-03 22:11               ` Alan Stern
2010-05-03 22:24                 ` Arve Hjønnevåg
2010-05-02  7:01   ` Pavel Machek
2010-05-14  4:11 [PATCH 0/8] Suspend block api (version 7) Arve Hjønnevåg
2010-05-14  4:11 ` [PATCH 1/8] PM: Add suspend block api Arve Hjønnevåg
2010-05-18 13:11   ` Pavel Machek
2010-05-20  9:11     ` Florian Mickler
2010-05-20  9:26       ` Florian Mickler
2010-05-20 22:18         ` Rafael J. Wysocki
2010-05-21  6:04           ` Florian Mickler
2010-05-27 15:41           ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=p2qd6200be21004291639qd65002a8l4cfbf635962615ba@mail.gmail.com \
    --to=arve@android.com \
    --cc=akpm@linux-foundation.org \
    --cc=cornelia.huck@de.ibm.com \
    --cc=fengguang.wu@intel.com \
    --cc=jbarnes@virtuousgeek.org \
    --cc=len.brown@intel.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=maximlevitsky@gmail.com \
    --cc=nigel@tuxonice.net \
    --cc=oleg@redhat.com \
    --cc=pavel@ucw.cz \
    --cc=rdunlap@xenotime.net \
    --cc=rjw@sisk.pl \
    --cc=stern@rowland.harvard.edu \
    --cc=tj@kernel.org \
    --cc=tom.leiming@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).