[Devel] [PATCH hci-8.1 0/4] sched: restore Container iowait accounting

[Devel] [PATCH hci-8.1 0/4] sched: restore Container iowait accounting

[Vasileios Almpanis]

Container iowait is always 0 on vz10, both in cpu.proc.stat and in the
CT's /proc/stat, so top inside a CT shows 0 wa.

Two things are broken. cfs_rq::nr_iowait goes out of balance on a few
paths, and the group entity never gets sleep_start/block_start, so
neither idle nor iowait is accounted for the group. The counter has to
be fixed first, otherwise the groups would look blocked forever.

Patch 1 fixes the nr_iowait leak on the remote wakeup path.
Patch 2 fixes nr_iowait for throttled and class changing tasks.
Patch 3 accounts sleep and block time of the group entity.
Patch 4 switches a sleeping group between idle and iowait.

The accounting depends on kernel.sched_schedstats=1, same as on vz7.

Known issues:
 - If kernel.sched_schedstats is switched off while a group entity has
   an open sleep or block period, the stamp stays set and cpu.proc.stat
   keeps adding idle or iowait until the group is enqueued again, even
   after schedstats are back on. This can be fixed separately by
   clearing the stamps in set_schedstats().
 - cfs_rq::nr_iowait is not hierarchical, so a CT that runs its IO in
   nested cpu cgroups shows it as idle.

Tested on a vz10 node with direct IO reads in a CT, with CPU and IO
limits, and with class switches, cgroup moves and affinity changes
during IO. fio shows no difference against the base kernel.

https://virtuozzo.atlassian.net/browse/VSTOR-147823

Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
---
Vasileios Almpanis (4):
      sched: fix cfs_rq::nr_iowait leak on the remote wakeup path
      sched: keep cfs_rq::nr_iowait balanced for throttled and class changing tasks
      sched: account sleep and block time of a task group entity
      sched: switch a sleeping task group entity between idle and iowait

 kernel/sched/core.c  | 35 ++++++++++++++++++-----------
 kernel/sched/fair.c  | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 kernel/sched/sched.h | 12 ++++++++++
 3 files changed, 94 insertions(+), 15 deletions(-)
---
base-commit: defa968cbe8c38f1ff0231042dee07f00d27b2c9
change-id: 20261013-ct-iowait-d8e9a32b8135

-- 
Best regards, Vasileios Almpanis
Software Developer, Virtuozzo.

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 1/4] sched: fix cfs_rq::nr_iowait leak on the remote wakeup path

[Vasileios Almpanis]

try_to_wake_up() decrements cfs_rq::nr_iowait only after the
p->on_cpu check. If the task's CPU is still in schedule(), the wakeup
is queued with ttwu_queue_wakelist() and try_to_wake_up() returns
early, so the counter is never decremented. It keeps growing and shows
up as a bogus procs_blocked in the CT's /proc/stat:

  # grep procs_blocked /proc/stat
  procs_blocked 70

With NO_TTWU_QUEUE the leak goes away.

Fix it by doing the decrement where rq::nr_iowait is handled: in
try_to_wake_up() before set_task_cpu() for migrating wakeups, and in
ttwu_do_activate() for all others, which also covers the wakelist.
This way non migrating wakeups don't take the rq lock for it anymore.

A task on the wakelist is TASK_WAKING but still counted, so add
task_in_nr_iowait() and use it in sched_move_task() instead of
skipping every TASK_WAKING task.

https://virtuozzo.atlassian.net/browse/VSTOR-147823
Fixes: 415fe063c53e ("sched: Account cfs_rq::nr_iowait")
Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

Feature: sched: enhance task states granularity and accounting
---
 kernel/sched/core.c  | 26 +++++++++++++-------------
 kernel/sched/sched.h | 12 ++++++++++++
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9275501bc3ad..dc016d264d19 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3775,6 +3775,8 @@ ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
 	if (p->in_iowait) {
 		delayacct_blkio_end(p);
 		atomic_dec(&task_rq(p)->nr_iowait);
+		if (p->sched_class->nr_iowait_dec)
+			p->sched_class->nr_iowait_dec(p);
 	}
 
 	if (p->sched_interruptible_sleep)
