sched/pelt: Continue to relax the sync of util_sum with util_avg

Rick reported performance regressions in bugzilla because of cpu frequency
being lower than before:
    https://bugzilla.kernel.org/show_bug.cgi?id=215045

He bisected the problem to:
commit 1c35b07e6d39 ("sched/fair: Ensure _sum and _avg values stay consistent")

This commit forces util_sum to be synced with the new util_avg after
removing the contribution of a task and before the next periodic sync. By
doing so util_sum is rounded to its lower bound and might lost up to
LOAD_AVG_MAX-1 of accumulated contribution which has not yet been
reflected in util_avg.

update_tg_cfs_util() is not the only place where we round util_sum and
lost some accumulated contributions that are not already reflected in
util_avg. Modify update_tg_cfs_util() and detach_entity_load_avg() to not
sync util_sum with the new util_avg. Instead of always setting util_sum to
the low bound of util_avg, which can significantly lower the utilization,
we propagate the difference. In addition, we also check that cfs's util_sum
always stays above the lower bound for a given util_avg as it has been
observed that sched_entity's util_sum is sometimes above cfs one.

Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Tested-by: Sachin Sant <sachinp@linux.ibm.com>
Link: https://lkml.kernel.org/r/20220111134659.24961-3-vincent.guittot@linaro.org

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d8f068d..ad2809c 100644 (file)
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3451,11 +3451,11 @@ void set_task_rq_fair(struct sched_entity *se,
 static inline void
 update_tg_cfs_util(struct cfs_rq *cfs_rq, struct sched_entity *se, struct cfs_rq *gcfs_rq)
 {
-       long delta = gcfs_rq->avg.util_avg - se->avg.util_avg;
-       u32 divider;
+       long delta_sum, delta_avg = gcfs_rq->avg.util_avg - se->avg.util_avg;
+       u32 new_sum, divider;
 
        /* Nothing to update */
-       if (!delta)
+       if (!delta_avg)
                return;
 
        /*
@@ -3464,13 +3464,20 @@ update_tg_cfs_util(struct cfs_rq *cfs_rq, struct sched_entity *se, struct cfs_rq
         */
        divider = get_pelt_divider(&cfs_rq->avg);
 
+
        /* Set new sched_entity's utilization */
        se->avg.util_avg = gcfs_rq->avg.util_avg;
-       se->avg.util_sum = se->avg.util_avg * divider;
+       new_sum = se->avg.util_avg * divider;
+       delta_sum = (long)new_sum - (long)se->avg.util_sum;
+       se->avg.util_sum = new_sum;
 
        /* Update parent cfs_rq utilization */
-       add_positive(&cfs_rq->avg.util_avg, delta);
-       cfs_rq->avg.util_sum = cfs_rq->avg.util_avg * divider;
+       add_positive(&cfs_rq->avg.util_avg, delta_avg);
+       add_positive(&cfs_rq->avg.util_sum, delta_sum);
+
+       /* See update_cfs_rq_load_avg() */
+       cfs_rq->avg.util_sum = max_t(u32, cfs_rq->avg.util_sum,
+                                         cfs_rq->avg.util_avg * PELT_MIN_DIVIDER);
 }
 
 static inline void
@@ -3790,7 +3797,11 @@ static void detach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
 
        dequeue_load_avg(cfs_rq, se);
        sub_positive(&cfs_rq->avg.util_avg, se->avg.util_avg);
-       cfs_rq->avg.util_sum = cfs_rq->avg.util_avg * divider;
+       sub_positive(&cfs_rq->avg.util_sum, se->avg.util_sum);
+       /* See update_cfs_rq_load_avg() */
+       cfs_rq->avg.util_sum = max_t(u32, cfs_rq->avg.util_sum,
+                                         cfs_rq->avg.util_avg * PELT_MIN_DIVIDER);
+
        sub_positive(&cfs_rq->avg.runnable_avg, se->avg.runnable_avg);
        cfs_rq->avg.runnable_sum = cfs_rq->avg.runnable_avg * divider;