[PATCH 0/8] Suspend block api (version 8)
in [Kernel]
|
Prev: [ANN] Linux Security Summit 2010 - Announcement and CFP
Next: [PATCH 4/8] PM: suspend_block: Add debugfs file
From: Rafael J. Wysocki on 3 Jun 2010 17:10 On Thursday 03 June 2010, mark gross wrote: > 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. In that case sorting the structure is rather not going to improve things, but it might be worth using a hashtable or something similar. Rafael -- 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: Bryan Huntsman on 3 Jun 2010 18:00 >> Yes, having a QoS parameter per-subsystem (or even per-device) is very >> important for SoCs that have independently controlled powerdomains. >> If all devices/subsystems in a particular powerdomain have QoS >> parameters that permit, the power state of that powerdomain can be >> lowered independently from system-wide power state and power states of >> other power domains. >> > This seems similar to that pm_qos generalization into bus drivers we where > waving our hands at during the collab summit in April? We never did get > into meaningful detail at that time. > > --mgross I think there is definitely a need for QoS parameters per-device. I've been pondering how to incorporate this concept into runtime_pm. One idea would be to add pm_qos-like callbacks to struct dev_pm_ops, e.g. runtime_pm_qos_add/update/remove_requirement(). Requirements would be passed up the tree to the first parent that cares, usually a bus driver. Is this similar to what you guys were discussing at the collab summit? Thanks. - Bryan -- 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 4 Jun 2010 00:10 On Thu, Jun 3, 2010 at 2:05 PM, Rafael J. Wysocki <rjw(a)sisk.pl> wrote: > On Thursday 03 June 2010, James Bottomley wrote: >> On Thu, 2010-06-03 at 00:10 -0700, Arve Hj�nnev�g wrote: >> > On Wed, Jun 2, 2010 at 10:40 PM, mark gross <640e9920(a)gmail.com> wrote: >> > > On Wed, Jun 02, 2010 at 09:54:15PM -0700, Brian Swetland wrote: >> > >> On Wed, Jun 2, 2010 at 8:18 PM, mark gross <640e9920(a)gmail.com> wrote: >> > >> > On Wed, Jun 02, 2010 at 02:58:30PM -0700, Arve Hj�nnev�g wrote: >> > >> >> >> > >> >> 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 >> > >> > >> > >> > >> > >> >> > >> I see about 80 (based on the batteryinfo dump) on my Nexus One >> > >> (QSD8250, Android Froyo): >> > > >> > > shucks. >> > > >> > > well I think for a pm_qos class that has boolean dynamic range we can >> > > get away with not walking the list on every request update. �we can use >> > > a counter, and the list will be for mostly for stats. >> > > >> > >> > Did you give any thought to my suggestion to only use one entry per >> > unique value on the first level list and then use secondary lists of >> > identical values. That way if you only have two constraints values the >> > list you have to walk when updating a request will never have more >> > than two entries regardless of how many total request you have. >> > >> > A request update then becomes something like this: >> > � if on primary list { >> > � � unlink from primary list >> > � � if secondary list is not empty >> > � � � get next secondary entry and add in same spot on primary list >> > � } >> > � unlink from secondary list >> > � find new spot on primary list >> > � if already there >> > � � add to secondary list >> > � else >> > � � add to primary list >> >> This is just reinventing hash bucketed lists. �To get the benefits, all >> we do is implement an N state constraint as backed by an N bucketed hash >> list, which the kernel already has all the internal mechanics for. > > Agreed. > No, a hash is used for quick lookup of a specific value, not to find an extreme value. It is however extremely similar to plists. The only difference is that plists link all the secondary lists together. If we want to have constraints that autoexpire, then keeping the secondary lists separate allows the same optimization as I did for wakelock/suspend_blocker timeouts where no timer is active if an (equal or stricter) non-expiring constraint is active. -- 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: Florian Mickler on 4 Jun 2010 09:10 On Thu, 3 Jun 2010 21:07:07 -0700 Arve Hj�nnev�g <arve(a)android.com> wrote: > On Thu, Jun 3, 2010 at 2:05 PM, Rafael J. Wysocki <rjw(a)sisk.pl> wrote: > > On Thursday 03 June 2010, James Bottomley wrote: > >> On Thu, 2010-06-03 at 00:10 -0700, Arve Hj�nnev�g wrote: > >> > A request update then becomes something like this: > >> > � if on primary list { > >> > � � unlink from primary list > >> > � � if secondary list is not empty > >> > � � � get next secondary entry and add in same spot on primary list > >> > � } > >> > � unlink from secondary list > >> > � find new spot on primary list > >> > � if already there > >> > � � add to secondary list > >> > � else > >> > � � add to primary list > >> > >> This is just reinventing hash bucketed lists. �To get the benefits, all > >> we do is implement an N state constraint as backed by an N bucketed hash > >> list, which the kernel already has all the internal mechanics for. > > > > Agreed. > > > > No, a hash is used for quick lookup of a specific value, not to find > an extreme value. It is however extremely similar to plists. The only > difference is that plists link all the secondary lists together. If we > want to have constraints that autoexpire, then keeping the secondary > lists separate allows the same optimization as I did for > wakelock/suspend_blocker timeouts where no timer is active if an > (equal or stricter) non-expiring constraint is active. Can you give an example for the optimization or elaborate about the negative effect of linking the secondary lists together? I don't understand right now. Would be hlist from list.h better? (I think that is what James is referring to?) That is a (single-linked-)list of double-linked-lists. 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 4 Jun 2010 10:30
On Thu, 2010-06-03 at 21:07 -0700, Arve Hjønnevåg wrote: > On Thu, Jun 3, 2010 at 2:05 PM, Rafael J. Wysocki <rjw(a)sisk.pl> wrote: > > On Thursday 03 June 2010, James Bottomley wrote: > >> On Thu, 2010-06-03 at 00:10 -0700, Arve Hjønnevåg wrote: > >> > On Wed, Jun 2, 2010 at 10:40 PM, mark gross <640e9920(a)gmail.com> wrote: > >> > > On Wed, Jun 02, 2010 at 09:54:15PM -0700, Brian Swetland wrote: > >> > >> On Wed, Jun 2, 2010 at 8:18 PM, mark gross <640e9920(a)gmail.com> wrote: > >> > >> > On Wed, Jun 02, 2010 at 02:58:30PM -0700, Arve Hjønnevåg wrote: > >> > >> >> > >> > >> >> 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 > >> > >> > > >> > >> > > >> > >> > >> > >> I see about 80 (based on the batteryinfo dump) on my Nexus One > >> > >> (QSD8250, Android Froyo): > >> > > > >> > > shucks. > >> > > > >> > > well I think for a pm_qos class that has boolean dynamic range we can > >> > > get away with not walking the list on every request update. we can use > >> > > a counter, and the list will be for mostly for stats. > >> > > > >> > > >> > Did you give any thought to my suggestion to only use one entry per > >> > unique value on the first level list and then use secondary lists of > >> > identical values. That way if you only have two constraints values the > >> > list you have to walk when updating a request will never have more > >> > than two entries regardless of how many total request you have. > >> > > >> > A request update then becomes something like this: > >> > if on primary list { > >> > unlink from primary list > >> > if secondary list is not empty > >> > get next secondary entry and add in same spot on primary list > >> > } > >> > unlink from secondary list > >> > find new spot on primary list > >> > if already there > >> > add to secondary list > >> > else > >> > add to primary list > >> > >> This is just reinventing hash bucketed lists. To get the benefits, all > >> we do is implement an N state constraint as backed by an N bucketed hash > >> list, which the kernel already has all the internal mechanics for. > > > > Agreed. > > > > No, a hash is used for quick lookup of a specific value, not to find > an extreme value. If you only have N possible values an N bucket hash list is rather efficient (provided N is small). But I would agree that knowing what N is represents an API change, and since plists can do this without changing the API, they're better. > It is however extremely similar to plists. The only > difference is that plists link all the secondary lists together. Right, so they would solve the *current* problem exactly. > If we > want to have constraints that autoexpire, then keeping the secondary > lists separate allows the same optimization as I did for > wakelock/suspend_blocker timeouts where no timer is active if an > (equal or stricter) non-expiring constraint is active. But this is a future discussion and not part of the patch. The way open source works is that we sort out the best implementation for the current conditions. If the implementation has to change because of future stuff, then we change it when the future stuff comes along. Changing implementations is easy (they don't have any externally visible impact). Changing the in-kernel API is slightly harder, but easily doable. It's only changing the user visible ABI that we worry about and try not to do. 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/ |