arm: topology: fix capacity calculation on SMP SoCs
PATCH 1/1 arm: topology: fix capacity calculation on SMP SoCs
This commit sets the capacity of the average CPU in SMP systems to SCHED_CAPACITY_SCALE.
Ignoring the condition "min_capacity==max_capacity" causes the function update_cpu_capacity( .. ) to generate out of range values [1]. This is because the default value of middle_capacity is used in the final calculation instead of a valid scaling factor.
Incidentally, when out of range values are generated and if SCHED_FEAT(ARCH_POWER, true), the load balancing algorithm makes the wrong decisions typically overallocating work on one of the cores while leaving the others unused.
[1] val > SCHED_CAPACITY_SCALE
--- arch/arm/kernel/topology.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm/kernel/topology.c +4 -4
@@ -137,14 +137,14 @@ static void __init parse_dt_topology(void)cpu_capacity (cpu) = capacity; } -/* If min and max capacities are equals, we bypass the update of the -* cpu_scale because all CPUs have the same capacity. Otherwise, we -* compute a middle_capacity factor that will ensure that the capacity +/* Compute a middle_capacity factor that will ensure that the capacity * of an 'average' CPU of the system will be as close as possible to * SCHED_CAPACITY_SCALE, which is the default value, but with the * constraint explained near table_efficiency[]. */ -if (4 *max_capacity < (3 *(max_capacity + min_capacity))) +if (min_capacity == max_capacity) + middle_capacity = min_capacity >>SCHED_CAPACITY_SHIFT ; +else if (4 *max_capacity < (3 *(max_capacity + min_capacity))) middle_capacity = (min_capacity + max_capacity) >> (SCHED_CAPACITY_SHIFT +1 );else
Signature · 1 line
2.1.4
PATCH 1/1 arm: topology: fix capacity calculation on SMP SoCs
Hi Jorge,
Quoted 8 lines · Jorge Ramirez-Ortiz
This commit sets the capacity of the average CPU in SMP systems to
SCHED_CAPACITY_SCALE.Ignoring the condition "min_capacity==max_capacity" causes the function update_cpu_capacity( .. ) to generate out of range values [1]. This is because the default value of
middle_capacityis used in the final calculation instead of a valid scaling factor.
Quoted 4 lines
Incidentally, when out of range values are generated and if
SCHED_FEAT(ARCH_POWER, true), the load balancing algorithm makes the wrong decisions typically overallocating work on one of the cores while leaving the others unused.
Have you got an example ?
Quoted 18 lines
[1] val >
SCHED_CAPACITY_SCALESigned-off-by Jorge Ramirez-Ortiz <[email protected]>--- arch/arm/kernel/topology.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm/kernel/topology.c +0
-2@@ -137,14 +137,14 @@ static void __init parse_dt_topology(void)cpu_capacity (cpu) = capacity; } -/* If min and max capacities are equals, we bypass the update of the -* cpu_scale because all CPUs have the same capacity. Otherwise, we
arch/arm/kernel/topology.c static void __init parse_dt_topology(void) · line 147
Quoted 10 lines
arch/arm/kernel/topology.c +4
-2-* compute a middle_capacity factor that will ensure that the capacity +/* Compute a middle_capacity factor that will ensure that the capacity * of an 'average' CPU of the system will be as close as possible to * SCHED_CAPACITY_SCALE, which is the default value, but with the * constraint explained near table_efficiency[]. */ -if (4 *max_capacity < (3 *(max_capacity + min_capacity))) +if (min_capacity == max_capacity) + middle_capacity = min_capacity >>SCHED_CAPACITY_SHIFT ; +else if (4 *max_capacity < (3 *(max_capacity + min_capacity)))
if min_capacity == max_capacity then the condition 4*max_capacity < (3*(max_capacity + min_capacity)) is true and middle_capacity = (min_capacity + max_capacity) >> (SCHED_CAPACITY_SHIFT+1) = 2*min_capacity >> (SCHED_CAPACITY_SHIFT+1) so middle capacity = min_capacity >> SCHED_CAPACITY_SHIFT
I don't see what your change does that is not already done by current code
Regards,
Vincent
Quoted 6 lines
arch/arm/kernel/topology.c +0
-1middle_capacity = (min_capacity + max_capacity) >> (SCHED_CAPACITY_SHIFT +1 ); else --2.1.4
PATCH 1/1 arm: topology: fix capacity calculation on SMP SoCs
Quoted 12 lines · Vincent Guittot
Hi Jorge,
Quoted 8 lines · Jorge Ramirez-Ortiz
This commit sets the capacity of the average CPU in SMP systems to
SCHED_CAPACITY_SCALE.Ignoring the condition "min_capacity==max_capacity" causes the function update_cpu_capacity( .. ) to generate out of range values [1]. This is because the default value of
middle_capacityis used in the final calculation instead of a valid scaling factor.
Quoted 5 lines
Quoted 4 lines
Incidentally, when out of range values are generated and if
SCHED_FEAT(ARCH_POWER, true), the load balancing algorithm makes the wrong decisions typically overallocating work on one of the cores while leaving the others unused.Have you got an example ?
This was tested on lsk 3.10 which was slightly different (see my comments below). I can get you the test source code.
Quoted 34 lines
Quoted 18 lines
[1] val >
SCHED_CAPACITY_SCALESigned-off-by Jorge Ramirez-Ortiz <[email protected]>--- arch/arm/kernel/topology.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm/kernel/topology.c +0
-2@@ -137,14 +137,14 @@ static void __init parse_dt_topology(void)cpu_capacity (cpu) = capacity; } -/* If min and max capacities are equals, we bypass the update of the -* cpu_scale because all CPUs have the same capacity. Otherwise, we arch/arm/kernel/topology.c static void __init parse_dt_topology(void) · line 147
Quoted 10 lines
arch/arm/kernel/topology.c +4
-2-* compute a middle_capacity factor that will ensure that the capacity +/* Compute a middle_capacity factor that will ensure that the capacity * of an 'average' CPU of the system will be as close as possible to * SCHED_CAPACITY_SCALE, which is the default value, but with the * constraint explained near table_efficiency[]. */ -if (4 *max_capacity < (3 *(max_capacity + min_capacity))) +if (min_capacity == max_capacity) + middle_capacity = min_capacity >>SCHED_CAPACITY_SHIFT ; +else if (4 *max_capacity < (3 *(max_capacity + min_capacity)))if
min_capacity==max_capacitythen the condition 4*max_capacity < (3*(max_capacity +min_capacity)) is true andmiddle_capacity= (min_capacity+max_capacity) >> (SCHED_CAPACITY_SHIFT+1) = 2*min_capacity >> (SCHED_CAPACITY_SHIFT+1) so middle capacity =min_capacity>>SCHED_CAPACITY_SHIFT
Quoted 1 line
I don't see what your change does that is not already done by current code
ah you are right! I didn't even consider it would be handled under that condition. My mistake.
It seem the problem is only present in lsk 3.10 then [1]
btw the comments above the condition should still be edited (cpu_scale is always updated)
ok ignore this patch and let's only fix it on the lsk in the same way it is done here.
thanks