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.