From: Florian Mickler on
On Wed, 02 Jun 2010 15:41:11 -0500
James Bottomley <James.Bottomley(a)suse.de> wrote:

> On Wed, 2010-06-02 at 21:47 +0200, Florian Mickler wrote:
> > On Wed, 02 Jun 2010 10:05:11 -0500
> > James Bottomley <James.Bottomley(a)suse.de> wrote:
> >
> > > On Tue, 2010-06-01 at 21:41 -0700, Arve Hj�nnev�g wrote:
> > > > 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.
> > >
> > > Depends. If you block the system from going into low power idle, does
> > > that mean you still want it to be fully suspended?
> > >
> > > If yes, then we do have independent constraints. If not, they have a
> > > hierarchy:
> > >
> > > * Fully Interactive (no low power idle or suspend)
> > > * Partially Interactive (may go into low power idle but not
> > > suspend)
> > > * None (may go into low power idle or suspend)
> > >
> > > Which is expressable as a ternary constraint.
> > >
> > > James
> > >
> >
> > But unblocking suspend at the moment is independent to getting idle.
> > If you have the requirement to stay in the highest-idle level (i.e.
> > best latency you can get) that does not (currently) mean, that you can
> > not suspend.
>
> I don't understand that as a reason. If we looks at this a qos
> constraints, you're saying that the system may not drop into certain low
> power states because it might turn something off that's currently being
> used by a driver or a process. Suspend is certainly the lowest state of
> that because it turns everything off, why would it be legal to drop into
> that?
>
> I also couldn't find this notion of separation of idleness power from
> suspend blocking in the original suspend block patch set ... if you can
> either tell me where it is, or give me an example of the separated use
> cases, I'd understand better.
>
> > To preserve that explicit fall-through while still having working
> > run-time-powermanagement I think the qos-constraints need to be
> > separated.
>
> OK, as above, why? or better yet, just give an example.

Hm. Maybe it is me who doesn't understand.

With proposed patchset:
1. As soon as we unblock suspend we go down. (i.e. suspending)
2. While suspend is blocked, the idle-loop does it's things. (i.e.
runtime power managment -> can give same power-result as suspend)

possible cases:
1:
- qos-latency-constraints: 1ms, [here: forbids anything other than
C1 idle state.]
- suspend is blocked

2: - qos latency-constraints: as in 1
- suspend unblocked

3: - qos latency-constraints: infinity, cpu in lowest power state.
- suspend is blocked

4: - qos latency-constraints: infinity, cpu in lowest power state.
- suspend unblocked


in case 2 and 4 we would suspend, regardeless of the qos-latency.

in case 1 and 3 we would stay awake, regardeless of the qos-latency
constraint.


If only one constraint, then case 2 (or 3) wouldn't be possible. But it
is possible now.

