[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d6699c3a-3df6-46a3-98db-e07c8722f106@arm.com>
Date: Tue, 27 Feb 2024 10:42:55 +0100
From: Dietmar Eggemann <dietmar.eggemann@....com>
To: Qais Yousef <qyousef@...alina.io>, Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Vincent Guittot <vincent.guittot@...aro.org>
Cc: linux-kernel@...r.kernel.org, Pierre Gondois <Pierre.Gondois@....com>
Subject: Re: [PATCH v6 2/4] sched/fair: Check a task has a fitting cpu when
updating misfit
On 20/02/2024 23:56, Qais Yousef wrote:
> If a misfit task is affined to a subset of the possible cpus, we need to
> verify that one of these cpus can fit it. Otherwise the load balancer
> code will continuously trigger needlessly leading the balance_interval
> to increase in return and eventually end up with a situation where real
> imbalances take a long time to address because of this impossible
> imbalance situation.
>
> This can happen in Android world where it's common for background tasks
> to be restricted to little cores.
>
> Similarly if we can't fit the biggest core, triggering misfit is
> pointless as it is the best we can ever get on this system.
>
> To be able to detect that; we use asym_cap_list to iterate through
> capacities in the system to see if the task is able to run at a higher
> capacity level based on its p->cpus_ptr. We do that when the affinity
> change, a fair task is forked, or when a task switched to fair policy.
> We store the max_allowed_capacity in task_struct to allow for cheap
> comparison in the fast path.
>
> Improve check_misfit_status() function by removing redundant checks.
> misfit_task_load will be 0 if the task can't move to a bigger CPU. And
> nohz_load_balance() already checks for cpu_check_capacity() before
s/nohz_load_balance()/nohz_balancer_kick() ?
> calling check_misfit_status().
Isn't there an issue with CPU hotplug.
On a tri-geared Juno:
root@...o:~# cat /sys/devices/system/cpu/cpu*/cpu_capacity
513
1024
1024
513
256
256
root@...o:~# taskset -pc 0,3-5 $$
[ 108.248425] set_task_max_allowed_capacity() [bash 1636]
max_allowed_capacity=513 nr_cpus_allowed=4 cpus_mask=0,3-5
echo 0 > /sys//devices/system/cpu/cpu0/online
echo 0 > /sys//devices/system/cpu/cpu3/online
[ 134.136887] set_task_max_allowed_capacity() [bash 1639]
max_allowed_capacity=513 nr_cpus_allowed=4 cpus_mask=0,3-5
Cpuset seems to be fine since it set task's cpumask.
[...]
> +/*
> + * Check the max capacity the task is allowed to run at for misfit detection.
Nitpick: It's rather a setter function so s/check/set here ?
> + */
> +static void set_task_max_allowed_capacity(struct task_struct *p)
> +{
> + struct asym_cap_data *entry;
> +
> + if (!sched_asym_cpucap_active())
> + return;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(entry, &asym_cap_list, link) {
> + cpumask_t *cpumask;
> +
> + cpumask = cpu_capacity_span(entry);
> + if (!cpumask_intersects(p->cpus_ptr, cpumask))
> + continue;
> +
> + p->max_allowed_capacity = entry->capacity;
> + break;
> + }
> + rcu_read_unlock();
> +}
[...]
> @@ -9601,16 +9644,10 @@ check_cpu_capacity(struct rq *rq, struct sched_domain *sd)
> (arch_scale_cpu_capacity(cpu_of(rq)) * 100));
> }
>
> -/*
> - * Check whether a rq has a misfit task and if it looks like we can actually
> - * help that task: we can migrate the task to a CPU of higher capacity, or
> - * the task's current CPU is heavily pressured.
> - */
> -static inline int check_misfit_status(struct rq *rq, struct sched_domain *sd)
> +/* Check if the rq has a misfit task */
> +static inline bool check_misfit_status(struct rq *rq, struct sched_domain *sd)
`struct sched_domain *sd` is not needed anymore.
Since there is only 1 user of check_misfit_status() you might remove it
entirely and use `rq->rq->misfit_task_load` directly in
nohz_balancer_kick() ?
[...]
Powered by blists - more mailing lists