Messages in this thread Patch in this message | | | Date | Tue, 27 Feb 2024 10:42:55 +0100 | Subject | Re: [PATCH v6 2/4] sched/fair: Check a task has a fitting cpu when updating misfit | From | Dietmar Eggemann <> |
| |
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@juno:~# cat /sys/devices/system/cpu/cpu*/cpu_capacity 513 1024 1024 513 256 256
root@juno:~# 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() ?
[...]
| |