[Devel] [PATCH RH9] loadavg: Fix handling of cfs_rq->nr_unint values

Nikolay Borisov nikolay.borisov at virtuozzo.com
Wed Nov 30 15:29:25 MSK 2022


Our custom nr_unint logic follows the kernel's one about the global
nr_uninterruptible one. One invariant that both share is that values
of nr_unint can go negative when a task is migrated from a CPU.
What's important is that when summed across all task groups they either result
in a positive number (we have sleeping tasks in uninterruptible state) or 0
(no uninterruptible processes).

Current code is broken in that the math in calc_load_ve always deals
with unsigned integers values which leads to enormous values of
'nr_active' when ->nr_unint is negative on a tg. The proper way to work
with nr_unint is to always cast it to an signed integer and also make
nr_active a signed values as well to properly deal with negative values.

Signed-off-by: Nikolay Borisov <nikolay.borisov at virtuozzo.com>
https://jira.sw.ru/browse/PSBM-143355
---
 kernel/sched/loadavg.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/loadavg.c b/kernel/sched/loadavg.c
index 8a5fa39b8b7b..c0c6b89a4958 100644
--- a/kernel/sched/loadavg.c
+++ b/kernel/sched/loadavg.c
@@ -114,7 +114,7 @@ extern raw_spinlock_t load_ve_lock;

 void calc_load_ve(void)
 {
-	unsigned long nr_unint, nr_active;
+	long nr_unint, nr_active;
 	struct task_group *tg;
 	int i;

@@ -134,13 +134,21 @@ void calc_load_ve(void)
 			 * overhead for activate/deactivate operations. So, we
 			 * don't account child cgroup unint tasks here.
 			 */
-			nr_active += tg->cfs_rq[i]->nr_unint;
+			nr_active += (int)tg->cfs_rq[i]->nr_unint;
 #endif
 #ifdef CONFIG_RT_GROUP_SCHED
 			nr_active += tg->rt_rq[i]->rt_nr_running;
 #endif
 		}
-		nr_active *= FIXED_1;
+		/*
+		 * Unless we screwed somewhere, nr_active must always be >= 0, because
+		 * we can have no runnable tasks and no taks in uninterruptible sleep
+		 */
+		if (WARN_ON_ONCE(nr_active < 0)) {
+			nr_active = 0;
+		} else {
+			nr_active *= FIXED_1;
+		}

 		tg->avenrun[0] = calc_load(tg->avenrun[0], EXP_1, nr_active);
 		tg->avenrun[1] = calc_load(tg->avenrun[1], EXP_5, nr_active);
--
2.31.1



More information about the Devel mailing list