Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH] vhost: Run TIF_NOTIFY_RESUME work in the vhost_task loop
@ 2026-10-31  1:33 Aamir Ahmed
  2026-10-31 19:05 ` Shakeel Butt
  2026-11-01  1:33 ` sashiko-bot
  0 siblings, 2 replies; 3+ messages in thread
From: Aamir Ahmed @ 2026-10-31  1:33 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang, Eugenio Pérez
  Cc: Valentin Schneider, Aaron Lu, Peter Zijlstra, kvm, virtualization,
	netdev, linux-kernel, Aamir Ahmed, stable

Since commit e1fad12dcb66 ("sched/fair: Switch to task based throttle
model"), a throttled task is only dequeued when TWA_RESUME task_work
runs on its way back to user mode.

vhost workers never return to user mode, and vhost_task_fn() only runs
task_work via get_signal() when a signal is pending. A busy vhost worker
therefore ignores its cgroup's cpu.max, while its runtime is still
charged to the group, throttling the group's other tasks (typically the
VM's vCPUs) harder.

Run pending notify-resume work at the top of the worker loop, as io-wq
does in io_run_task_work(). Commit 12013e3d4695 ("KVM: powerpc: Use
generic xfer to guest work function") fixed the same problem for KVM.

Fixes: e1fad12dcb66 ("sched/fair: Switch to task based throttle model")
Cc: stable@vger.kernel.org # 6.18+
Assisted-by: LLM
Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
---
Tested on x86_64 with a vhost_task-based reproducer: in a 0.2-CPU cgroup
a busy worker drops from 100% to 19% of a CPU. Reproducer available on
request.

 kernel/vhost_task.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/vhost_task.c b/kernel/vhost_task.c
index 3717885a599..7fa96e1b85b 100644
--- a/kernel/vhost_task.c
+++ b/kernel/vhost_task.c
@@ -6,6 +6,7 @@
 #include <linux/completion.h>
 #include <linux/sched/task.h>
 #include <linux/sched/vhost_task.h>
+#include <linux/resume_user_mode.h>
 #include <linux/sched/signal.h>
 
 enum vhost_task_flags {
@@ -31,6 +32,14 @@ static int vhost_task_fn(void *data)
 	for (;;) {
 		bool did_work;
 
+		/*
+		 * We never return to user mode, so TWA_RESUME task_work (e.g. CFS
+		 * bandwidth throttling) would otherwise never run. Run it here,
+		 * where no locks are held, like io-wq workers do.
+		 */
+		if (test_thread_flag(TIF_NOTIFY_RESUME))
+			resume_user_mode_work(NULL);
+
 		if (signal_pending(current)) {
 			struct ksignal ksig;
 

base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] vhost: Run TIF_NOTIFY_RESUME work in the vhost_task loop
  2026-10-31  1:33 [PATCH] vhost: Run TIF_NOTIFY_RESUME work in the vhost_task loop Aamir Ahmed
@ 2026-10-31 19:05 ` Shakeel Butt
  2026-11-01  1:33 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: Shakeel Butt @ 2026-10-31 19:05 UTC (permalink / raw)
  To: Aamir Ahmed
  Cc: Michael S. Tsirkin, Jason Wang, Eugenio Pérez,
	Valentin Schneider, Aaron Lu, Peter Zijlstra, kvm, virtualization,
	netdev, linux-kernel, stable

On Sat, Oct 31, 2026 at 01:33:13AM +0000, Aamir Ahmed wrote:
> Since commit e1fad12dcb66 ("sched/fair: Switch to task based throttle
> model"), a throttled task is only dequeued when TWA_RESUME task_work
> runs on its way back to user mode.
> 
> vhost workers never return to user mode, and vhost_task_fn() only runs
> task_work via get_signal() when a signal is pending. A busy vhost worker
> therefore ignores its cgroup's cpu.max, while its runtime is still
> charged to the group, throttling the group's other tasks (typically the
> VM's vCPUs) harder.
> 
> Run pending notify-resume work at the top of the worker loop, as io-wq
> does in io_run_task_work(). Commit 12013e3d4695 ("KVM: powerpc: Use
> generic xfer to guest work function") fixed the same problem for KVM.
> 
> Fixes: e1fad12dcb66 ("sched/fair: Switch to task based throttle model")
> Cc: stable@vger.kernel.org # 6.18+
> Assisted-by: LLM
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>

I have this (almost) same patch but for different reasons i.e. memory.high and
block throtlling. 

http://lore.kernel.org/20261031181004.4193627-1-shakeel.butt@linux.dev

If you don't mind, can you update the commit message to also include that it
will fix memory.high and cgroup block throttling as well?

You can add:

Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
Tested-by: Shakeel Butt <shakeel.butt@linux.dev>


BTW there is a similar issue with io_uring SQPOLL thread which I am fixing
(testing at the moment), you might be interested.

thanks,
Shakeel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] vhost: Run TIF_NOTIFY_RESUME work in the vhost_task loop
  2026-10-31  1:33 [PATCH] vhost: Run TIF_NOTIFY_RESUME work in the vhost_task loop Aamir Ahmed
  2026-10-31 19:05 ` Shakeel Butt
@ 2026-11-01  1:33 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-11-01  1:33 UTC (permalink / raw)
  To: Aamir Ahmed; +Cc: kvm, virtualization, Michael S. Tsirkin, Eugenio Perez

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] vhost: missing cond_resched() and loop in vhost_task_fn() after task_work
--

commit 97d56a24fed4c711b0eae24aaadc9595a3a04199
Author: Aamir Ahmed <elb12345@hotmail.co.uk>

vhost: Run TIF_NOTIFY_RESUME work in the vhost_task loop

This patch runs pending TIF_NOTIFY_RESUME work via resume_user_mode_work()
at the top of the vhost_task_fn() worker loop so that TWA_RESUME task_work,
such as CFS bandwidth throttling, is processed for vhost worker threads.

> diff --git a/kernel/vhost_task.c b/kernel/vhost_task.c
> index 3717885a5992a..7fa96e1b85b66 100644
> --- a/kernel/vhost_task.c
> +++ b/kernel/vhost_task.c
> @@ -6,6 +6,7 @@
>  #include <linux/completion.h>
>  #include <linux/sched/task.h>
>  #include <linux/sched/vhost_task.h>
> +#include <linux/resume_user_mode.h>
>  #include <linux/sched/signal.h>
>  
>  enum vhost_task_flags {
> @@ -31,6 +32,14 @@ static int vhost_task_fn(void *data)
>  	for (;;) {
>  		bool did_work;
>  
> +		/*
> +		 * We never return to user mode, so TWA_RESUME task_work (e.g. CFS
> +		 * bandwidth throttling) would otherwise never run. Run it here,
> +		 * where no locks are held, like io-wq workers do.
> +		 */
> +		if (test_thread_flag(TIF_NOTIFY_RESUME))
> +			resume_user_mode_work(NULL);

[Severity: High]
Should this loop while TIF_NOTIFY_RESUME is set and call cond_resched()
after resume_user_mode_work(NULL), similar to io_assign_current_work() in
io_uring/io-wq.c:

	if (work) {
		io_run_task_work();
		cond_resched();
	}

When resume_user_mode_work() runs task_work_run() -> throttle_cfs_rq_work()
in kernel/sched/fair.c, throttle_cfs_rq_work() dequeues current from its
cfs_rq (clearing p->se.on_rq), places it on cfs_rq->throttled_limbo_list,
sets p->throttled = true, and only sets TIF_NEED_RESCHED via
resched_curr(rq) without calling schedule():

	dequeue_task_fair(rq, p, DEQUEUE_SLEEP | DEQUEUE_THROTTLE);
	list_add(&p->throttle_node, &cfs_rq->throttled_limbo_list);
	p->throttled = true;
	resched_curr(rq);

Without a cond_resched() and loop here, can several problems occur on
CONFIG_PREEMPT_NONE or CONFIG_PREEMPT_VOLUNTARY kernels (or when a newly
unthrottled task is immediately re-throttled on schedule-in)?

1. Because unlocking task_rq_lock at the end of throttle_cfs_rq_work() does
not preempt current under CONFIG_PREEMPT_NONE or CONFIG_PREEMPT_VOLUNTARY,
vhost_task_fn() proceeds directly into vtsk->fn(vtsk->data) while current
is already on throttled_limbo_list with TIF_NEED_RESCHED pending.

For example, when vtsk->fn is kvm_nx_huge_page_recovery_worker() calling
kvm_recover_nx_huge_pages() in arch/x86/kvm/mmu/mmu.c, it acquires
srcu_read_lock(&kvm->srcu) and kvm->mmu_lock and then checks need_resched():

	rcu_idx = srcu_read_lock(&kvm->srcu);
	if (is_tdp_mmu)
		read_lock(&kvm->mmu_lock);
	else
		write_lock(&kvm->mmu_lock);
	...
		if (need_resched() || rwlock_needbreak(&kvm->mmu_lock)) {
			...
			if (is_tdp_mmu)
				cond_resched_rwlock_read(&kvm->mmu_lock);
			else
				cond_resched_rwlock_write(&kvm->mmu_lock);

At that point it schedules out into throttled_limbo_list for the remainder
of the CFS bandwidth period while holding srcu_read_lock(&kvm->srcu),
stalling synchronize_srcu(&kvm->srcu). Similarly, on CONFIG_PREEMPT_NONE,
vhost_run_work_list() executes work->fn(work) (which acquires vq->mutex)
before reaching its own cond_resched() at the end of the work loop, and any
CPU time spent before rescheduling is not charged by update_curr() because
p->se.on_rq is already 0.

2. If vtsk->fn(vtsk->data) returns false (!did_work) while current is on
throttled_limbo_list, vhost_task_fn() calls schedule() with state set to
TASK_INTERRUPTIBLE. In __schedule() -> try_to_block_task() -> block_task()
-> dequeue_task_fair() -> dequeue_throttled_task():

	list_del_init(&p->throttle_node);

	/* task blocked after throttled */
	if (flags & DEQUEUE_SLEEP) {
		p->throttled = false;
		return;
	}

This removes the task from throttled_limbo_list and clears p->throttled, so
a subsequent vhost_task_wake() while the cgroup is still throttled will
enqueue the worker back onto the throttled cfs_rq.

3. Even if cond_resched() schedules the throttled task out, when the task
is later unthrottled and scheduled back in via set_next_task_fair(), if
sibling tasks have already consumed the newly replenished quota,
set_next_task_fair() calls task_throttle_setup_work(p) to set
TIF_NOTIFY_RESUME again without setting TIF_NEED_RESCHED:

	if (throttled)
		task_throttle_setup_work(p);

If vhost_task_fn() only checks TIF_NOTIFY_RESUME with an if statement
instead of a while loop around resume_user_mode_work(NULL) and
cond_resched(), returning from cond_resched() will proceed into vtsk->fn()
with the new TIF_NOTIFY_RESUME work still pending.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/AS8P251MB00012469E6ECBEA2B7D87BB0C86C2@AS8P251MB0001.EURP251.PROD.OUTLOOK.COM?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-01  1:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-31  1:33 [PATCH] vhost: Run TIF_NOTIFY_RESUME work in the vhost_task loop Aamir Ahmed
2026-10-31 19:05 ` Shakeel Butt
2026-11-01  1:33 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox