On Thu, Mar 15, 2018 at 11:19:55AM +0800, Ke Wang wrote:
Hi Leo,
Thanks for your feedback.
The devation you mentioned is a issue we can talk further.
Actually, my question is the "energy_diff re-factor" patch is not just re-factor, it also changed the calculation of group_idle_state().
Before the re-factor, when calculating the idle state of CPU level in single/multiple cluster case or idle state of CLUSTER level in multi-clusters case for previous sched group, it will not "goto end" due to src_in_grp != dst_in_grp. After the re-factor, for above mentioned idle state calculation for previous sched group(cpu_idx = EAS_CPU_PRV), it will always "goto end" due to src_in_grp == dst_in_grp.
For example(4 LITTLE+4big, single cluster case), Index | src_cpu | next_cpu ------+------------------+-----------------+------------------ CPU # | CPU_0 (LITTLE) | CPU_1 (LITTLE)
Be honest, I am confused with below example :) Let's calculate step by step.
Before re-factor:
The calculation of idle index if put task on previous cpu: cpu0: idle_get_state_idx + idle estimate (src_in_grp != dst_in_grp)
sched_group_cpus(sg) = 0x1 src_in_grp = 1, dst_in_grp = 0, so (src_in_grp != dst_in_grp)
cpu1: idle_get_state_idx + idle estimate (src_in_grp != dst_in_grp)
sched_group_cpus(sg) = 0x2 src_in_grp = 0, dst_in_grp = 1, so (src_in_grp != dst_in_grp)
cpu2: idle_get_state_idx + idle estimate (src_in_grp != dst_in_grp)
sched_group_cpus(sg) = 0x4 src_in_grp = 0, dst_in_grp = 0, so (src_in_grp == dst_in_grp)
I'm confused why these two variable can be unequal.
cpu3: idle_get_state_idx + idle estimate (src_in_grp != dst_in_grp)
sched_group_cpus(sg) = 0x8 src_in_grp = 0, dst_in_grp = 0, so (src_in_grp == dst_in_grp)
I'm confused why these two variable can be unequal.
cluster0: idle_get_state_idx (src_in_grp == dst_in_grp)
sched_group_cpus(sg) = 0xf src_in_grp = 1, dst_in_grp = 1, so (src_in_grp == dst_in_grp)
After re-factor:
The calculation of idle index if put task on previous cpu: cpu0: idle_get_state_idx (src_in_grp == dst_in_grp)
Is sched_group_cpus(sg) = 0x10? EAS_CPU_PRV is CPU_0, and EAS_CPU_NTX is CPU_1 src_in_grp = 0, dst_in_grp = 0, so (src_in_grp == dst_in_grp)
cpu1: idle_get_state_idx (src_in_grp == dst_in_grp)
sched_group_cpus(sg) = 0x20 EAS_CPU_PRV is CPU_0, and EAS_CPU_NTX is CPU_1 src_in_grp = 0, dst_in_grp = 0, so (src_in_grp == dst_in_grp)
cpu2: idle_get_state_idx (src_in_grp == dst_in_grp)
sched_group_cpus(sg) = 0x40 EAS_CPU_PRV is CPU_0, and EAS_CPU_NTX is CPU_1 src_in_grp = 0, dst_in_grp = 0, so (src_in_grp == dst_in_grp)
cpu3: idle_get_state_idx (src_in_grp == dst_in_grp)
sched_group_cpus(sg) = 0x80 EAS_CPU_PRV is CPU_0, and EAS_CPU_NTX is CPU_1 src_in_grp = 0, dst_in_grp = 0, so (src_in_grp == dst_in_grp)
cluster0: idle_state_idx (src_in_grp == dst_in_grp)
Here I assume: sched_group_cpus(sg) = 0xf0 EAS_CPU_PRV is CPU_0, and EAS_CPU_NTX is CPU_1 src_in_grp = 0, dst_in_grp = 0, so (src_in_grp == dst_in_grp)
Thanks, Leo Yan
On 14 March 2018 at 17:05, Leo Yan leo.yan@linaro.org wrote:
On Wed, Mar 14, 2018 at 04:48:33PM +0800, Leo Yan wrote:
On Wed, Mar 14, 2018 at 03:29:12PM +0800, Ke Wang wrote:
hi Chris, Patrick, Quentin,
On android-4.9-eas-dev branch, after the commit of energy_diff re-factor("sched/fair: re-factor energy_diff to use a single (extensible) energy_env"), the calculation of group_idle_state for previous cpu had changed.
Before the re-factor, for eenv_before, there is chance to continue doing group_idle_state estimation(not "0goto end" directly); After the re-factor, for cpu_idx=EAS_CPU_PRV, there is no chance to do estimation("goto end" directly);
Is it deliberate for this change? If yes, do you have any test result for this?
-static int group_idle_state(struct energy_env *eenv, struct sched_group *sg) +static int group_idle_state(struct energy_env *eenv, int cpu_idx) {
struct sched_group *sg = eenv->sg; int i, state = INT_MAX; int src_in_grp, dst_in_grp; long grp_util = 0;@@ -5556,8 +5610,10 @@ static int group_idle_state(struct energy_env *eenv, struct sched_group *sg) /* Take non-cpuidle idling into account (active idle/arch_cpu_idle()) */ state++;
src_in_grp = cpumask_test_cpu(eenv->src_cpu, sched_group_cpus(sg));dst_in_grp = cpumask_test_cpu(eenv->dst_cpu, sched_group_cpus(sg));
src_in_grp = cpumask_test_cpu(eenv->cpu[EAS_CPU_PRV].cpu_id,sched_group_cpus(sg));dst_in_grp = cpumask_test_cpu(eenv->cpu[cpu_idx].cpu_id,sched_group_cpus(sg));Hi Ke,
Are you asking question for single cluster case? If we consider multiple clusters case, here might introduce mess, please see below combination:
eenv->sg is for big cluster scheduling group:
Index | EAS_CPU_PRV | EAS_CPU_NTX | EAS_CPU_BKP ------+------------------+-----------------+------------------ CPU # | CPU_0 (LITTLE) | CPU_4 (big) | CPU_5 (big) ------+------------------+-----------------+------------------ Flow | goto end | not goto end | not goto end
For upper case, the big cluster energy calculation uses different idle state estimation for CPU_0/CPU_4/CPU_5. This might introduce big devation for final result, thoughts?
Just reminding, 'eenv->sg' is not only for cluster level scheduling group, it also can be for CPU level. There have more cases for consideration.
if (src_in_grp == dst_in_grp) { /* both CPUs under consideration are in the same group or not in * either group, migration should leave idle state the same.@@ -5571,7 +5627,7 @@ static int group_idle_state(struct energy_env *eenv, struct sched_group *sg) */ for_each_cpu(i, sched_group_cpus(sg)) { grp_util += cpu_util_wake(i, eenv->p);
if (unlikely(i == eenv->trg_cpu))
if (unlikely(i == eenv->cpu[cpu_idx].cpu_id)) grp_util += eenv->util_delta; }Thanks, Ke Wang _______________________________________________ eas-dev mailing list eas-dev@lists.linaro.org https://lists.linaro.org/mailman/listinfo/eas-dev