A possible use case as an example?
(hmm... i'm trying my imagination hard now):
Your sound needs low latency, so that could be a cause for the
qos-latency constraint.

And unblocking suspend could nonetheless happen:
For example... you have an firefox open and don't want to
prevent suspend for that case when the display is turned off


Cheers,
Flo

--
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: James Bottomley on
On Wed, 2010-06-02 at 15:27 -0700, Arve Hjønnevåg wrote:
> On Wed, Jun 2, 2010 at 1:41 PM, James Bottomley <James.Bottomley(a)suse.de> wrote:
> > On Wed, 2010-06-02 at 21:47 +0200, Florian Mickler wrote:
> >> On Wed, 02 Jun 2010 10:05:11 -0500
> >> James Bottomley <James.Bottomley(a)suse.de> wrote:
> >>
> >> > On Tue, 2010-06-01 at 21:41 -0700, Arve Hjønnevåg wrote:
> >> > > 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.
> >> >
> >> > Depends. If you block the system from going into low power idle, does
> >> > that mean you still want it to be fully suspended?
> >> >
> >> > If yes, then we do have independent constraints. If not, they have a
> >> > hierarchy:
> >> >
> >> > * Fully Interactive (no low power idle or suspend)
> >> > * Partially Interactive (may go into low power idle but not
> >> > suspend)
> >> > * None (may go into low power idle or suspend)
> >> >
> >> > Which is expressable as a ternary constraint.
> >> >
> >> > James
> >> >
> >>
> >> But unblocking suspend at the moment is independent to getting idle.
> >> If you have the requirement to stay in the highest-idle level (i.e.
> >> best latency you can get) that does not (currently) mean, that you can
> >> not suspend.
> >
> > I don't understand that as a reason. If we looks at this a qos
> > constraints, you're saying that the system may not drop into certain low
> > power states because it might turn something off that's currently being
> > used by a driver or a process. Suspend is certainly the lowest state of
> > that because it turns everything off, why would it be legal to drop into
> > that?
>
> Because the driver gets called on suspend which gives it a change to
> stop using it.
>
> >
> > I also couldn't find this notion of separation of idleness power from
> > suspend blocking in the original suspend block patch set ... if you can
> > either tell me where it is, or give me an example of the separated use
> > cases, I'd understand better.
> >
>
> The suspend block patchset only deals with suspend, not low power idle
> modes. The original wakelock patchset had two wakelock types, idle and
> suspend.

OK, so that explains why I didn't see it ...

> >> To preserve that explicit fall-through while still having working
> >> run-time-powermanagement I think the qos-constraints need to be
> >> separated.
> >
> > OK, as above, why? or better yet, just give an example.
> >
>
> The i2c bus on the Nexus One is used by the other core to turn off the
> power you our core when we enter the lowest power mode. This means
> that we cannot enter that low power mode while the i2c bus is active,
> so we block low power idle modes. At some point we also tries to block
> suspend in this case, but this caused a lot of failed suspend attempts
> since the frequency scaling code would try to ramp up while freezing
> processes which in turn aborted the process freezing attempts.

OK, so this is a device specific power constraint state. I suppose it
makes sense to have a bunch of those, because the device isn't
necessarily going to know what idle power mode it can't go into, so the
cpu govenor should sort it out rather than have the device specify a
minimum state.

James


--
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: Gross, Mark on


>-----Original Message-----
>From: Florian Mickler [mailto:florian(a)mickler.org]
>Sent: Wednesday, June 02, 2010 4:06 PM
>To: James Bottomley
>Cc: Arve Hj�nnev�g; Neil Brown; tytso(a)mit.edu; Peter Zijlstra; LKML; Alan
>Cox; Gross, Mark; Thomas Gleixner; Linux OMAP Mailing List; Linux PM;
>felipe.balbi(a)nokia.com
>Subject: Re: [linux-pm] [PATCH 0/8] Suspend block api (version 8)
>
>On Wed, 02 Jun 2010 15:41:11 -0500
>James Bottomley <James.Bottomley(a)suse.de> wrote:
>
>> On Wed, 2010-06-02 at 21:47 +0200, Florian Mickler wrote:
>> > On Wed, 02 Jun 2010 10:05:11 -0500
>> > James Bottomley <James.Bottomley(a)suse.de> wrote:
>> >
>> > > On Tue, 2010-06-01 at 21:41 -0700, Arve Hj�nnev�g wrote:
>> > > > 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.
>> > >
>> > > Depends. If you block the system from going into low power idle,
>does
>> > > that mean you still want it to be fully suspended?
>> > >
>> > > If yes, then we do have independent constraints. If not, they have a
>> > > hierarchy:
>> > >
>> > > * Fully Interactive (no low power idle or suspend)
>> > > * Partially Interactive (may go into low power idle but not
>> > > suspend)
>> > > * None (may go into low power idle or suspend)
>> > >
>> > > Which is expressable as a ternary constraint.
>> > >
>> > > James
>> > >
>> >
>> > But unblocking suspend at the moment is independent to getting idle.
>> > If you have the requirement to stay in the highest-idle level (i.e.
>> > best latency you can get) that does not (currently) mean, that you can
>> > not suspend.
>>
>> I don't understand that as a reason. If we looks at this a qos
>> constraints, you're saying that the system may not drop into certain low
>> power states because it might turn something off that's currently being
>> used by a driver or a process. Suspend is certainly the lowest state of
>> that because it turns everything off, why would it be legal to drop into
>> that?
>>
>> I also couldn't find this notion of separation of idleness power from
>> suspend blocking in the original suspend block patch set ... if you can
>> either tell me where it is, or give me an example of the separated use
>> cases, I'd understand better.
>>
>> > To preserve that explicit fall-through while still having working
>> > run-time-powermanagement I think the qos-constraints need to be
>> > separated.
>>
>> OK, as above, why? or better yet, just give an example.
>
>Hm. Maybe it is me who doesn't understand.
>
>With proposed patchset:
>1. As soon as we unblock suspend we go down. (i.e. suspending)
>2. While suspend is blocked, the idle-loop does it's things. (i.e.
>runtime power managment -> can give same power-result as suspend)
>
>possible cases:
>1:
> - qos-latency-constraints: 1ms, [here: forbids anything other than
> C1 idle state.]
> - suspend is blocked
>
>2: - qos latency-constraints: as in 1
> - suspend unblocked
>
>3: - qos latency-constraints: infinity, cpu in lowest power state.
> - suspend is blocked
>
>4: - qos latency-constraints: infinity, cpu in lowest power state.
> - suspend unblocked
>
>
>in case 2 and 4 we would suspend, regardeless of the qos-latency.
>
>in case 1 and 3 we would stay awake, regardeless of the qos-latency
>constraint.
>
>
>If only one constraint, then case 2 (or 3) wouldn't be possible. But it
>is possible now.
>
>A possible use case as an example?
>(hmm... i'm trying my imagination hard now):
> Your sound needs low latency, so that could be a cause for the
> qos-latency constraint.
>
> And unblocking suspend could nonetheless happen:
> For example... you have an firefox open and don't want to
> prevent suspend for that case when the display is turned off
>
>
[mtg: ] This has been a pain point for the PM_QOS implementation. They change the constrain back and forth at the transaction level of the i2c driver. The pm_qos code really wasn't made to deal with such hot path use, as each such change triggers a re-computation of what the aggregate qos request is.

We've had a number of attempts at fixing this, but I think the proper fix is to bolt a "disable C-states > x" interface into cpu_idle that bypases pm_qos altogether. Or, perhaps add a new pm_qos API that does the equivalent operation, overriding whatever constraint is active.

--mgross


--
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: mark gross on
On Thu, Jun 03, 2010 at 12:03:49AM +0200, Rafael J. Wysocki wrote:
> On Wednesday 02 June 2010, mark gross wrote:
> > On Tue, Jun 01, 2010 at 08:50:02PM -0700, Arve Hj�nnev�g wrote:
> > > On Tue, Jun 1, 2010 at 7:05 AM, mark gross <640e9920(a)gmail.com> wrote:
> > > > On Tue, Jun 01, 2010 at 09:07:37AM +0200, Florian Mickler wrote:
> > > ...
> > > >> +static void update_target_val(int pm_qos_class, s32 val)
> > > >> +{
> > > >> + s32 extreme_value;
> > > >> + s32 new_value;
> > > >> + extreme_value = atomic_read(&pm_qos_array[pm_qos_class]->target_value);
> > > >> + new_value = pm_qos_array[pm_qos_class]->comparitor(val,extreme_value);
> > > >> + if (extreme_value != new_value)
> > > >> + atomic_set(&pm_qos_array[pm_qos_class]->target_value,new_value);
> > > >> +}
> > > >> +
> > > >
> > > > Only works 1/2 the time, but I like the idea!
> > > > It fails to get the righ answer when constraints are reduced. But, this
> > > > idea is a good improvement i'll roll into the next pm_qos update!
> > > >
> > >
> > > I think it would be a better idea to track your constraints with a
> > > sorted data structure. That way you can to better than O(n) for both
> > > directions. If you have a lot of constraints with the same value, it
> > > may even be worthwhile to have a two stage structure where for
> > > instance you use a rbtree for the unique values and list for identical
> > > constraints.
> >
> > I don't agree, we went through this tree vrs list discussion a few times
> > before in other areas of the kernel. Wherever the list tended to be
> > short, a simple list wins. However; we can try it, after we have some
> > metrics and stress test cases identified we can measure its effectivenes
> > against.
>
> How many different values are there to handle?
>

for the current pm_qos users its tiny. I've never heard of more than a
few < 10. However; for the new "interactive" class to provide suspend
blocker functionality, I expect the number to be up to 20.

but realistically I bet we never get more than 10ish.

One constraint constraint request per module from isr to user mode.
Once in user mode there would be only a few (assuming Android user
space) I think just from the power HAL, input HAL, and the RIL.

Still a pretty small number I don't think we need to worry about scaling
as much as we need to worry about performance.

--mgross
--
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: mark gross on
On Wed, Jun 02, 2010 at 02:58:30PM -0700, Arve Hj�nnev�g wrote:
> 2010/6/2 mark gross <640e9920(a)gmail.com>:
> > On Tue, Jun 01, 2010 at 08:50:02PM -0700, Arve Hj�nnev�g wrote:
> >> On Tue, Jun 1, 2010 at 7:05 AM, mark gross <640e9920(a)gmail.com> wrote:
> >> > On Tue, Jun 01, 2010 at 09:07:37AM +0200, Florian Mickler wrote:
> >> ...
> >> >> +static void update_target_val(int pm_qos_class, s32 val)
> >> >> +{
> >> >> + � � s32 extreme_value;
> >> >> + � � s32 new_value;
> >> >> + � � extreme_value = atomic_read(&pm_qos_array[pm_qos_class]->target_value);
> >> >> + � � new_value = pm_qos_array[pm_qos_class]->comparitor(val,extreme_value);
> >> >> + � � if (extreme_value != new_value)
> >> >> + � � � � � � atomic_set(&pm_qos_array[pm_qos_class]->target_value,new_value);
> >> >> +}
> >> >> +
> >> >
> >> > Only works 1/2 the time, but I like the idea!
> >> > It fails to get the righ answer when constraints are reduced. �But, this
> >> > idea is a good improvement i'll roll into the next pm_qos update!
> >> >
> >>
> >> I think it would be a better idea to track your constraints with a
> >> sorted data structure. That way you can to better than O(n) for both
> >> directions. If you have a lot of constraints with the same value, it
> >> may even be worthwhile to have a two stage structure where for
> >> instance you use a rbtree for the unique values and list for identical
> >> constraints.
> >
> > I don't agree, we went through this tree vrs list discussion a few times
> > before in other areas of the kernel. �Wherever the list tended to be
> > short, a simple list wins. �However; we can try it, after we have some
> > metrics and stress test cases identified we can measure its effectivenes
> > against.
> >
>
> The list is not short. You have all the inactive and active
> constraints on the same list. If you change it to a two level list
> though, the list of unique values (which is the list you have to walk)
> may be short enough for a tree to be overkill.

what have you seen in practice from the wake-lock stats?

I'm having a hard time seeing where you could get more than just a
handfull. However; one could go to a dual list (like the scheduler) and
move inactive nodes from an active to inactive list, or we could simply
remove them from the list uppon inactivity. which would would well
after I change the api to have the client allocate the memory for the
nodes... BUT, if your moving things in and out of a list a lot, I'm not
sure the break even point where changing the structure helps.

We'll need to try it.

I think we will almost never see more than 10 list elements.

--mgross

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