2023-09-05 16:04:15

by Dietmar Eggemann

[permalink] [raw]
Subject: Re: [PATCH] sched/fair: Check a task has a fitting cpu when updating misfit

On 29/08/2023 17:35, Qais Yousef wrote:
> On 08/29/23 16:10, Vincent Guittot wrote:
>> On Sun, 20 Aug 2023 at 22:34, Qais Yousef <[email protected]> wrote:

[...]

>>> kernel/sched/fair.c | 50 ++++++++++++++++++++++++++++++++++++++-------
>>> 1 file changed, 43 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>> index 0b7445cd5af9..f08c5f3bf895 100644
>>> --- a/kernel/sched/fair.c
>>> +++ b/kernel/sched/fair.c
>>> @@ -4853,17 +4853,50 @@ static inline int task_fits_cpu(struct task_struct *p, int cpu)
>>>
>>> static inline void update_misfit_status(struct task_struct *p, struct rq *rq)
>>> {
>>> + unsigned long uclamp_min, uclamp_max;
>>> + unsigned long util, cap_level;
>>> + bool has_fitting_cpu = false;
>>> + int cpu = cpu_of(rq);
>>> +
>>> if (!sched_asym_cpucap_active())
>>> return;
>>>
>>> - if (!p || p->nr_cpus_allowed == 1) {
>>> - rq->misfit_task_load = 0;
>>> - return;
>>> - }
>>> + if (!p || p->nr_cpus_allowed == 1)
>>> + goto out;
>>>
>>> - if (task_fits_cpu(p, cpu_of(rq))) {
>>> - rq->misfit_task_load = 0;
>>> - return;
>>> + uclamp_min = uclamp_eff_value(p, UCLAMP_MIN);
>>> + uclamp_max = uclamp_eff_value(p, UCLAMP_MAX);
>>> + util = task_util_est(p);
>>> +
>>> + if (util_fits_cpu(util, uclamp_min, uclamp_max, cpu) > 0)
>>> + goto out;

util_fits_cpu() checks fits_capacity(util, capacity_of(cpu)) but the
capacity pressure could change between update_misfit_status() and CFS lb?

>>> +
>>> + cap_level = capacity_orig_of(cpu);
>>> +
>>> + /* If we can't fit the biggest CPU, that's the best we can ever get. */
>>> + if (cap_level == SCHED_CAPACITY_SCALE)
>>> + goto out;
>>> +
>>> + /*
>>> + * If the task affinity is not set to default, make sure it is not
>>> + * restricted to a subset where no CPU can ever fit it. Triggering
>>> + * misfit in this case is pointless as it has no where better to move
>>> + * to. And it can lead to balance_interval to grow too high as we'll
>>> + * continuously fail to move it anywhere.
>>> + */
>>> + if (!cpumask_equal(p->cpus_ptr, cpu_possible_mask)) {
>>> + for_each_cpu(cpu, p->cpus_ptr) {
>>
>> I haven't looked at the problem in detail and at other possibilities
>> so far but for_each_cpu doesn't scale and update_misfit_status() being
>> called in pick_next_task_fair() so you must find another way to detect
>> this
>
> Okay, will do.

We have LIST_HEAD(asym_cap_list) (list of cpumasks according to
cpu_capacity_orig CPU groups) in kernel/sched/topology.c to set
SD_ASYM_CPUCAPACITY{,_FULL} for asymmetric CPU capacity systems.
Maybe this could be made usable in fair.c as well?

But checking via util_fits_cpu() wouldn't work then since it's per-CPU.
The check of p's CPU affinity, its uclamped util_avg value and the
cpu_capacity_orig is sufficient here. Then using those cpumasks could work.