Re: [PATCH 4/9] KVM: selftests: Drop unreachable, dead code from hardware disable test

Sean Christopherson <[email protected]> Fri, 31 Jul 2026 06:00:26 -0700
Newsgroups dev.linux.lists.sashiko-reviews,dev.linux.lists.kvmarm,org.kernel.vger.kvm
Message-ID <[email protected]>
On Thu, Jul 30, 2026, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > diff --git a/tools/testing/selftests/kvm/hardware_disable_test.c b/tools/testing/selftests/kvm/hardware_disable_test.c
> > index 1bcbd13e99947..e08aef94382f3 100644
> > --- a/tools/testing/selftests/kvm/hardware_disable_test.c
> > +++ b/tools/testing/selftests/kvm/hardware_disable_test.c
> 
> [ ... ]
> 
> > @@ -96,21 +84,19 @@ static void run_test(u32 run)
> [ ... ]
> >  	pr_debug("%s: [%d] all threads launched\n", __func__, run);
> >  	sem_post(sem);
> > -	for (i = 0; i < VCPU_NUM; ++i)
> > -		check_join(threads[i], &b);
> > -	/* Should not be reached */
> > -	TEST_ASSERT(false, "%s: [%d] child escaped the ninja", __func__, run);
> > +
> > +	/* Wait for the parent to SIGKILL this child. */
> > +	while (1);
> 
> [Severity: Medium]
> Since the child and parent share the same CPU affinity, will this busy spin
> loop cause unnecessary scheduler contention with the parent process when the
> parent's timer fires?

Doesn't seem like it?  Or rather, any contention between the parent and the
immediate child doesn't seem to matter.

> Because the processes are pinned to a single CPU core, spinning in this loop
> burns CPU cycles and forces the parent to contend with the spinning child for
> CPU time.

Yep.

> Would it be better to use a blocking wait like pause() or sleep() here to
> reduce scheduler latency and avoid 100% CPU utilization?

Yeah, it makes sense to throw pause() in here, if only to maintain equivalent
functionality.  Because that's effectively what pthread_join() will do since the
grandchildren never exit.