@@ -4355,20 +4357,20 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 		 */
 		smp_cond_load_acquire(&p->on_cpu, !VAL);
 
-		if (p->in_iowait && p->sched_class->nr_iowait_dec) {
-			struct rq_flags rf;
-			struct rq *rq;
-
-			rq = __task_rq_lock(p, &rf);
-			p->sched_class->nr_iowait_dec(p);
-			__task_rq_unlock(rq, &rf);
-		}
-
 		cpu = select_task_rq(p, p->wake_cpu, &wake_flags);
 		if (task_cpu(p) != cpu) {
 			if (p->in_iowait) {
 				delayacct_blkio_end(p);
 				atomic_dec(&task_rq(p)->nr_iowait);
+				/* On the old CPU, see also ttwu_do_activate() */
+				if (p->sched_class->nr_iowait_dec) {
+					struct rq_flags rf;
+					struct rq *rq;
+
+					rq = __task_rq_lock(p, &rf);
+					p->sched_class->nr_iowait_dec(p);
+					__task_rq_unlock(rq, &rf);
+				}
 			}
 
 			wake_flags |= WF_MIGRATED;
@@ -9139,8 +9141,7 @@ void sched_move_task(struct task_struct *tsk, bool for_autogroup)
 	if (queued)
 		dequeue_task(rq, tsk, queue_flags);
 	else {
-		if (!(READ_ONCE(tsk->__state) ==  TASK_WAKING) && tsk->in_iowait &&
-		    tsk->sched_class->nr_iowait_dec)
+		if (task_in_nr_iowait(tsk) && tsk->sched_class->nr_iowait_dec)
 			tsk->sched_class->nr_iowait_dec(tsk);
 
 		if (tsk->sched_contributes_to_load)
@@ -9159,8 +9160,7 @@ void sched_move_task(struct task_struct *tsk, bool for_autogroup)
 	if (queued)
 		enqueue_task(rq, tsk, queue_flags);
 	else {
-		if (!(READ_ONCE(tsk->__state) ==  TASK_WAKING) && tsk->in_iowait &&
-		    tsk->sched_class->nr_iowait_inc)
+		if (task_in_nr_iowait(tsk) && tsk->sched_class->nr_iowait_inc)
 			tsk->sched_class->nr_iowait_inc(tsk);
 
 		if (tsk->sched_contributes_to_load)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 2a3f73933456..176077ba4201 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2337,6 +2337,18 @@ static inline int task_on_rq_queued(struct task_struct *p)
 	return READ_ONCE(p->on_rq) == TASK_ON_RQ_QUEUED;
 }
 
+/*
+ * A sleeping iowait task stays counted in nr_iowait of its sched class until
+ * it is woken up: a migrating wakeup drops it in try_to_wake_up(), any other
+ * in ttwu_do_activate(), which for a wakelist wakeup runs later while the
+ * task is TASK_WAKING. Called with p->pi_lock and the task's rq lock held.
+ */
+static inline bool task_in_nr_iowait(struct task_struct *p)
+{
+	return p->in_iowait && !task_on_rq_queued(p) &&
+	       (READ_ONCE(p->__state) != TASK_WAKING || !p->sched_remote_wakeup);
+}
+
 static inline int task_on_rq_migrating(struct task_struct *p)
 {
 	return READ_ONCE(p->on_rq) == TASK_ON_RQ_MIGRATING;

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 2/4] sched: keep cfs_rq::nr_iowait balanced for throttled and class changing tasks

[Vasileios Almpanis]

Two more paths break the cfs_rq::nr_iowait pairing:

 - A throttled task can go to sleep in iowait before it returns to
   user space, e.g. in blkcg_maybe_throttle_current().
   dequeue_throttled_task() doesn't increment the counter but the
   wakeup decrements it, so it wraps around.
 - A sleeping iowait task can change its class through
   sched_setscheduler() or rt_mutex_setprio(). Fair to RT leaves the
   counter incremented forever, RT to fair decrements it without an
   increment.

A CT with both CPU and IO limits hits the first one.

Fix it by incrementing the counter in dequeue_throttled_task() and by
moving the task between the class counters in check_class_changed(),
like sched_move_task() does for a cgroup change.

https://virtuozzo.atlassian.net/browse/VSTOR-147823
Fixes: 415fe063c53e ("sched: Account cfs_rq::nr_iowait")
Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

Feature: sched: enhance task states granularity and accounting
---
 kernel/sched/core.c | 8 ++++++++
 kernel/sched/fair.c | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index dc016d264d19..1acb98c0e8fa 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2271,6 +2271,14 @@ void check_class_changed(struct rq *rq, struct task_struct *p,
 			 int oldprio)
 {
 	if (prev_class != p->sched_class) {
+		/* Move a sleeping iowait task to the new class accounting */
+		if (task_in_nr_iowait(p)) {
+			if (prev_class->nr_iowait_dec)
+				prev_class->nr_iowait_dec(p);
+			if (p->sched_class->nr_iowait_inc)
+				p->sched_class->nr_iowait_inc(p);
+		}
+
 		if (prev_class->switched_from)
 			prev_class->switched_from(rq, p);
 
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 5dd350a0e08b..df296edb828d 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5834,6 +5834,8 @@ static void dequeue_throttled_task(struct task_struct *p, int flags)
 	/* task blocked after throttled */
 	if (flags & DEQUEUE_SLEEP) {
 		p->throttled = false;
+		if (p->in_iowait)
+			task_cfs_rq(p)->nr_iowait++;
 		return;
 	}
 

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 3/4] sched: account sleep and block time of a task group entity

[Vasileios Almpanis]

CT idle and iowait in cpu.proc.stat come from the schedstats of the
group entity: cpu_cgroup_update_stat() reads iowait_sum and
sum_sleep_runtime. But only tasks get sleep_start/block_start, so for
a group both stay 0 and the CT shows 0 idle and 0 iowait.
fixup_vcpustat_delta() then gives all idle time of the CT's /proc/stat
to IDLE, so iowait there is always 0 too.

vz7 dequeue_sleeper() had a branch for group entities that was lost
when porting to newer kernels. Restore it in update_stats_dequeue_fair():
when a group entity is dequeued on sleep, start a block period if its
cfs_rq has iowait sleepers and a sleep period otherwise. The existing
enqueue path closes it.

Like on vz7, skip throttled groups, they are not idle, just waiting
for bandwidth. If a group has nothing to run when it is unthrottled,
tg_unthrottle_up() doesn't enqueue it, so start its period there.

https://virtuozzo.atlassian.net/browse/VSTOR-147823
Fixes: 095994c57b84 ("ve/sched/stat: Introduce functions to calculate vcpustat data")
Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

Feature: sched: emulate virtual cpus for Containers
---
 kernel/sched/fair.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index df296edb828d..2df5a9b47480 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1332,6 +1332,21 @@ update_stats_enqueue_fair(struct cfs_rq *cfs_rq, struct sched_entity *se, int fl
 		update_stats_enqueue_sleeper_fair(cfs_rq, se);
 }
 
+static inline int throttled_hierarchy(struct cfs_rq *cfs_rq);
+
+/* The group goes idle, blocked if it has iowait sleepers */
+static inline void
+update_stats_group_sleep_start(struct cfs_rq *cfs_rq, struct sched_entity *se)
+{
+	struct sched_statistics *stats;
+
+	stats = __schedstats_from_se(se);
+	if (group_cfs_rq(se)->nr_iowait)
+		__schedstat_set(stats->block_start, rq_clock(rq_of(cfs_rq)));
+	else
+		__schedstat_set(stats->sleep_start, rq_clock(rq_of(cfs_rq)));
+}
+
 static inline void
 update_stats_dequeue_fair(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
 {
@@ -1358,6 +1373,9 @@ update_stats_dequeue_fair(struct cfs_rq *cfs_rq, struct sched_entity *se, int fl
 		if (state & TASK_UNINTERRUPTIBLE)
 			__schedstat_set(tsk->stats.block_start,
 				      rq_clock(rq_of(cfs_rq)));
+	} else if ((flags & DEQUEUE_SLEEP) &&
+		   !throttled_hierarchy(group_cfs_rq(se))) {
+		update_stats_group_sleep_start(cfs_rq, se);
 	}
 }
 
@@ -3825,8 +3843,6 @@ static void reweight_task_fair(struct rq *rq, struct task_struct *p,
 	load->inv_weight = lw->inv_weight;
 }
 
-static inline int throttled_hierarchy(struct cfs_rq *cfs_rq);
-
 #ifdef CONFIG_FAIR_GROUP_SCHED
 /*
  * All this does is approximate the hierarchical proportion which includes that
@@ -5904,6 +5920,7 @@ static int tg_unthrottle_up(struct task_group *tg, void *data)
 {
 	struct rq *rq = data;
 	struct cfs_rq *cfs_rq = tg->cfs_rq[cpu_of(rq)];
+	struct sched_entity *se = tg->se[cpu_of(rq)];
 	struct task_struct *p, *tmp;
 
 	if (--cfs_rq->throttle_count)
@@ -5933,6 +5950,15 @@ static int tg_unthrottle_up(struct task_group *tg, void *data)
 		enqueue_task_fair(rq_of(cfs_rq), p, ENQUEUE_WAKEUP);
 	}
 
+	/*
+	 * The group may have gone idle while throttled, when no sleep or
+	 * block period is started for it. Start it now if nothing runs.
+	 */
+	if (schedstat_enabled() && se && !se->on_rq && !cfs_rq->nr_queued &&
+	    !schedstat_val(__schedstats_from_se(se)->sleep_start) &&
+	    !schedstat_val(__schedstats_from_se(se)->block_start))
+		update_stats_group_sleep_start(cfs_rq_of(se), se);
+
 	/* Add cfs_rq with load or one or more already running entities to the list */
 	if (!cfs_rq_is_decayed(cfs_rq))
 		list_add_leaf_cfs_rq(cfs_rq);

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 4/4] sched: switch a sleeping task group entity between idle and iowait

[Vasileios Almpanis]

A sleeping group entity is accounted as blocked if its cfs_rq had
iowait sleepers when it was dequeued and as idle otherwise. But
nr_iowait can change while the group sleeps, e.g. when its iowait task
wakes up on another CPU, moves to another cgroup or changes its class.
The group then keeps accounting iowait on a CPU where nothing waits
for IO anymore.

Fix it like vz7 nr_iowait_{inc,dec}_fair() did: when nr_iowait changes,
close the current period of the sleeping group and start the other
kind. This needs an up to date rq clock, so update it in the migrating
wakeup path of try_to_wake_up(). The other callers already do it.

https://virtuozzo.atlassian.net/browse/VSTOR-147823
Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

Feature: sched: emulate virtual cpus for Containers
---
 kernel/sched/core.c |  1 +
 kernel/sched/fair.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 1acb98c0e8fa..77587e9982b7 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4376,6 +4376,7 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 					struct rq *rq;
 
 					rq = __task_rq_lock(p, &rf);
+					update_rq_clock(rq);
 					p->sched_class->nr_iowait_dec(p);
 					__task_rq_unlock(rq, &rf);
 				}
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 2df5a9b47480..9f76caefc64a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -13694,11 +13694,40 @@ static unsigned int get_rr_interval_fair(struct rq *rq, struct task_struct *task
 }
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
+/*
+ * A sleeping group entity is accounted as blocked while its cfs_rq has
+ * iowait sleepers and as idle otherwise. Switch it when nr_iowait changes
+ * without an enqueue, e.g. the iowait task wakes up on another CPU.
+ */
+static void nr_iowait_update_group(struct cfs_rq *cfs_rq,
+				   struct sched_entity *se)
+{
+	struct sched_statistics *stats;
+	struct rq *rq;
+
+	if (!schedstat_enabled() || !se)
+		return;
+
+	rq = rq_of(cfs_rq);
+	stats = __schedstats_from_se(se);
+	if (cfs_rq->nr_iowait ? !schedstat_val(stats->sleep_start) :
+				!schedstat_val(stats->block_start))
+		return;
+
+	__update_stats_enqueue_sleeper(rq, NULL, stats);
+
+	if (cfs_rq->nr_iowait)
+		__schedstat_set(stats->block_start, rq_clock(rq));
+	else
+		__schedstat_set(stats->sleep_start, rq_clock(rq));
+}
+
 static void nr_iowait_dec_fair(struct task_struct *p)
 {
 	struct cfs_rq *cfs_rq = task_cfs_rq(p);
 
 	cfs_rq->nr_iowait--;
+	nr_iowait_update_group(cfs_rq, p->se.parent);
 }
 
 static void nr_iowait_inc_fair(struct task_struct *p)
@@ -13706,6 +13735,7 @@ static void nr_iowait_inc_fair(struct task_struct *p)
 	struct cfs_rq *cfs_rq = task_cfs_rq(p);
 
 	cfs_rq->nr_iowait++;
+	nr_iowait_update_group(cfs_rq, p->se.parent);
 }
 #endif /* CONFIG_FAIR_GROUP_SCHED */
 

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 1/4] sched: fix cfs_rq::nr_iowait leak on the remote wakeup path

