Hi,
In an attempt to clarify and simplify find_best_target I've identified four areas that I believe might benefit from simplifications/clarifications/fixing.
Please consider the ideas below as proposals towards this purpose. Also, consider these listed from narrow scope to broader scope.
1. Do not consider boost for !prefer_idle tasks, but always try to minimize capacity_orig. This was more or less done already by not having boosted and not-prefer idle tasks in typical Android configurations, but was never enforced in find_best_target code. This was discussed on this list as well in the "sched/fair: Prefer low capacity idle-CPU for boosted non-prefer-idle tasks" thread.
Leo, Viresh, I would encourage you to push your patch to Gerrit and continue the discussion there, as it is a valid point on its own, and less controversial than the broader scope patch at 2.
2. Do not consider boost for prefer_idle tasks, but always try to maximize capacity_orig. This is the discussion that the patch at https://android-review.googlesource.com/c/kernel/common/+/636583 is trying to trigger, although in scope it touches on the point above as well.
3. Make order of CPUs irrelevant for CPU selection in find_best_target. This is a patch I'll try to push to Gerrit tomorrow after I have more test results. This will not incorporate code for any of the points above and it will try to mimic the selection of a CPU that is done now in find_best target. But if the points above are proven on their own, this fact will simplify the code for this item and make the implementation of the item at 2. unnecessary.
4. Remove the prefer_idle case from find_best_target. https://android-review.googlesource.com/q/topic:%22strf-mainline%22+(status:...) This has the broadest scope but it is more difficult to validate.
Although broader scope items would make some narrow scope items unnecessary, they have value on their own if the boarder scope items cannot be proven in a reasonable amount of time, as they will provide fixes and support earlier than it takes to make a full re-factor of code.
- 1 will provide valuable fixes now. - 2 will simplify find_best_target decision logic to facilitate and simplify 3. - 3 will add support for tri-gear platforms for which the current order of CPUs will be incorrect in some cases. - 4 will bring us closer to the mainline behavior.
I'm adding the present of pretty pictures from what I call "validation by storm": I've created some kernels with combinations of the above items and tested them on a Pixel 2 device, just to make sure there are no important regressions introduced by them. But I believe all of these need to be validated independently to make sure we don't miss important corner cases. Results at https://gist.github.com/ionela-voinescu/f89815591c7f50864188094bb8c53ec4.
Given that it's difficult for all involved to keep up with discussions on both gerrit and eas-dev, I'd suggest to discuss design and direction ideas as part of this thread and push patches and individual test references to android-review directly.
Let me know what you think. All comments are welcomed!
Best regards, Ionela.
Hi Ionela,
On Tue, Mar 13, 2018 at 07:37:13PM +0000, Ionela Voinescu wrote:
Hi,
In an attempt to clarify and simplify find_best_target I've identified four areas that I believe might benefit from simplifications/clarifications/fixing.
Please consider the ideas below as proposals towards this purpose. Also, consider these listed from narrow scope to broader scope.
- Do not consider boost for !prefer_idle tasks, but always try to
minimize capacity_orig. This was more or less done already by not having boosted and not-prefer idle tasks in typical Android configurations, but was never enforced in find_best_target code. This was discussed on this list as well in the "sched/fair: Prefer low capacity idle-CPU for boosted non-prefer-idle tasks" thread.
Leo, Viresh, I would encourage you to push your patch to Gerrit and continue the discussion there, as it is a valid point on its own, and less controversial than the broader scope patch at 2.
- Do not consider boost for prefer_idle tasks, but always try to
maximize capacity_orig. This is the discussion that the patch at https://android-review.googlesource.com/c/kernel/common/+/636583 is trying to trigger, although in scope it touches on the point above as well.
- Make order of CPUs irrelevant for CPU selection in find_best_target.
This is a patch I'll try to push to Gerrit tomorrow after I have more test results. This will not incorporate code for any of the points above and it will try to mimic the selection of a CPU that is done now in find_best target. But if the points above are proven on their own, this fact will simplify the code for this item and make the implementation of the item at 2. unnecessary.
My first feeling is item 3 is dependent on item 1; right?
- Remove the prefer_idle case from find_best_target. https://android-review.googlesource.com/q/topic:%22strf-mainline%22+(status:...) This has the broadest scope but it is more difficult to validate.
If item 4 is not mature enough and in the middle stage, we can use one saperate function for 'perfer_idle' case, and another function for non latency senstive path; this is much easier way to refactor code and from then on we can focus on optimization saperately for these two pathes [1].
[1] https://git.linaro.org/people/leo.yan/linux-eas-opt.git/commit/?h=android-hi...
Although broader scope items would make some narrow scope items unnecessary, they have value on their own if the boarder scope items cannot be proven in a reasonable amount of time, as they will provide fixes and support earlier than it takes to make a full re-factor of code.
- 1 will provide valuable fixes now.
- 2 will simplify find_best_target decision logic to facilitate and simplify 3.
- 3 will add support for tri-gear platforms for which the current order of CPUs will be incorrect in some cases.
Here means tri-clusters case? If so, fbt() is hard to handle energy model overlap issue within them, this issue exists in two clusters case, and will get worse for tri-clusters cases.
So will item 3 handle energy model overlap issue? Before I wrote some patches for cluster based candidate selection [2], how about you think for this?
[2] https://git.linaro.org/people/leo.yan/linux-eas-opt.git/commit/?h=android-hi...
Thanks, Leo Yan
- 4 will bring us closer to the mainline behavior.
I'm adding the present of pretty pictures from what I call "validation by storm": I've created some kernels with combinations of the above items and tested them on a Pixel 2 device, just to make sure there are no important regressions introduced by them. But I believe all of these need to be validated independently to make sure we don't miss important corner cases. Results at https://gist.github.com/ionela-voinescu/f89815591c7f50864188094bb8c53ec4.
Given that it's difficult for all involved to keep up with discussions on both gerrit and eas-dev, I'd suggest to discuss design and direction ideas as part of this thread and push patches and individual test references to android-review directly.
Let me know what you think. All comments are welcomed!
Best regards, Ionela.
Hi Leo,
On 14/03/18 01:00, Leo Yan wrote:
Hi Ionela,
On Tue, Mar 13, 2018 at 07:37:13PM +0000, Ionela Voinescu wrote:
Hi,
In an attempt to clarify and simplify find_best_target I've identified four areas that I believe might benefit from simplifications/clarifications/fixing.
Please consider the ideas below as proposals towards this purpose. Also, consider these listed from narrow scope to broader scope.
- Do not consider boost for !prefer_idle tasks, but always try to
minimize capacity_orig. This was more or less done already by not having boosted and not-prefer idle tasks in typical Android configurations, but was never enforced in find_best_target code. This was discussed on this list as well in the "sched/fair: Prefer low capacity idle-CPU for boosted non-prefer-idle tasks" thread.
Leo, Viresh, I would encourage you to push your patch to Gerrit and continue the discussion there, as it is a valid point on its own, and less controversial than the broader scope patch at 2.
- Do not consider boost for prefer_idle tasks, but always try to
maximize capacity_orig. This is the discussion that the patch at https://android-review.googlesource.com/c/kernel/common/+/636583 is trying to trigger, although in scope it touches on the point above as well.
- Make order of CPUs irrelevant for CPU selection in find_best_target.
This is a patch I'll try to push to Gerrit tomorrow after I have more test results. This will not incorporate code for any of the points above and it will try to mimic the selection of a CPU that is done now in find_best target. But if the points above are proven on their own, this fact will simplify the code for this item and make the implementation of the item at 2. unnecessary.
My first feeling is item 3 is dependent on item 1; right?
Yes, it is, due to the cascading of conditions. I initially tried to have this code mimic exactly the behavior of find_best_target now, for both prefer_idle and !prefer_idle cases, but the code becomes unnecessarily complicated, when I believe 1 should be a fairly straightforward policy.
I'm waiting a bit more with this code, as to have more clarity on that side before diverting attention.
Case 2 will also simplify the code for this, although the implementation would be different - no start_cpu, but policies to maximise or minimise capacity_orig.
- Remove the prefer_idle case from find_best_target. https://android-review.googlesource.com/q/topic:%22strf-mainline%22+(status:...) This has the broadest scope but it is more difficult to validate.
If item 4 is not mature enough and in the middle stage, we can use one saperate function for 'perfer_idle' case, and another function for non latency senstive path; this is much easier way to refactor code and from then on we can focus on optimization saperately for these two pathes [1].
I don't believe separating them in their own functions will help too much with clarity, maybe just help people visualise them as separate issues. The duplication of code might not make it worth it though.
I will focus more on this code in the following days and I'll be able to have a better assessment of it.
[1] https://git.linaro.org/people/leo.yan/linux-eas-opt.git/commit/?h=android-hi...
Cool! You've already done it! Let me take a look and see if my opinion changes.
Although broader scope items would make some narrow scope items unnecessary, they have value on their own if the boarder scope items cannot be proven in a reasonable amount of time, as they will provide fixes and support earlier than it takes to make a full re-factor of code.
- 1 will provide valuable fixes now.
- 2 will simplify find_best_target decision logic to facilitate and simplify 3.
- 3 will add support for tri-gear platforms for which the current order of CPUs will be incorrect in some cases.
Here means tri-clusters case? If so, fbt() is hard to handle energy model overlap issue within them, this issue exists in two clusters case, and will get worse for tri-clusters cases.
It means having three groups of CPUs (preferably in their own DVFS domains) each with different capacity ranges. They might or might not all share a last level cache, that's why I was avoiding the term tri-cluster. When you say energy model overlap, I suppose you mean power-performance curves overlap, right?
So will item 3 handle energy model overlap issue? Before I wrote some patches for cluster based candidate selection [2], how about you think for this?
At the moment it does not. You mentioned this solution before and it does seem very interesting. I will definitely take a look. Feel free to post it on eas-dev so we can discuss it in a public forum. The same recommendation applies to the split of find_best_target cases.
Thank you very much, Ionela.
[2] https://git.linaro.org/people/leo.yan/linux-eas-opt.git/commit/?h=android-hi...
Thanks, Leo Yan
- 4 will bring us closer to the mainline behavior.
I'm adding the present of pretty pictures from what I call "validation by storm": I've created some kernels with combinations of the above items and tested them on a Pixel 2 device, just to make sure there are no important regressions introduced by them. But I believe all of these need to be validated independently to make sure we don't miss important corner cases. Results at https://gist.github.com/ionela-voinescu/f89815591c7f50864188094bb8c53ec4.
Given that it's difficult for all involved to keep up with discussions on both gerrit and eas-dev, I'd suggest to discuss design and direction ideas as part of this thread and push patches and individual test references to android-review directly.
Let me know what you think. All comments are welcomed!
Best regards, Ionela.
On Wed, Mar 14, 2018 at 06:12:46PM +0000, Ionela Voinescu wrote:
[...]
Although broader scope items would make some narrow scope items unnecessary, they have value on their own if the boarder scope items cannot be proven in a reasonable amount of time, as they will provide fixes and support earlier than it takes to make a full re-factor of code.
- 1 will provide valuable fixes now.
- 2 will simplify find_best_target decision logic to facilitate and simplify 3.
- 3 will add support for tri-gear platforms for which the current order of CPUs will be incorrect in some cases.
Here means tri-clusters case? If so, fbt() is hard to handle energy model overlap issue within them, this issue exists in two clusters case, and will get worse for tri-clusters cases.
It means having three groups of CPUs (preferably in their own DVFS domains) each with different capacity ranges. They might or might not all share a last level cache, that's why I was avoiding the term tri-cluster.
Thanks for clarification.
When you say energy model overlap, I suppose you mean power-performance curves overlap, right?
Yes.
[...]
Thanks, Leo Yan
-
Ionela Voinescu -
Leo Yan