Re: [PATCH 2/2] sched/fair: Use IRQ scaling for all sched classes

From: Pierre Gondois
Date: Wed Aug 24 2022 - 11:23:12 EST




On 8/23/22 18:26, Vincent Guittot wrote:
On Fri, 19 Aug 2022 at 17:33, Pierre Gondois <pierre.gondois@xxxxxxx> wrote:

The time spent executing IRQ handlers is not reflected in the
utilization of CPU. IRQ scaling reduces rq CFS, RT and DL
util by reflecting the CPU capacity reduction due to IRQs.

commit 9033ea11889f ("cpufreq/schedutil: Take time spent in interrupts
into account")
introduced the notion of IRQ scaling for the now called
effective_cpu_util() function with the following expression (for the
CPU util):
IRQ util_avg + (max_cap - IRQ util_avg / max_cap ) * /Sum rq util_avg

commit 523e979d3164 ("sched/core: Use PELT for scale_rt_capacity()")
introduced IRQ scaling for scale_rt_capacity(), but without scaling
RT and DL rq util.

scale_rt_capacity() excludes RT and DL rq signals from IRQ scaling.
Only the available capacity is scaled. However RT and DL rq util
should also be scaled.

RT and DL are not excluded, they are removed before scaling the
available time. We compute the available cpu capacity in the clock
task domain before scaling it in the full clock domain

Let imagine that we have the following timeshare:

| 100% |
|-IRQ-|-RT--|-----|
| 33% | 33% | 33% |
| 100% |
| 50% | 50% |

irq uses a third of the time

RT uses a third of the time but 50% of the clock task as the time in
interrupt context is not accounted for. This means that the RT
utilization is 50%

In order to know what is available for others we do:

100% - 50% RT = 50% available in the clock task context

Then, we scale the 50% of available time to take the time stolen by IRQ

(100% - 33% IRQ) * 50% / 100% = 33%

So the available capacity for others is 33% of the original capacity
which is correct

What you're proposing:

free = (100% - 33% IRQ) * 100% / 100% = 67%

used = 50% RT + 33% IRQ == 83% whereas it should be 33% RT + 33% IRQ == 66%

Then free < used which means that there is no capacity available for
others which is not true

Ok yes indeed, the scaling is correct as it is.

Thanks for the detailed explanation,
Pierre



Applying IRQ scaling allows to extract the IRQ util avg. So IRQ util
avg should also be subtracted from the available capacity.
Thermal pressure is not execution time but reduces the maximum
possible capacity of a CPU. So IRQ scaling should not be applied.

thermal pressure uses clock_task so it has the same constraint as RT,
DL and CFS signal i.e. irq time is not accounted


Thus, in this order:
- subtract thermal pressure
- apply IRQ scaling on the remaining capacity (RT + DL + CFS + free)
- subtract IRQ util

Also, sort variables in reverse tree order.

Signed-off-by: Pierre Gondois <pierre.gondois@xxxxxxx>
---
kernel/sched/fair.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index bcae7bdd5582..546e490d6753 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8468,16 +8468,23 @@ static inline void init_sd_lb_stats(struct sd_lb_stats *sds)

static unsigned long scale_rt_capacity(int cpu)
{
- struct rq *rq = cpu_rq(cpu);
unsigned long max = arch_scale_cpu_capacity(cpu);
+ struct rq *rq = cpu_rq(cpu);
+ unsigned long irq, thermal;
unsigned long used, free;
- unsigned long irq;

irq = cpu_util_irq(rq);

if (unlikely(irq >= max))
return 1;

+ thermal = thermal_load_avg(rq);
+ if (unlikely(thermal >= max))
+ return 1;
+
+ free = max - thermal;
+ free = scale_irq_capacity(free, irq, max);
+
/*
* avg_rt.util_avg and avg_dl.util_avg track binary signals
* (running and not running) with weights 0 and 1024 respectively.
@@ -8486,14 +8493,12 @@ static unsigned long scale_rt_capacity(int cpu)
*/
used = READ_ONCE(rq->avg_rt.util_avg);
used += READ_ONCE(rq->avg_dl.util_avg);
- used += thermal_load_avg(rq);
+ used += irq;

rq->avg_rt.util_avg and irq==cpu_util_irq(rq)) are not in the same
time scale so you can't add them



- if (unlikely(used >= max))
+ if (unlikely(used >= free))
return 1;

- free = max - used;
-
- return scale_irq_capacity(free, irq, max);
+ return free - used;
}

static void update_cpu_capacity(struct sched_domain *sd, int cpu)
--
2.25.1