[no-reply]

> try_to_wake_up() decrements cfs_rq::nr_iowait only after the
> p->on_cpu check. If the task's CPU is still in schedule(), the wakeup
> is queued with ttwu_queue_wakelist() and try_to_wake_up() returns
> early, so the counter is never decremented. It keeps growing and shows
> up as a bogus procs_blocked in the CT's /proc/stat:
> 
>   # grep procs_blocked /proc/stat
>   procs_blocked 70
> 
> With NO_TTWU_QUEUE the leak goes away.
> 
> Fix it by doing the decrement where rq::nr_iowait is handled: in
> try_to_wake_up() before set_task_cpu() for migrating wakeups, and in
> ttwu_do_activate() for all others, which also covers the wakelist.
> This way non migrating wakeups don't take the rq lock for it anymore.
> 
> A task on the wakelist is TASK_WAKING but still counted, so add
> task_in_nr_iowait() and use it in sched_move_task() instead of
> skipping every TASK_WAKING task.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-147823
> Fixes: 415fe063c53e ("sched: Account cfs_rq::nr_iowait")
> Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-ct-iowait-v1-0-9d65fe37cf89@virtuozzo.com?part=1


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 2/4] sched: keep cfs_rq::nr_iowait balanced for throttled and class changing tasks

[no-reply]

