2010/6/1 <linux-pm-request@xxxxxxxxxxxxxxxxxxxxxxxxxx>
Send linux-pm mailing list submissions to
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
or, via email, send a message with subject or body 'help' to
linux-pm-request@xxxxxxxxxxxxxxxxxxxxxxxxxx
You can reach the person managing the list at
linux-pm-owner@xxxxxxxxxxxxxxxxxxxxxxxxxx
When replying, please edit your Subject line so it is more specific
than "Re: Contents of linux-pm digest..."
Today's Topics:
1. Re: [PATCH 1/2] suspend: Move NVS save/restore code to
generic suspend functionality (Rafael J. Wysocki)
2. Re: [PATCH 0/8] Suspend block api (version 8) (Neil Brown)
3. Re: [PATCH 0/8] Suspend block api (version 8) (Arve Hj?nnev?g)
4. Re: [PATCH 0/8] Suspend block api (version 8) (mark gross)
5. Re: [PATCH 0/8] Suspend block api (version 8) (mark gross)
----------------------------------------------------------------------
Message: 1
Date: Wed, 2 Jun 2010 01:34:46 +0200
From: "Rafael J. Wysocki" <rjw@xxxxxxx>
Subject: Re: [PATCH 1/2] suspend: Move NVS save/restore
code to generic suspend functionality
To: Maxim Levitsky <maximlevitsky@xxxxxxxxx>
Cc: "linux-acpi@xxxxxxxxxxxxxxx" <linux-acpi@xxxxxxxxxxxxxxx>,
linux-pm <linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx>, Matthew Garrett
<mjg@xxxxxxxxxx>
Message-ID: <201006020134.46962.rjw@xxxxxxx>
Content-Type: Text/Plain; charset="utf-8"
On Wednesday 02 June 2010, Maxim Levitsky wrote:
> On Wed, 2010-06-02 at 00:40 +0200, Rafael J. Wysocki wrote:
> > On Tuesday 01 June 2010, Maxim Levitsky wrote:
> > > On Tue, 2010-06-01 at 23:31 +0200, Rafael J. Wysocki wrote:
> > > > On Tuesday 01 June 2010, Maxim Levitsky wrote:
> > > > > Saving platform non-volatile state may be required for suspend to RAM as
> > > > > well as hibernation. Move it to more generic code.
> > > > >
> > > > > Signed-off-by: Matthew Garrett <mjg@xxxxxxxxxx>
> > > > > Signed-off-by: Matthew Garrett <maximlevitsky@xxxxxxxxx>
> > > >
> > > > You made a mistake here.
> > > >
> > > > Also, why are you resending the Matthews patches? I think Len has seen them
> > > > already.
> > > Yea, a copypaste.
> > >
> > > (I was told that if one submits modified patch, it adds his
> > > Signed-off-by.)
> > >
> > > I rebased these on top of
> > > ACPI / EC / PM: Fix race between EC transactions and system suspend'
> > >
> > > To be honest, I just want to get some feedback on this.
> > > This was major issue that kept me from using otherwise prefect suspend
> > > to ram.
> >
> > I think this is a change we should try, but there is a chance it will break
> > some systems.
> I doubt that. BIOSes are tested against windows, and since it has this
> behavior, all systems this patch could break wouldn't work in windows
> ether.
Still, some systems were broken by the saving-restoring NVS over hibernation.
> I was doing all kinds of attempts to fix this bug, and finally I found
> the cause for it.
I know and I'm not objecting to the patches. I'm only saying we should be
careful with them.
Rafael
------------------------------
Message: 2
Date: Wed, 2 Jun 2010 10:43:18 +1000
From: Neil Brown <neilb@xxxxxxx>
Subject: Re: [PATCH 0/8] Suspend block api (version 8)
To: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>, LKML
<linux-kernel@xxxxxxxxxxxxxxx>, Florian Mickler <florian@xxxxxxxxxxx>,
Linux PM <linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx>, Thomas Gleixner
<tglx@xxxxxxxxxxxxx>, Linux OMAP Mailing List
<linux-omap@xxxxxxxxxxxxxxx>, Felipe Balbi <felipe.balbi@xxxxxxxxx>,
Alan Cox <alan@xxxxxxxxxxxxxxxxxxx>
Message-ID: <20100602104318.0c41b3cd@xxxxxxxxxxxxxx>
Content-Type: text/plain; charset=US-ASCII
On Tue, 1 Jun 2010 10:47:49 -0400 (EDT)
Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, 1 Jun 2010, Neil Brown wrote:
>
> > > With wakeup events the problem isn't so bad. Wakeup events are always
> > > noticed, and if the system is designed properly they will either abort
> > > a suspend-in-progress or else cause the system to resume as soon as the
> > > suspend is complete. (Linux is not yet properly designed in this
> > > sense.)
> >
> > This is exactly the issue. Linux doesn't get this right. A wakeup that is
> > processed by the driver just before suspend may not get all the way out to
> > it's final destination (possibly in meat-space) until after some other event
> > wakes the device. It is not possible to guarantee full delivery of a wakeup
> > event with Linux in its current state. This is a bug.
>
> That wasn't what I meant. An important requirement is that a wakeup
> event received after the device has been suspended, but before the
> system suspend is complete, should cause the system suspend to abort.
> This is one of the things suspend blockers were meant to help with. In
> theory it should already work correctly -- but it doesn't. That's a
> real bug.
>
> The other things you're talking about aren't really bugs. That is,
> they are things which the kernel _ought_ to do but it wasn't _intended_
> to do. They are misfeatures: failures of design, not failures of
> execution.
A fairly subtle distinction, but I can accept it. We are working with a
design failure.
But when a design failure results in a feature not being usable in a
race-free way, I would still call that failure of design a bug - a bug in the
design.
>
> > I don't think I'm missing that. Wakeup events are the only events of
> > interest. But wakeup events are more than just low-level hardware events.
> > They are also the virtual events that are a direct consequence of the
> > original hardware event.
>
> This is a matter of semantics and it is highly debatable.
>
> > A UART asserts 'data ready' which is routed to an IO interrupt line that
> > wakes the device. This is a wakeup event.
>
> Yes.
>
> > So are the bits that appear on the data line which represent the data
> > So are the characters that appear on the serial port
> > So is the "here is a message from the GSM processor" event
> > So is the "there is a new TXT message" event
> > So is the "'beep beep beep' out the speaker" event
>
> None of those things can cause the system to wake up after it is
> suspended. Indeed, they can't happen at all if the CPU isn't running
> (unless the GSM processor runs autonomously -- let's ignore such
> details). Hence it makes little sense to call them "wakeup" events.
>
I think we are seeing this very differently. In my mind these are in one
sense all the same event, just represented in different ways.
One of the reasons that we have an OS is to support levels of abstraction.
A disk drives stores sectors, but I want to store files
A monitor displays pixels, but I want to display text
A network sends packets, but I want to send Email.
The OS (including but not only the kernel) handles the conversion.
It is still my Email that is going out over the network even though the
network card has no concept of 'email'.
Similarly the event of importance above in the notification that a TXT has
arrived.
At the lowest level, it appears as 'data-ready' on a UART. At the highest
level is it sound-waves in the air. But it is the same event. And the role
of the OS is to allow me to treat it all as one event - one abstract concept.
Obviously the involvement of the kernel ends when the event reaches or
crosses the boundary to user-space, and user-space must be responsible for
taking it to the boundary to meat-space.
But the kernel needs to ensure that this holistic wake-up event can be
handled without racing with a suspend request.
If you just protect the lowest level representation of the event, but don't
allow the higher level representations to be protected, then it is a bit like
a filesystem that requires you to send SCSI commands to the disk drive if you
want to be sure your data is safe.
> > All these events are wakeup events, and should abort or instant-resume the
> > system. Currently only the first can be guaranteed to do this (... and maybe
> > the second, depending on the details of the implementation).
> > suspend-blocks effectively pass a lock from one event to the next to ensure
> > that all of these event block the suspend. There are various other ways to
> > achieve the same thing, and I think it can be done with no significant change
> > to the API. But some how we need to allow all of these events to be
> > linked wake-up events, not just the first one.
>
> That's not entirely clear. Yes, for Android's use case that's what you
> want. But in other situations maybe you don't. Consider the example
> of someone who closes the lid of their laptop and throws it in a
> backpack. The computer should suspend immediately; it shouldn't be
> delayed by these "virtual wakeup" events.
Completely agree. Old behaviour must remain unchanged, new behaviour must be
explicitly requested.
At each level, it must be explicitly stated that an event is to be treated
like a wake-up event. I believe that is already the case in drivers - they
need to explicitly configure the interrupt to cause a wake-from-suspend.
I've considered several possibilities for how user-space could explicitly say
that events are wake-event. My current favourite is to use fcntl(F_SETOWN)
to request a signal be sent to the process that is initiating the suspend
when the relevant events are ready to be consumed by user-space.
This would work for events that come through the input system (button press)
or through the network, or rtc alarm events. I suspect all potential
wake-events could be made to use this scheme, but I don't have an exhausting
list at present.
So a suspend request waits just long enough to close any race, checks if
there are signals pending, and proceeds with the suspend if there aren't.
The "just long enough" is a little tricky to measure, though the in-kernel
part of suspend-blockers presumably can be used to get that right.
If the suspending process hasn't requested the delivery of these signals, the
suspend will possibly pause to make sure that any events-in-transit in the
kernel have reached their in-kernel destination (milliseconds at most) and
then proceed with the suspend as normal.
So yes, there are different use cases and we should support all of them,
both "I shut the lid and my laptop really stays asleep" and "I never miss a
TXT because my phone went to sleep at a bad time". The process that
initiates the suspend has a role in choosing what can wake it up.
Thanks,
NeilBrown
------------------------------
Message: 3
Date: Tue, 1 Jun 2010 18:10:42 -0700
From: Arve Hj?nnev?g <arve@xxxxxxxxxxx>
Subject: Re: [PATCH 0/8] Suspend block api (version 8)
To: James Bottomley <James.Bottomley@xxxxxxx>
Cc: Neil Brown <neilb@xxxxxxx>, tytso@xxxxxxx, Peter Zijlstra
<peterz@xxxxxxxxxxxxx>, LKML <linux-kernel@xxxxxxxxxxxxxxx>, Florian
Mickler <florian@xxxxxxxxxxx>, Alan Cox <alan@xxxxxxxxxxxxxxxxxxx>,
mark.gross@xxxxxxxxx, Thomas Gleixner <tglx@xxxxxxxxxxxxx>, Linux OMAP
Mailing List <linux-omap@xxxxxxxxxxxxxxx>, Linux PM
<linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx>, felipe.balbi@xxxxxxxxx
Message-ID:
<AANLkTikObnAZbyOxeYAhXcEbimO3XTTO-HlC9CD_HH6t@xxxxxxxxxxxxxx>
Content-Type: text/plain; charset=ISO-8859-1
On Tue, Jun 1, 2010 at 3:36 PM, James Bottomley <James.Bottomley@xxxxxxx> wrote:
> On Wed, 2010-06-02 at 00:24 +0200, Rafael J. Wysocki wrote:
>> On Tuesday 01 June 2010, James Bottomley wrote:
>> > On Tue, 2010-06-01 at 14:51 +0100, Matthew Garrett wrote:
>> > > On Mon, May 31, 2010 at 04:21:09PM -0500, James Bottomley wrote:
>> > >
>> > > > You're the one mentioning x86, not me. ?I already explained that some
>> > > > MSM hardware (the G1 for example) has lower power consumption in S3
>> > > > (which I'm using as an ACPI shorthand for suspend to ram) than any
>> > > > suspend from idle C state. ?The fact that current x86 hardware has the
>> > > > same problem may be true, but it's not entirely relevant.
>> > >
>> > > As long as you can set a wakeup timer, an S state is just a C state with
>> > > side effects. The significant one is that entering an S state stops the
>> > > process scheduler and any in-kernel timers. I don't think Google care at
>> > > all about whether suspend is entered through an explicit transition or
>> > > something hooked into cpuidle - the relevant issue is that they want to
>> > > be able to express a set of constraints that lets them control whether
>> > > or not the scheduler keeps on scheduling, and which doesn't let them
>> > > lose wakeup events in the process.
>> >
>> > Exactly, so my understanding of where we currently are is:
>>
>> Thanks for the recap.
>>
>> > ? ? ?1. pm_qos will be updated to be able to express the android suspend
>> > ? ? ? ? blockers as interactivity constraints (exact name TBD, but
>> > ? ? ? ? probably /dev/cpu_interactivity)
>>
>> I think that's not been decided yet precisely enough. ?I saw a few ideas
>> here and there in the thread, but which of them are we going to follow?
>
> Well, android only needs two states (block and don't block), so that
> gets translated as 2 s32 values (say 0 and INT_MAX). ?I've seen defines
> like QOS_INTERACTIVE and QOS_NONE (or QOS_DRECKLY or QOS_MANANA) to
> describe these, but if all we're arguing over is the define name, that's
> progress.
I think we need separate state constraints for suspend and idle low
power modes. On the msm platform only a subset of the interrupts can
wake up from the low power mode, so we block the use if the low power
mode from idle while other interrupts are enabled. We do not block
suspend however if those interrupts are not marked as wakeup
interrupts. Most constraints that prevent suspend are not hardware
specific and should not prevent entering low power modes from idle. In
other words we may need to prevent low power idle modes while allowing
suspend, and we may need to prevent suspend while allowing low power
idle modes.
It would also be good to not have an implementation that gets slower
and slower the more clients you have. With binary constraints this is
trivial.
>
> The other piece they need is the suspend block name, which comes with
> the stats API, and finally we need to decide what the actual constraint
> is called (which is how the dev node gets its name) ...
>
>> > ? ? ?2. pm_qos will be updated to be callable from atomic context
>> > ? ? ?3. pm_qos will be updated to export statistics initially closely
>> > ? ? ? ? matching what suspend blockers provides (simple update of the rw
>> > ? ? ? ? interface?)
4. It would be useful to change pm_qos_add_request to not allocate
anything so can add constraints from init functions that currently
cannot fail.
>> >
>> > After this is done, the current android suspend block patch becomes a
>> > re-_expression_ in kernel space in terms of pm_qos, with the current
>> > userspace wakelocks being adapted by the android framework into pm_qos
>> > requirements expressed to /dev/cpu_interactivity (or whatever name is
>> > chosen). ?Then opportunistic suspend is either a small add-on kernel
>> > patch they have in their tree to suspend when the interactivity
>> > constraint goes to NONE, or it could be done entirely by a userspace
>> > process. ?Long term this could migrate to the freezer and suspend from
>> > idle approach as the various problem timers get fixed.
>> >
>> > I think the big unresolved issue is the stats extension. ?For android,
>> > we need just a name written along with the value, so we have something
>> > to hang the stats off ... current pm_qos userspace users just write a
>> > value, so the name would be optional. ?From the kernel, we probably just
>> > need an additional API that takes a stats name or NULL if none
>> > (pm_qos_add_request_named()?). ?Then reading the stats could be done by
>> > implementing a fops read routine on the misc device.
>>
>> Is the original idea of having that information in debugfs objectionable?
>
> Well ... debugfs is usually used to get around the sysfs rules. ?In this
> case, pm_qos has a dev interface ... I don't specifically object to
> using debugfs, but I don't see any reason to forbid it from being a
> simple dev read interface either.
>
We don't currently have a dev interface for stats so this is not an
immediate requirement. The suspend blocker debugfs interface is just
as good as the proc interface we have for wakelocks.
--
Arve Hj?nnev?g
------------------------------
Message: 4
Date: Tue, 1 Jun 2010 19:45:38 -0700
From: mark gross <640e9920@xxxxxxxxx>
Subject: Re: [PATCH 0/8] Suspend block api (version 8)
To: James Bottomley <James.Bottomley@xxxxxxx>
Cc: tytso@xxxxxxx, Peter Zijlstra <peterz@xxxxxxxxxxxxx>,
felipe.balbi@xxxxxxxxx, LKML <linux-kernel@xxxxxxxxxxxxxxx>, Florian
Mickler <florian@xxxxxxxxxxx>, Thomas Gleixner <tglx@xxxxxxxxxxxxx>,
Linux OMAP Mailing List <linux-omap@xxxxxxxxxxxxxxx>, Linux PM
<linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx>, Alan Cox
<alan@xxxxxxxxxxxxxxxxxxx>
Message-ID: <20100602024538.GA5764@xxxxxxxx>
Content-Type: text/plain; charset=us-ascii
On Tue, Jun 01, 2010 at 04:01:25PM -0500, James Bottomley wrote:
> On Tue, 2010-06-01 at 14:51 +0100, Matthew Garrett wrote:
> > On Mon, May 31, 2010 at 04:21:09PM -0500, James Bottomley wrote:
> >
> > > You're the one mentioning x86, not me. I already explained that some
> > > MSM hardware (the G1 for example) has lower power consumption in S3
> > > (which I'm using as an ACPI shorthand for suspend to ram) than any
> > > suspend from idle C state. The fact that current x86 hardware has the
> > > same problem may be true, but it's not entirely relevant.
> >
> > As long as you can set a wakeup timer, an S state is just a C state with
> > side effects. The significant one is that entering an S state stops the
> > process scheduler and any in-kernel timers. I don't think Google care at
> > all about whether suspend is entered through an explicit transition or
> > something hooked into cpuidle - the relevant issue is that they want to
> > be able to express a set of constraints that lets them control whether
> > or not the scheduler keeps on scheduling, and which doesn't let them
> > lose wakeup events in the process.
>
> Exactly, so my understanding of where we currently are is:
>
> 1. pm_qos will be updated to be able to express the android suspend
> blockers as interactivity constraints (exact name TBD, but
> probably /dev/cpu_interactivity)
> 2. pm_qos will be updated to be callable from atomic context
> 3. pm_qos will be updated to export statistics initially closely
> matching what suspend blockers provides (simple update of the rw
> interface?)
>
> After this is done, the current android suspend block patch becomes a
> re-_expression_ in kernel space in terms of pm_qos, with the current
> userspace wakelocks being adapted by the android framework into pm_qos
> requirements expressed to /dev/cpu_interactivity (or whatever name is
> chosen). Then opportunistic suspend is either a small add-on kernel
> patch they have in their tree to suspend when the interactivity
> constraint goes to NONE, or it could be done entirely by a userspace
> process. Long term this could migrate to the freezer and suspend from
> idle approach as the various problem timers get fixed.
This is all nice but, all this does is implement the exact same thing as
the wake lock / suspend blocker API as a pm_qos request-class. It
leaves the overlapping constraint issue from ISR to user mode in place
depending on exactly how the oppertunistic suspend is implemented.
I expect it will be via a notifier on the pm_qos request-class update
that would do exactly what the wake lock code does today. just load up
an a "suspend_on_non_interactivity" driver that registers for the call
back, have it enabled by the user mode PM, and you have the equivelent
architecture as what was proposed by the wake lock patches.
it gives the Android guys what they want, without adding a new
subsystem, minimizing the changes and makes most of the architecture
much more politicaly acceptible.
But doesn't it have the same issues with getting the overlapping
constraints right from wake up source to user mode and dealing with the
wake up envents in a sane way? Instead of sprinkling suspend-blockers
about the kernel we'll sprinkle pm_qos_requests about. I like getting
more users of pm_qos, but isn't this the same thing?
>
> I think the big unresolved issue is the stats extension. For android,
> we need just a name written along with the value, so we have something
> to hang the stats off ... current pm_qos userspace users just write a
> value, so the name would be optional. From the kernel, we probably just
> need an additional API that takes a stats name or NULL if none
> (pm_qos_add_request_named()?). Then reading the stats could be done by
> implementing a fops read routine on the misc device.
I don't think the status would be a big deal to add.
However; I am really burned out by this discussion. I am willing to
stub this out ASAP if it puts this behind us if the principles in the
discussion are in more or less agreement.
--mgross
For the record, I still like my low power event idea, which could
coexist with the above.
------------------------------
Message: 5
Date: Tue, 1 Jun 2010 19:51:23 -0700
From: mark gross <640e9920@xxxxxxxxx>
Subject: Re: [PATCH 0/8] Suspend block api (version 8)
To: Florian Mickler <florian@xxxxxxxxxxx>
Cc: 640e9920@xxxxxxxxx, markgross@xxxxxxxxxxx, Peter Zijlstra
<peterz@xxxxxxxxxxxxx>, Brian Swetland <swetland@xxxxxxxxxx>, Ingo
Molnar <mingo@xxxxxxx>, LKML <linux-kernel@xxxxxxxxxxxxxxx>, Linux PM
<linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx>, Thomas Gleixner
<tglx@xxxxxxxxxxxxx>, Alan Cox <alan@xxxxxxxxxxxxxxxxxxx>
Message-ID: <20100602025123.GB5764@xxxxxxxx>
Content-Type: text/plain; charset=iso-8859-1
On Tue, Jun 01, 2010 at 09:07:37AM +0200, Florian Mickler wrote:
> On Mon, 31 May 2010 16:26:17 -0700
> mark gross <640e9920@xxxxxxxxx> wrote:
>
> > On Mon, May 31, 2010 at 11:38:55PM +0200, Rafael J. Wysocki wrote:
> > > On Monday 31 May 2010, Arve Hj?nnev?g wrote:
> > > > 2010/5/29 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>:
> > > > > On Sat, 29 May 2010, Arve Hj?nnev?g wrote:
> > > > >
> > > > >> > In place of in-kernel suspend blockers, there will be a new type of QoS
> > > > >> > constraint -- call it QOS_EVENTUALLY. It's a very weak constraint,
> > > > >> > compatible with all cpuidle modes in which runnable threads are allowed
> > > > >> > to run (which is all of them), but not compatible with suspend.
> > > > >> >
> > > > >> This sound just like another API rename. It will work, but given that
> > > > >> suspend blockers was the name least objectionable last time around,
> > > > >> I'm not sure what this would solve.
> > > > >
> > > > > It's not just a rename. By changing this into a QoS constraint, we
> > > > > make it more generally useful. Instead of standing on its own, it
> > > > > becomes part of the PM-QOS framework.
> > > > >
> > > >
> > > > We cannot use the existing pm-qos framework. It is not safe to call
> > > > from atomic context.
> > >
> > > We've just merged a patch that fixed that if I'm not mistaken. Mark, did your
> > > PM QoS update fix that?
> > >
> >
> > I'm pretty sure it can be called in atomic context, and if its not I'm
> > sure we can fix that. It can be called in atomic context. I don't
> > think it was ever a problem to call it in atomic context. The problem it
> > had was that crappy list of string compares. Thats been fixed.
> >
> > --mgross
> >
>
> Well, the register call uses kzalloc. Apart from that I
> think we're good.
registering shouldn't need to be called in atomic context. Its the
update_request that needs to be callible form atomic context.
--mgross
------------------------------
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
End of linux-pm Digest, Vol 69, Issue 9
***************************************
_______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
