Re: [PATCH v2 06/12] KVM: selftests: Affine threads to random CPUs in hardware disable test

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Randomly selected CPUs are not removed from `allowed_cpu_set`, leading to duplicate selections and artificially reduced test concurrency.
--

--- Patch [6]: [PATCH v2 06/12] KVM: selftests: Affine threads to random CPUs in hardware disable test ---
commit 093b65b96e15e7cdd5b611bd7d8556fb88cae6e6
Author: Sean Christopherson <[email protected]>

KVM: selftests: Affine threads to random CPUs in hardware disable test

This patch modifies the hardware disable test to affine worker threads
to random CPUs instead of hardcoding them to CPUs 0-3. It also honors
the starting CPU set during this process.

> diff --git a/tools/testing/selftests/kvm/hardware_disable_test.c b/tools/testing/selftests/kvm/hardware_disable_test.c
> index d8288147b0a69..bd23c13003707 100644
> --- a/tools/testing/selftests/kvm/hardware_disable_test.c
> +++ b/tools/testing/selftests/kvm/hardware_disable_test.c
> @@ -149,12 +149,15 @@ void wait_for_child_setup(pid_t pid)
>  
>  int main(int argc, char **argv)
>  {
> +	cpu_set_t allowed_cpu_set;
>  	u32 i;
>  	int s, r;
>  	pid_t pid;
>  
> -	for (i = 0; i < VCPU_NUM; i++)
> -		CPU_SET(i, &threads_cpu_set);
> +	kvm_sched_getaffinity(0, sizeof(cpu_set_t), &allowed_cpu_set);
> +
> +	for (i = 0; i < VCPU_NUM && CPU_COUNT(&allowed_cpu_set); i++)
> +		CPU_SET(kvm_pick_random_cpu(&allowed_cpu_set), &threads_cpu_set);

[Severity: Medium]
Since kvm_pick_random_cpu() doesn't clear the selected CPU from
allowed_cpu_set, can the same CPU be selected multiple times here?

Because the selected CPU is not cleared (for example, with CPU_CLR),
threads_cpu_set could end up with fewer than VCPU_NUM unique CPUs. This
might artificially reduce the test's concurrency.

Additionally, the loop condition checks CPU_COUNT(&allowed_cpu_set), but
since allowed_cpu_set is never modified inside the loop, this count
will remain constant. Was there an intention to remove the chosen CPU
from allowed_cpu_set in each iteration?

>  	sem = sem_open("vm_sem", O_CREAT | O_EXCL, 0644, 0);
>  	sem_unlink("vm_sem");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6