MISFIT_POWER requires moving the task to a more efficient CPU.
This can happen when a big task is capped by uclamp_max, but another
task wakes up on this CPU that can lift the capping, in this case we
need to migrate it to another, likely smaller, CPU to save power.
To enable that we need to be smarter about which CPU should do the pull.
But this requires enabling load balance on all CPUs so that the correct
CPU does the pull. Instead of the current behavior of nominating the CPU
with the largest capacity in the group to do the pull.
This is important to ensure the MISFIT_POWER tasks don't end up on most
performant CPUs, which is the default behavior of the load balance. We
could end up wasting energy unnecessarily or interfere with more
important tasks on these big CPUs - leading to worse user experience.
To ensure optimal decision is made, we need to enable calling feec() to
pick the most efficient CPU for us. Which means we need to force feec()
to ignore overutilized flag. If feec() returns the same value as the CPU
that is doing the balance, we perform the pull. Otherwise we'd have to
defer for another CPU to do the pull.
To minimize the overhead, this is only done for MISFIT_POWER.
For capacity aware scheduling or none HMP systems, we will pick a CPU
that we won't cause its uclamp_max to be uncapped.
Signed-off-by: Qais Yousef (Google) <[email protected]>
---
kernel/sched/fair.c | 77 ++++++++++++++++++++++++++++++++++++++++----
kernel/sched/sched.h | 1 +
2 files changed, 71 insertions(+), 7 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index dd49b89a6e3e..328467dbe88b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5066,10 +5066,33 @@ static inline int task_fits_cpu(struct task_struct *p, int cpu)
static inline int is_misfit_task(struct task_struct *p, struct rq *rq,
misfit_reason_t *reason)
{
+ unsigned long rq_util_max;
+ unsigned long p_util_min;
+ unsigned long p_util_max;
+ unsigned long util;
+
if (!p || p->nr_cpus_allowed == 1)
return 0;
- if (task_fits_cpu(p, cpu_of(rq)))
+ rq_util_max = uclamp_rq_get(rq, UCLAMP_MAX);
+ p_util_min = uclamp_eff_value(p, UCLAMP_MIN);
+ p_util_max = uclamp_eff_value(p, UCLAMP_MAX);
+ util = task_util_est(p);
+
+ if (uclamp_is_used()) {
+ /*
+ * Check if a big task is capped by uclamp max is now sharing
+ * the cpu with something else uncapped and must be moved away
+ */
+ if (rq_util_max > p_util_max && util > p_util_max) {
+ if (reason)
+ *reason = MISFIT_POWER;
+
+ return 1;
+ }
+ }
+
+ if (util_fits_cpu(util, p_util_min, p_util_max, cpu_of(rq)))
return 0;
if (reason)
@@ -7923,7 +7946,8 @@ compute_energy(struct energy_env *eenv, struct perf_domain *pd,
* other use-cases too. So, until someone finds a better way to solve this,
* let's keep things simple by re-using the existing slow path.
*/
-static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
+static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu,
+ bool ignore_ou)
{
struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_rq_mask);
unsigned long prev_delta = ULONG_MAX, best_delta = ULONG_MAX;
@@ -7940,7 +7964,7 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
rcu_read_lock();
pd = rcu_dereference(rd->pd);
- if (!pd || READ_ONCE(rd->overutilized))
+ if (!pd || (READ_ONCE(rd->overutilized) && !ignore_ou))
goto unlock;
/*
@@ -8144,7 +8168,7 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int wake_flags)
return cpu;
if (sched_energy_enabled()) {
- new_cpu = find_energy_efficient_cpu(p, prev_cpu);
+ new_cpu = find_energy_efficient_cpu(p, prev_cpu, false);
if (new_cpu >= 0)
return new_cpu;
new_cpu = prev_cpu;
@@ -9030,6 +9054,7 @@ static int detach_tasks(struct lb_env *env)
{
struct list_head *tasks = &env->src_rq->cfs_tasks;
unsigned long util, load;
+ misfit_reason_t reason;
struct task_struct *p;
int detached = 0;
@@ -9118,9 +9143,28 @@ static int detach_tasks(struct lb_env *env)
case migrate_misfit:
/* This is not a misfit task */
- if (!is_misfit_task(p, cpu_rq(env->src_cpu), NULL))
+ if (!is_misfit_task(p, cpu_rq(env->src_cpu), &reason))
goto next;
+ if (reason == MISFIT_POWER) {
+ if (sched_energy_enabled()) {
+ int new_cpu = find_energy_efficient_cpu(p, env->src_cpu, true);
+ if (new_cpu != env->dst_cpu)
+ goto next;
+ } else {
+ unsigned long dst_uclamp_max = uclamp_rq_get(env->dst_rq, UCLAMP_MAX);
+ unsigned long p_uclamp_max = uclamp_eff_value(p, UCLAMP_MAX);
+
+ /*
+ * Pick a task that will not cause us
+ * to uncap dst_cpu. Or give up until
+ * another CPU tries to do the pull.
+ */
+ if (p_uclamp_max > dst_uclamp_max)
+ goto next;
+ }
+ }
+
env->imbalance = 0;
break;
}
@@ -11203,6 +11247,18 @@ static int should_we_balance(struct lb_env *env)
if (!cpumask_test_cpu(env->dst_cpu, env->cpus))
return 0;
+ /*
+ * For MISFIT_POWER, we need every CPU to do the lb so that we can pick
+ * the most energy efficient one via EAS if available or by making sure
+ * the dst_rq uclamp_max higher than the misfit task's uclamp_max.
+ *
+ * We don't want to do a pull if both src and dst cpus are in
+ * MISFIT_POWER state.
+ */
+ if (env->src_rq->misfit_reason == MISFIT_POWER &&
+ env->dst_rq->misfit_reason != MISFIT_POWER)
+ return 1;
+
/*
* In the newly idle case, we will allow all the CPUs
* to do the newly idle load balance.
@@ -11431,8 +11487,12 @@ static int load_balance(int this_cpu, struct rq *this_rq,
* We do not want newidle balance, which can be very
* frequent, pollute the failure counter causing
* excessive cache_hot migrations and active balances.
+ *
+ * MISFIT_POWER can also trigger a lot of failed misfit
+ * migrations as we need to ask every CPU to do the pull and
+ * expectedly lots of failures will incur.
*/
- if (idle != CPU_NEWLY_IDLE)
+ if (idle != CPU_NEWLY_IDLE && env.src_rq->misfit_reason != MISFIT_POWER)
sd->nr_balance_failed++;
if (need_active_balance(&env)) {
@@ -11515,8 +11575,11 @@ static int load_balance(int this_cpu, struct rq *this_rq,
* repeatedly reach this code, which would lead to balance_interval
* skyrocketing in a short amount of time. Skip the balance_interval
* increase logic to avoid that.
+ *
+ * So does MISFIT_POWER which asks every CPU to do the pull as we can't
+ * tell which one would be the best one to move to before hand.
*/
- if (env.idle == CPU_NEWLY_IDLE)
+ if (env.idle == CPU_NEWLY_IDLE || env.src_rq->misfit_reason == MISFIT_POWER)
goto out;
/* tune up the balancing interval */
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 399b6526afab..3852109ffe62 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -964,6 +964,7 @@ struct balance_callback {
typedef enum misfit_reason {
MISFIT_PERF, /* Requires moving to a more performant CPU */
+ MISFIT_POWER, /* Requires moving to a more efficient CPU */
} misfit_reason_t;
/*
--
2.34.1
On 12/9/23 02:17, Qais Yousef wrote:
> MISFIT_POWER requires moving the task to a more efficient CPU.
>
> This can happen when a big task is capped by uclamp_max, but another
> task wakes up on this CPU that can lift the capping, in this case we
> need to migrate it to another, likely smaller, CPU to save power.
>
> To enable that we need to be smarter about which CPU should do the pull.
> But this requires enabling load balance on all CPUs so that the correct
> CPU does the pull. Instead of the current behavior of nominating the CPU
> with the largest capacity in the group to do the pull.
>
> This is important to ensure the MISFIT_POWER tasks don't end up on most
> performant CPUs, which is the default behavior of the load balance. We
> could end up wasting energy unnecessarily or interfere with more
> important tasks on these big CPUs - leading to worse user experience.
>
> To ensure optimal decision is made, we need to enable calling feec() to
> pick the most efficient CPU for us. Which means we need to force feec()
> to ignore overutilized flag. If feec() returns the same value as the CPU
> that is doing the balance, we perform the pull. Otherwise we'd have to
> defer for another CPU to do the pull.
>
> To minimize the overhead, this is only done for MISFIT_POWER.
>
> For capacity aware scheduling or none HMP systems, we will pick a CPU
> that we won't cause its uclamp_max to be uncapped.
>
> Signed-off-by: Qais Yousef (Google) <[email protected]>
> ---
> kernel/sched/fair.c | 77 ++++++++++++++++++++++++++++++++++++++++----
> kernel/sched/sched.h | 1 +
> 2 files changed, 71 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index dd49b89a6e3e..328467dbe88b 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5066,10 +5066,33 @@ static inline int task_fits_cpu(struct task_struct *p, int cpu)
> static inline int is_misfit_task(struct task_struct *p, struct rq *rq,
> misfit_reason_t *reason)
> {
> + unsigned long rq_util_max;
> + unsigned long p_util_min;
> + unsigned long p_util_max;
> + unsigned long util;
> +
> if (!p || p->nr_cpus_allowed == 1)
> return 0;
>
> - if (task_fits_cpu(p, cpu_of(rq)))
> + rq_util_max = uclamp_rq_get(rq, UCLAMP_MAX);
> + p_util_min = uclamp_eff_value(p, UCLAMP_MIN);
> + p_util_max = uclamp_eff_value(p, UCLAMP_MAX);
> + util = task_util_est(p);
> +
> + if (uclamp_is_used()) {
> + /*
> + * Check if a big task is capped by uclamp max is now sharing
> + * the cpu with something else uncapped and must be moved away
> + */
> + if (rq_util_max > p_util_max && util > p_util_max) {
> + if (reason)
> + *reason = MISFIT_POWER;
> +
> + return 1;
> + }
> + }
> +
> + if (util_fits_cpu(util, p_util_min, p_util_max, cpu_of(rq)))
> return 0;
>
> if (reason)
> @@ -7923,7 +7946,8 @@ compute_energy(struct energy_env *eenv, struct perf_domain *pd,
> * other use-cases too. So, until someone finds a better way to solve this,
> * let's keep things simple by re-using the existing slow path.
> */
> -static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
> +static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu,
> + bool ignore_ou)
> {
> struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_rq_mask);
> unsigned long prev_delta = ULONG_MAX, best_delta = ULONG_MAX;
> @@ -7940,7 +7964,7 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
>
> rcu_read_lock();
> pd = rcu_dereference(rd->pd);
> - if (!pd || READ_ONCE(rd->overutilized))
> + if (!pd || (READ_ONCE(rd->overutilized) && !ignore_ou))
> goto unlock;
>
> /*
> @@ -8144,7 +8168,7 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int wake_flags)
> return cpu;
>
> if (sched_energy_enabled()) {
> - new_cpu = find_energy_efficient_cpu(p, prev_cpu);
> + new_cpu = find_energy_efficient_cpu(p, prev_cpu, false);
> if (new_cpu >= 0)
> return new_cpu;
> new_cpu = prev_cpu;
> @@ -9030,6 +9054,7 @@ static int detach_tasks(struct lb_env *env)
> {
> struct list_head *tasks = &env->src_rq->cfs_tasks;
> unsigned long util, load;
> + misfit_reason_t reason;
> struct task_struct *p;
> int detached = 0;
>
> @@ -9118,9 +9143,28 @@ static int detach_tasks(struct lb_env *env)
>
> case migrate_misfit:
> /* This is not a misfit task */
> - if (!is_misfit_task(p, cpu_rq(env->src_cpu), NULL))
> + if (!is_misfit_task(p, cpu_rq(env->src_cpu), &reason))
> goto next;
>
> + if (reason == MISFIT_POWER) {
> + if (sched_energy_enabled()) {
> + int new_cpu = find_energy_efficient_cpu(p, env->src_cpu, true);
> + if (new_cpu != env->dst_cpu)
> + goto next;
> + } else {
> + unsigned long dst_uclamp_max = uclamp_rq_get(env->dst_rq, UCLAMP_MAX);
> + unsigned long p_uclamp_max = uclamp_eff_value(p, UCLAMP_MAX);
> +
> + /*
> + * Pick a task that will not cause us
> + * to uncap dst_cpu. Or give up until
> + * another CPU tries to do the pull.
> + */
> + if (p_uclamp_max > dst_uclamp_max)
> + goto next;
> + }
> + }
> +
> env->imbalance = 0;
> break;
> }
> @@ -11203,6 +11247,18 @@ static int should_we_balance(struct lb_env *env)
> if (!cpumask_test_cpu(env->dst_cpu, env->cpus))
> return 0;
>
> + /*
> + * For MISFIT_POWER, we need every CPU to do the lb so that we can pick
> + * the most energy efficient one via EAS if available or by making sure
> + * the dst_rq uclamp_max higher than the misfit task's uclamp_max.
> + *
> + * We don't want to do a pull if both src and dst cpus are in
> + * MISFIT_POWER state.
> + */
> + if (env->src_rq->misfit_reason == MISFIT_POWER &&
In case someone tries the patch, it seems the src_rq field of the
struct lb_env env in load_balance() is not initialized, so I think accesses
to env->src_rq->misfit_reason should be replaced by:
(env->src_rq && env->src_rq->misfit_reason)
> + env->dst_rq->misfit_reason != MISFIT_POWER)
> + return 1;
> +
> /*
> * In the newly idle case, we will allow all the CPUs
> * to do the newly idle load balance.
> @@ -11431,8 +11487,12 @@ static int load_balance(int this_cpu, struct rq *this_rq,
> * We do not want newidle balance, which can be very
> * frequent, pollute the failure counter causing
> * excessive cache_hot migrations and active balances.
> + *
> + * MISFIT_POWER can also trigger a lot of failed misfit
> + * migrations as we need to ask every CPU to do the pull and
> + * expectedly lots of failures will incur.
> */
> - if (idle != CPU_NEWLY_IDLE)
> + if (idle != CPU_NEWLY_IDLE && env.src_rq->misfit_reason != MISFIT_POWER)
> sd->nr_balance_failed++;
>
> if (need_active_balance(&env)) {
> @@ -11515,8 +11575,11 @@ static int load_balance(int this_cpu, struct rq *this_rq,
> * repeatedly reach this code, which would lead to balance_interval
> * skyrocketing in a short amount of time. Skip the balance_interval
> * increase logic to avoid that.
> + *
> + * So does MISFIT_POWER which asks every CPU to do the pull as we can't
> + * tell which one would be the best one to move to before hand.
> */
> - if (env.idle == CPU_NEWLY_IDLE)
> + if (env.idle == CPU_NEWLY_IDLE || env.src_rq->misfit_reason == MISFIT_POWER)
> goto out;
>
> /* tune up the balancing interval */
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 399b6526afab..3852109ffe62 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -964,6 +964,7 @@ struct balance_callback {
>
> typedef enum misfit_reason {
> MISFIT_PERF, /* Requires moving to a more performant CPU */
> + MISFIT_POWER, /* Requires moving to a more efficient CPU */
> } misfit_reason_t;
>
> /*
Hello Qais,
I tried to do as you indicated at:
https://lore.kernel.org/all/20231228233848.piyodw2s2ytli37a@airbuntu/
without success. I can see that the task is migrated from a big CPU to
smaller CPUs, but it doesn't seem to be related to the new MISFIT_POWER
feature.
Indeed, if the uclamp_max value of a CPU-bound task is set to 0, isn't it
normal have EAS/feec() migrating the task to smaller CPUs ? I added tracing
inside is_misfit_task() and load_balance()'s misfit path and could not see
this path being used.
On 12/9/23 02:17, Qais Yousef wrote:
> MISFIT_POWER requires moving the task to a more efficient CPU.
>
> This can happen when a big task is capped by uclamp_max, but another
> task wakes up on this CPU that can lift the capping, in this case we
> need to migrate it to another, likely smaller, CPU to save power.
Just to be sure, are we talking about the following path, where sugov
decides which OPP to select ?
sugov_get_util()
\-effective_cpu_util()
\-uclamp_rq_util_with()
To try to describe the issue in my own words, IIUC, the issue comes from
the fact that during energy estimations in feec(), we don't estimate the
impact of enqueuing a task on the rq's UCLAMP_MAX value. So a rq with a
little UCLAMP_MAX value might see the value grows if an uncapped task
is enqueued, leading to raising the frequency and consuming more
power.
Thus, this patch tries to detect such scenario and migrate the clamped
tasks.
Maybe another approach would be to estimate the impact of enqueuing a
task on the rq's UCLAMP_MAX value ?
Regards,
Pierre
>
> To enable that we need to be smarter about which CPU should do the pull.
> But this requires enabling load balance on all CPUs so that the correct
> CPU does the pull. Instead of the current behavior of nominating the CPU
> with the largest capacity in the group to do the pull.
>
> This is important to ensure the MISFIT_POWER tasks don't end up on most
> performant CPUs, which is the default behavior of the load balance. We
> could end up wasting energy unnecessarily or interfere with more
> important tasks on these big CPUs - leading to worse user experience.
>
> To ensure optimal decision is made, we need to enable calling feec() to
> pick the most efficient CPU for us. Which means we need to force feec()
> to ignore overutilized flag. If feec() returns the same value as the CPU
> that is doing the balance, we perform the pull. Otherwise we'd have to
> defer for another CPU to do the pull.
>
> To minimize the overhead, this is only done for MISFIT_POWER.
>
> For capacity aware scheduling or none HMP systems, we will pick a CPU
> that we won't cause its uclamp_max to be uncapped.
>
> Signed-off-by: Qais Yousef (Google) <[email protected]>
> ---
> kernel/sched/fair.c | 77 ++++++++++++++++++++++++++++++++++++++++----
> kernel/sched/sched.h | 1 +
> 2 files changed, 71 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index dd49b89a6e3e..328467dbe88b 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5066,10 +5066,33 @@ static inline int task_fits_cpu(struct task_struct *p, int cpu)
> static inline int is_misfit_task(struct task_struct *p, struct rq *rq,
> misfit_reason_t *reason)
> {
> + unsigned long rq_util_max;
> + unsigned long p_util_min;
> + unsigned long p_util_max;
> + unsigned long util;
> +
> if (!p || p->nr_cpus_allowed == 1)
> return 0;
>
> - if (task_fits_cpu(p, cpu_of(rq)))
> + rq_util_max = uclamp_rq_get(rq, UCLAMP_MAX);
> + p_util_min = uclamp_eff_value(p, UCLAMP_MIN);
> + p_util_max = uclamp_eff_value(p, UCLAMP_MAX);
> + util = task_util_est(p);
> +
> + if (uclamp_is_used()) {
> + /*
> + * Check if a big task is capped by uclamp max is now sharing
> + * the cpu with something else uncapped and must be moved away
> + */
> + if (rq_util_max > p_util_max && util > p_util_max) {
> + if (reason)
> + *reason = MISFIT_POWER;
> +
> + return 1;
> + }
> + }
> +
> + if (util_fits_cpu(util, p_util_min, p_util_max, cpu_of(rq)))
> return 0;
>
> if (reason)
> @@ -7923,7 +7946,8 @@ compute_energy(struct energy_env *eenv, struct perf_domain *pd,
> * other use-cases too. So, until someone finds a better way to solve this,
> * let's keep things simple by re-using the existing slow path.
> */
> -static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
> +static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu,
> + bool ignore_ou)
> {
> struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_rq_mask);
> unsigned long prev_delta = ULONG_MAX, best_delta = ULONG_MAX;
> @@ -7940,7 +7964,7 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
>
> rcu_read_lock();
> pd = rcu_dereference(rd->pd);
> - if (!pd || READ_ONCE(rd->overutilized))
> + if (!pd || (READ_ONCE(rd->overutilized) && !ignore_ou))
> goto unlock;
>
> /*
> @@ -8144,7 +8168,7 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int wake_flags)
> return cpu;
>
> if (sched_energy_enabled()) {
> - new_cpu = find_energy_efficient_cpu(p, prev_cpu);
> + new_cpu = find_energy_efficient_cpu(p, prev_cpu, false);
> if (new_cpu >= 0)
> return new_cpu;
> new_cpu = prev_cpu;
> @@ -9030,6 +9054,7 @@ static int detach_tasks(struct lb_env *env)
> {
> struct list_head *tasks = &env->src_rq->cfs_tasks;
> unsigned long util, load;
> + misfit_reason_t reason;
> struct task_struct *p;
> int detached = 0;
>
> @@ -9118,9 +9143,28 @@ static int detach_tasks(struct lb_env *env)
>
> case migrate_misfit:
> /* This is not a misfit task */
> - if (!is_misfit_task(p, cpu_rq(env->src_cpu), NULL))
> + if (!is_misfit_task(p, cpu_rq(env->src_cpu), &reason))
> goto next;
>
> + if (reason == MISFIT_POWER) {
> + if (sched_energy_enabled()) {
> + int new_cpu = find_energy_efficient_cpu(p, env->src_cpu, true);
> + if (new_cpu != env->dst_cpu)
> + goto next;
> + } else {
> + unsigned long dst_uclamp_max = uclamp_rq_get(env->dst_rq, UCLAMP_MAX);
> + unsigned long p_uclamp_max = uclamp_eff_value(p, UCLAMP_MAX);
> +
> + /*
> + * Pick a task that will not cause us
> + * to uncap dst_cpu. Or give up until
> + * another CPU tries to do the pull.
> + */
> + if (p_uclamp_max > dst_uclamp_max)
> + goto next;
> + }
> + }
> +
> env->imbalance = 0;
> break;
> }
> @@ -11203,6 +11247,18 @@ static int should_we_balance(struct lb_env *env)
> if (!cpumask_test_cpu(env->dst_cpu, env->cpus))
> return 0;
>
> + /*
> + * For MISFIT_POWER, we need every CPU to do the lb so that we can pick
> + * the most energy efficient one via EAS if available or by making sure
> + * the dst_rq uclamp_max higher than the misfit task's uclamp_max.
> + *
> + * We don't want to do a pull if both src and dst cpus are in
> + * MISFIT_POWER state.
> + */
> + if (env->src_rq->misfit_reason == MISFIT_POWER &&
> + env->dst_rq->misfit_reason != MISFIT_POWER)
> + return 1;
> +
> /*
> * In the newly idle case, we will allow all the CPUs
> * to do the newly idle load balance.
> @@ -11431,8 +11487,12 @@ static int load_balance(int this_cpu, struct rq *this_rq,
> * We do not want newidle balance, which can be very
> * frequent, pollute the failure counter causing
> * excessive cache_hot migrations and active balances.
> + *
> + * MISFIT_POWER can also trigger a lot of failed misfit
> + * migrations as we need to ask every CPU to do the pull and
> + * expectedly lots of failures will incur.
> */
> - if (idle != CPU_NEWLY_IDLE)
> + if (idle != CPU_NEWLY_IDLE && env.src_rq->misfit_reason != MISFIT_POWER)
> sd->nr_balance_failed++;
>
> if (need_active_balance(&env)) {
> @@ -11515,8 +11575,11 @@ static int load_balance(int this_cpu, struct rq *this_rq,
> * repeatedly reach this code, which would lead to balance_interval
> * skyrocketing in a short amount of time. Skip the balance_interval
> * increase logic to avoid that.
> + *
> + * So does MISFIT_POWER which asks every CPU to do the pull as we can't
> + * tell which one would be the best one to move to before hand.
> */
> - if (env.idle == CPU_NEWLY_IDLE)
> + if (env.idle == CPU_NEWLY_IDLE || env.src_rq->misfit_reason == MISFIT_POWER)
> goto out;
>
> /* tune up the balancing interval */
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 399b6526afab..3852109ffe62 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -964,6 +964,7 @@ struct balance_callback {
>
> typedef enum misfit_reason {
> MISFIT_PERF, /* Requires moving to a more performant CPU */
> + MISFIT_POWER, /* Requires moving to a more efficient CPU */
> } misfit_reason_t;
>
> /*
Hi Pierre
On 01/04/24 15:28, Pierre Gondois wrote:
> Hello Qais,
>
> I tried to do as you indicated at:
> https://lore.kernel.org/all/20231228233848.piyodw2s2ytli37a@airbuntu/
> without success. I can see that the task is migrated from a big CPU to
> smaller CPUs, but it doesn't seem to be related to the new MISFIT_POWER
> feature.
Hmmm. It is possible something went wrong while preparing these set of patches.
I do remember trying this patch quickly, but judging by the bug you found
I might have missed doing a recent run after a set of changes. So something
could have gotten broken.
Let me retry it and see what's going on.
> Indeed, if the uclamp_max value of a CPU-bound task is set to 0, isn't it
> normal have EAS/feec() migrating the task to smaller CPUs ? I added tracing
> inside is_misfit_task() and load_balance()'s misfit path and could not see
> this path being used.
I did have similar debug messages and I could see them triggered. To be honest
I spent most of the time working on this in the past against 5.10 and 5.15
kernels. And when I started the forward port I already was working on removal
max aggregation and this whole patch needed to be rewritten so I kept it as
a guideline. My focus was on getting the misfit generalization done (patch
1 and 2) and demonstrate how this can be potentially used to implement better
logic to balance based on power.
The main ideas are:
1. We need to detect the MISFIT_POWER.
2. We need to force every CPU to try to pull.
3. We need to use feec() to decide which CPU to pull.
I'm not sure if there's potentially another better way. So I was hoping to see
if there are other PoVs to consider.
>
> On 12/9/23 02:17, Qais Yousef wrote:
> > MISFIT_POWER requires moving the task to a more efficient CPU.
> >
> > This can happen when a big task is capped by uclamp_max, but another
> > task wakes up on this CPU that can lift the capping, in this case we
> > need to migrate it to another, likely smaller, CPU to save power.
>
> Just to be sure, are we talking about the following path, where sugov
> decides which OPP to select ?
> sugov_get_util()
> \-effective_cpu_util()
> \-uclamp_rq_util_with()
>
> To try to describe the issue in my own words, IIUC, the issue comes from
> the fact that during energy estimations in feec(), we don't estimate the
> impact of enqueuing a task on the rq's UCLAMP_MAX value. So a rq with a
> little UCLAMP_MAX value might see the value grows if an uncapped task
> is enqueued, leading to raising the frequency and consuming more
> power.
> Thus, this patch tries to detect such scenario and migrate the clamped
> tasks.
Yes, to a big degree. See below.
> Maybe another approach would be to estimate the impact of enqueuing a
> task on the rq's UCLAMP_MAX value ?
I'd like to think we'll remove rq uclamp value altogether, hopefully.
Generally I'd welcome ideas on what kind of MISFIT_POWER scenarios we have.
With uclamp_max it is the fact that these tasks can be busy loops (their
util_avg is too high) and will cause anything else RUNNING to run at max
frequency regardless of their util_avg. UCLAMP_MAX tells us we have opportunity
to move them somewhere less expensive.
Detection logic will be harder without rq uclamp_max.
My first ever detection logic was actually to check if the task is running on
the smallest fitting CPU; if not then move it to that. Then I switched to
detection based on whether it is capped or not with feec() deciding which is
the best next place to go to.
There's another problem is that these tasks when they end up on a big core,
they'll make the CPU look busy and can prevent other tasks from running
'comfortably' along side it. So not only they waste power, but they're getting
in the way of other work to get their work done with less interference. I'm not
sure if this should be treated as a different type of misfit though.
We need to get MISFIT_POWER in first and then see if the interference issue is
not automatically resolved then. From power perspective, a busy loop but capped
to be able to run on a mid or little, it is wrong to keep it on the big for
extended period of time from power perspective. And by product the interference
issue should be resolved, in theory at least.
Also not sure if we can have non UCLAMP_MAX based MISFIT_POWER. I couldn't come
up with a scenario yet, but I don't think we need to restrict ourselves to
UCLAMP_MAX only ones. So ideas are welcome :-)
Thanks!
--
Qais Yousef