Re: [PATCH v3 08/10] KVM: selftests: Trigger save+restore randomly in the #PF stress test

[Yosry Ahmed <yosry@kernel.org>]

On Mon, Jun 29, 2026 at 06:48:37PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The aggressive 1ms signal interval can starve the guest and cause the test to deterministically fail on slower or nested environments.
> --
> 
> commit 4750524a9d34a97e2957bdf96a22b57c5404cb65
> Author: Yosry Ahmed <yosry@kernel.org>
> 
> KVM: selftests: Trigger save+restore randomly in the #PF stress test
> 
> This patch introduces a background thread in the page fault stress test that
> sends SIGUSR1 to the vCPU thread to trigger exits to userspace. This allows
> exercising race conditions between page fault handling in the guest and
> save/restore paths in the host.
> 
> > diff --git a/tools/testing/selftests/kvm/x86/stress_save_restore_pf_test.c b/tools/testing/selftests/kvm/x86/stress_save_restore_pf_test.c
> > index 1b6f64bbcf937..bbbb5bb2a2ee1 100644
> > --- a/tools/testing/selftests/kvm/x86/stress_save_restore_pf_test.c
> > +++ b/tools/testing/selftests/kvm/x86/stress_save_restore_pf_test.c
> 
> [ ... ]
> 
> > @@ -94,15 +96,41 @@ static void guest_access_memory(void *arg)
> > +static void *sigusr_thread_fn(void *arg)
> > +{
> > +	pthread_t vcpu_thread = (pthread_t)arg;
> > +
> > +	for (;;) {
> > +		pthread_testcancel();
> > +		pthread_kill(vcpu_thread, SIGUSR1);
> > +		usleep(msecs_to_usecs(1));
> > +	}
> > +	return NULL;
> > +}
> 
> [ ... ]
> 
> > @@ -151,18 +179,30 @@ int main(int argc, char *argv[])
> >  	while (count++ < NR_ITERATIONS) {
> > +		/*
> > +		 * Only handle SIGUSR while the vCPU is running, otherwise
> > +		 * ignore it to avoid interrupting other ioctls/syscalls.
> > +		 */
> > +		vcpu_sigusr_listen();
> >  		r = __vcpu_run(vcpu);
> > -		TEST_ASSERT(!r, "vcpu_run failed");
> > -		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
> > -
> > -		get_ucall(vcpu, &uc);
> > -		if (uc.cmd == UCALL_ABORT) {
> > +		if (r == -1)
> > +			TEST_ASSERT_EQ(errno, EINTR);
> > +		vcpu_sigusr_ignore();
> 
> [Severity: Medium]
> Can the aggressive 1ms signal interval from sigusr_thread_fn starve the guest
> and cause deterministic test failures in slower environments?
> 
> The first time __vcpu_run executes for a newly created VM, it performs
> expensive kernel-side initializations like setting up the shadow MMU. If this
> setup takes longer than 1ms (which is probable in nested or heavily loaded CI
> environments), the background thread will send SIGUSR1 before the guest
> actually executes.
> 
> When KVM detects the pending signal, it will abort the entry and return
> -EINTR. Because main catches -EINTR and immediately proceeds to save the
> unchanged state and recreate the VM for the next iteration, the test can get
> stuck in a cycle where the guest never makes forward progress.
> 
> After exhausting all iterations doing nothing but VM recreations, the test
> would then fail the guest_faults > 0 assertion at the end. Could we verify
> that the guest actually ran and made forward progress before destroying the
> VM, or perhaps increase the signal interval?

The signal interval was initially 100us and that did cause problems on
slower kernels (with DEBUG options or ASAN). With 1 msec, we don't
observe any problems internally with DEBUG or ASAN builds on a variety
of platforms. So I think we are good with 1 msec.

Increasing the interval too much will probably make the test less
effective, as the signal is less likely to hit in interesting race
windows.