> Two more paths break the cfs_rq::nr_iowait pairing:
> 
>  - A throttled task can go to sleep in iowait before it returns to
>    user space, e.g. in blkcg_maybe_throttle_current().
>    dequeue_throttled_task() doesn't increment the counter but the
>    wakeup decrements it, so it wraps around.
>  - A sleeping iowait task can change its class through
>    sched_setscheduler() or rt_mutex_setprio(). Fair to RT leaves the
>    counter incremented forever, RT to fair decrements it without an
>    increment.
> 
> A CT with both CPU and IO limits hits the first one.
> 
> Fix it by incrementing the counter in dequeue_throttled_task() and by
> moving the task between the class counters in check_class_changed(),
> like sched_move_task() does for a cgroup change.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-147823
> Fixes: 415fe063c53e ("sched: Account cfs_rq::nr_iowait")
> Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-ct-iowait-v1-0-9d65fe37cf89@virtuozzo.com?part=2


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 4/4] sched: switch a sleeping task group entity between idle and iowait

[no-reply]

> A sleeping group entity is accounted as blocked if its cfs_rq had
> iowait sleepers when it was dequeued and as idle otherwise. But
> nr_iowait can change while the group sleeps, e.g. when its iowait task
> wakes up on another CPU, moves to another cgroup or changes its class.
> The group then keeps accounting iowait on a CPU where nothing waits
> for IO anymore.
> 
> Fix it like vz7 nr_iowait_{inc,dec}_fair() did: when nr_iowait changes,
> close the current period of the sleeping group and start the other
> kind. This needs an up to date rq clock, so update it in the migrating
> wakeup path of try_to_wake_up(). The other callers already do it.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-147823
> Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-ct-iowait-v1-0-9d65fe37cf89@virtuozzo.com?part=4


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 3/4] sched: account sleep and block time of a task group entity

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] sched: group iowait/idle classification uses non-hierarchical cfs_rq->nr_iowait
--

