Date:	Tue, 1 Jun 2010 21:41:25 -0700
From:	Arve Hjønnevåg <arve@...roid.com>
To:	James Bottomley <James.Bottomley@...e.de>
Cc:	Neil Brown <neilb@...e.de>, tytso@....edu,
	Peter Zijlstra <peterz@...radead.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Florian Mickler <florian@...kler.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>, mark.gross@...el.com,
	Thomas Gleixner <tglx@...utronix.de>,
	Linux OMAP Mailing List <linux-omap@...r.kernel.org>,
	Linux PM <linux-pm@...ts.linux-foundation.org>,
	felipe.balbi@...ia.com
Subject: Re: [linux-pm] [PATCH 0/8] Suspend block api (version 8)

2010/6/1 James Bottomley <James.Bottomley@...e.de>:
> On Tue, 2010-06-01 at 18:10 -0700, Arve Hjønnevåg wrote:
>> On Tue, Jun 1, 2010 at 3:36 PM, James Bottomley <James.Bottomley@...e.de> 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.
>
> Well, as I said, pm_qos is s32 ... it's easy to make the constraint
> ternary instead of binary.

No, they have to be two separate constraints, otherwise a constraint
to block suspend would override a constraint to block a low power idle
mode or the other way around.

>
>> 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.
>
> Well, that's an implementation detail ... ordering the list or using a

True. I think we also need timeout support in the short term though
which is also somewhat simpler to implement in an efficient way for
binary constraints.

> btree would significantly fix that.  However, the most number of
> constraint users I've seen in android is around 60 ... that's not huge
> from a kernel linear list perspective, so is this really a concern? ...
> particularly when most uses don't necessarily change the constrain, so a
> list search isn't done.

The search is done every time any of them changes.

>
>> > 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.
>
> Sure .. we do that for the delayed work queues, it's just an API which
> takes the structure as an argument leaving it the responsibility of the
> caller to free.
>
>> >> > 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.
>
> OK, great ... what actually exports the statistics is just an
> implementation detail.
>
> James
>
>
>
>



-- 
Arve Hjønnevåg
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists