Hello, Paolo.
On Tue, Oct 29, 2024 at 11:59:43PM +0100, Paolo Bonzini wrote:
...
> For the freezing part, would this be anything more than
>
> fdiff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index d16ce8174ed6..b7b6a1c1b6a4 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -47,6 +47,7 @@
> #include <linux/kern_levels.h>
> #include <linux/kstrtox.h>
> #include <linux/kthread.h>
> +#include <linux/freezer.h>
> #include <linux/wordpart.h>
> #include <asm/page.h>
> @@ -7429,22 +7430,27 @@ static long get_nx_huge_page_recovery_timeout(u64 start_time)
> static int kvm_nx_huge_page_recovery_worker(struct kvm *kvm, uintptr_t data)
> {
> u64 start_time;
> + u64 end_time;
> +
> + set_freezable();
> while (true) {
> start_time = get_jiffies_64();
> + end_time = start_time + get_nx_huge_page_recovery_timeout(start_time);
> + for (;;) {
> set_current_state(TASK_INTERRUPTIBLE);
> + if (kthread_freezable_should_stop(NULL))
> + break;
> + start_time = get_jiffies_64();
> + if ((s64)(end_time - start_time) <= 0)
> + break;
> + schedule_timeout(end_time - start_time);
> }
> set_current_state(TASK_RUNNING);
> + if (kthread_freezable_should_stop(NULL))
So, this is the PM and cgroup1 freezer which traps tasks in a pretty opaque
state which is problematic when only a part of the system is frozen, so the
cgroup2 freezer is implemented in the signal delivery / trap path so that
they behave similar to e.g. SIGSTOP stops.
> return 0;
> kvm_recover_nx_huge_pages(kvm);
>
> (untested beyond compilation).
>
> I'm not sure if the KVM worker thread should process signals. We want it
> to take the CPU time it uses from the guest, but otherwise it's not running
> on behalf of userspace in the way that io_wq_worker() is.
I see, so io_wq_worker()'s handle signals only partially. It sets
PF_USER_WORKER which ignores fatal signals, so the only signals which take
effect are STOP/CONT (and friends) which is handled in do_signal_stop()
which is also where the cgroup2 freezer is implemented.
Given that the kthreads are tied to user processes, I think it'd be better
to behave similarly to user tasks as possible in this regard if userspace
being able to stop/cont these kthreads are okay.
If that can't be done, we can add a mechanism to ignore these tasks when
determining when a cgroup is frozen provided that this doesn't affect the
snapshotting (ie. when taking a snapshot of frozen cgroup, activities from
the kthreads won't corrupt the snapshot).
Thanks.
--
tejun