commit b026bcc4d4882c5ba1716b8736a28e5f90a38798
Author: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

sched: account sleep and block time of a task group entity

This commit makes a group sched_entity start a sleep or block period when
it is dequeued with DEQUEUE_SLEEP. It chooses block if the group's cfs_rq
has iowait sleepers, and sleep otherwise. It also starts the period from
tg_unthrottle_up() when an unthrottled group has nothing to run.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-147823

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c

[ ... ]

> +/* The group goes idle, blocked if it has iowait sleepers */
> +static inline void
> +update_stats_group_sleep_start(struct cfs_rq *cfs_rq, struct sched_entity *se)
> +{
> +	struct sched_statistics *stats;
> +
> +	stats = __schedstats_from_se(se);
> +	if (group_cfs_rq(se)->nr_iowait)
> +		__schedstat_set(stats->block_start, rq_clock(rq_of(cfs_rq)));
> +	else
> +		__schedstat_set(stats->sleep_start, rq_clock(rq_of(cfs_rq)));
> +}

[Severity: Medium]
Is the classification here hierarchical? group_cfs_rq(se)->nr_iowait is
the counter of that group's own cfs_rq only.

dequeue_entity() increments nr_iowait only on the task's immediate cfs_rq:

    if ((flags & DEQUEUE_SLEEP) && entity_is_task(se)) {
        struct task_struct *tsk = task_of(se);

        if (tsk->in_iowait)
            cfs_rq->nr_iowait++;

Consider a task with in_iowait set that sleeps in a sub-cgroup S of a
container CT:

    dequeue_entity(task)
        S->cfs_rq->nr_iowait++
    dequeue_entity(S group entity, DEQUEUE_SLEEP)
        update_stats_group_sleep_start()
            S->nr_iowait != 0, so block_start is set
    dequeue_entity(CT group entity, DEQUEUE_SLEEP)
        update_stats_group_sleep_start()
            CT->nr_iowait == 0, so sleep_start is set

cpu_cgroup_update_stat() then reads iowait_sum and sum_sleep_runtime from
the CT group entity, so the time appears as idle and not as iowait.

The follow-up patch "sched: switch a sleeping task group entity between
idle and iowait" (5de73903cb61) adds nr_iowait_update_group(), called from
nr_iowait_inc_fair() and nr_iowait_dec_fair(). It re-evaluates only
p->se.parent, the immediate parent group entity, and the increments and
decrements still stay on the task's direct cfs_rq.

So even with the whole series applied, it looks like the CT entity stays
misclassified when the iowait sleepers are in nested cgroups, and the
stated goal is met only for a flat hierarchy. The same non-hierarchical
counter is read per task_group in cpuacct.c (tg_nr_iowait).

Should the iowait state be propagated to, or evaluated across, all
ancestor group entities?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-ct-iowait-v1-0-9d65fe37cf89@virtuozzo.com?part=3

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 3/4] sched: account sleep and block time of a task group entity

[Vasileios Almpanis]

On 10/14/26 1:28 PM, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] sched: group iowait/idle classification uses non-hierarchical cfs_rq->nr_iowait
> --
>
> commit b026bcc4d4882c5ba1716b8736a28e5f90a38798
> Author: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
>
> sched: account sleep and block time of a task group entity
>
> This commit makes a group sched_entity start a sleep or block period when
> it is dequeued with DEQUEUE_SLEEP. It chooses block if the group's cfs_rq
> has iowait sleepers, and sleep otherwise. It also starts the period from
> tg_unthrottle_up() when an unthrottled group has nothing to run.
>
> Link: https://virtuozzo.atlassian.net/browse/VSTOR-147823
>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
> [ ... ]
>
>> +/* The group goes idle, blocked if it has iowait sleepers */
>> +static inline void
>> +update_stats_group_sleep_start(struct cfs_rq *cfs_rq, struct sched_entity *se)
>> +{
>> +	struct sched_statistics *stats;
>> +
>> +	stats = __schedstats_from_se(se);
>> +	if (group_cfs_rq(se)->nr_iowait)
>> +		__schedstat_set(stats->block_start, rq_clock(rq_of(cfs_rq)));
>> +	else
>> +		__schedstat_set(stats->sleep_start, rq_clock(rq_of(cfs_rq)));
>> +}
> [Severity: Medium]
> Is the classification here hierarchical? group_cfs_rq(se)->nr_iowait is
> the counter of that group's own cfs_rq only.
>
> dequeue_entity() increments nr_iowait only on the task's immediate cfs_rq:
>
>      if ((flags & DEQUEUE_SLEEP) && entity_is_task(se)) {
>          struct task_struct *tsk = task_of(se);
>
>          if (tsk->in_iowait)
>              cfs_rq->nr_iowait++;
>
> Consider a task with in_iowait set that sleeps in a sub-cgroup S of a
> container CT:
>
>      dequeue_entity(task)
>          S->cfs_rq->nr_iowait++
>      dequeue_entity(S group entity, DEQUEUE_SLEEP)
>          update_stats_group_sleep_start()
>              S->nr_iowait != 0, so block_start is set
>      dequeue_entity(CT group entity, DEQUEUE_SLEEP)
>          update_stats_group_sleep_start()
>              CT->nr_iowait == 0, so sleep_start is set
>
> cpu_cgroup_update_stat() then reads iowait_sum and sum_sleep_runtime from
> the CT group entity, so the time appears as idle and not as iowait.
>
> The follow-up patch "sched: switch a sleeping task group entity between
> idle and iowait" (5de73903cb61) adds nr_iowait_update_group(), called from
> nr_iowait_inc_fair() and nr_iowait_dec_fair(). It re-evaluates only
> p->se.parent, the immediate parent group entity, and the increments and
> decrements still stay on the task's direct cfs_rq.
>
> So even with the whole series applied, it looks like the CT entity stays
> misclassified when the iowait sleepers are in nested cgroups, and the
> stated goal is met only for a flat hierarchy. The same non-hierarchical
> counter is read per task_group in cpuacct.c (tg_nr_iowait).
>
> Should the iowait state be propagated to, or evaluated across, all
> ancestor group entities?
Yes, it is not hierarchical and I have already written about it in the 
cover letter. cfs_rq:nr_iowait counts only the tasks sleeping on that 
cfs_rq, so with nested cpu cgroups inside the CT the CT group entity is 
accounted as idle instead of iowait.

Its the same with vz7. This series restores that behavior, it doesn't 
change the previous model. If we want to make it hierarchical I would 
rather do it in a separate ticket if there is a real need for it.

-- 
Best regards, Vasileios Almpanis
Software Developer, Virtuozzo.


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 0/4] sched: restore Container iowait accounting

[Konstantin Khorenko]

On Wed, 14 Oct 2026 11:16:23 +0000, Vasileios Almpanis wrote:
> sched: restore Container iowait accounting
> 
> Container iowait is always 0 on vz10, both in cpu.proc.stat and in the
> CT's /proc/stat, so top inside a CT shows 0 wa.
> 
> Two things are broken. cfs_rq::nr_iowait goes out of balance on a few
> paths, and the group entity never gets sleep_start/block_start, so
> neither idle nor iowait is accounted for the group. The counter has to
> be fixed first, otherwise the groups would look blocked forever.
> 
> [...]

Applied to branch-rh10-6.12.0-211.52.1.22.x.vz10-ovz, will be released in rh10-6.12.0-211.52.1.22.9.vz10, thanks!

[1/4] sched: fix cfs_rq::nr_iowait leak on the remote wakeup path
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/d9432bac2620
[2/4] sched: keep cfs_rq::nr_iowait balanced for throttled and class changing tasks
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/df90c35e9848
[3/4] sched: account sleep and block time of a task group entity
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/544bf5f61fc1
[4/4] sched: switch a sleeping task group entity between idle and iowait
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/ed6900fdc06c

-- 
Konstantin Khorenko <khorenko@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 0/4] sched: restore Container iowait accounting

[Konstantin Khorenko]

On 10/14/26 13:16, Vasileios Almpanis wrote:
> Container iowait is always 0 on vz10, both in cpu.proc.stat and in the
> CT's /proc/stat, so top inside a CT shows 0 wa.
> 
> Two things are broken. cfs_rq::nr_iowait goes out of balance on a few
> paths, and the group entity never gets sleep_start/block_start, so
> neither idle nor iowait is accounted for the group. The counter has to
> be fixed first, otherwise the groups would look blocked forever.
> 
> Patch 1 fixes the nr_iowait leak on the remote wakeup path.
> Patch 2 fixes nr_iowait for throttled and class changing tasks.
> Patch 3 accounts sleep and block time of the group entity.
> Patch 4 switches a sleeping group between idle and iowait.
> 
> The accounting depends on kernel.sched_schedstats=1, same as on vz7.
> 
> Known issues:
>  - If kernel.sched_schedstats is switched off while a group entity has
>    an open sleep or block period, the stamp stays set and cpu.proc.stat
>    keeps adding idle or iowait until the group is enqueued again, even
>    after schedstats are back on. This can be fixed separately by
>    clearing the stamps in set_schedstats().
>  - cfs_rq::nr_iowait is not hierarchical, so a CT that runs its IO in
>    nested cpu cgroups shows it as idle.

i have pushed the patchset, it looks ok.

But i really think we better address both known issues mentioned above in a separate series (or several series),
i think they are important.

+ can you please also write a selftest for this functionality?


> 
> Tested on a vz10 node with direct IO reads in a CT, with CPU and IO
> limits, and with class switches, cgroup moves and affinity changes
> during IO. fio shows no difference against the base kernel.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-147823
> 
> Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
> ---
> Vasileios Almpanis (4):
>       sched: fix cfs_rq::nr_iowait leak on the remote wakeup path
>       sched: keep cfs_rq::nr_iowait balanced for throttled and class changing tasks
>       sched: account sleep and block time of a task group entity
>       sched: switch a sleeping task group entity between idle and iowait
> 
>  kernel/sched/core.c  | 35 ++++++++++++++++++-----------
>  kernel/sched/fair.c  | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  kernel/sched/sched.h | 12 ++++++++++
>  3 files changed, 94 insertions(+), 15 deletions(-)
> ---
> base-commit: defa968cbe8c38f1ff0231042dee07f00d27b2c9
> change-id: 20261013-ct-iowait-d8e9a32b8135
> 

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel