Re: [PATCH v2 2/2] KVM: selftests: Randomize pCPU in steal time test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Uninitialized variable read leading to test flakes if pthread_getaffinity_np() fails.
--

--- Patch [2]: [PATCH v2 2/2] KVM: selftests: Randomize pCPU in steal time test ---
commit 0d5df2335fea13e6ab9b9a76b38f38b87bb2fb85
Author: Sean Christopherson <seanjc@google.com>
KVM: selftests: Randomize pCPU in steal time test

This commit updates the steal time test to pin its tasks to a random pCPU
instead of hardcoding pCPU0. It aims to increase test coverage and properly
handle parent tasks that lack pCPU0 in their CPU affinity mask.

> diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
> --- a/tools/testing/selftests/kvm/steal_time.c
> +++ b/tools/testing/selftests/kvm/steal_time.c
[ ... ]
> @@ -514,14 +514,12 @@ int main(int ac, char **av)
>  	long stolen_time;
>  	long run_delay;
>  	bool verbose;
> -	int i;
> +	int i, cpu;
>  
>  	verbose = ac > 1 && (!strncmp(av[1], "-v", 3) || !strncmp(av[1], "--verbose", 10));
>  
>  	/* Set CPU affinity so we can force preemption of the VCPU */
> -	CPU_ZERO(&cpuset);
> -	CPU_SET(0, &cpuset);
> -	pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset);
> +	cpu = pin_self_to_any_cpu();
>  
>  	/* Create a VM and an identity mapped memslot for the steal time structure */
>  	vm = vm_create_with_vcpus(NR_VCPUS, guest_code, vcpus);
[ ... ]
> @@ -557,7 +555,7 @@ int main(int ac, char **av)
>  		run_delay = get_run_delay();
>  		pthread_create(&thread, NULL, do_steal_time, NULL);
>  		pthread_getaffinity_np(thread, sizeof(cpuset), &cpuset);
> -		TEST_ASSERT(CPU_COUNT(&cpuset) == 1 && CPU_ISSET(0, &cpuset),
> +		TEST_ASSERT(CPU_COUNT(&cpuset) == 1 && CPU_ISSET(cpu, &cpuset),

[Severity: Medium]
Is there a race condition here if the worker thread exits before the parent
reads its affinity?

When pthread_create() spawns the worker thread, the child runs for a short
duration and exits. If the parent thread is preempted right after creation,
the child might finish its work and exit before pthread_getaffinity_np()
executes in the parent.

If the kernel task is already gone, pthread_getaffinity_np() will fail with
ESRCH. Because the explicit initialization of cpuset was removed earlier in
main(), cpuset remains completely uninitialized.

Could this cause CPU_COUNT() and CPU_ISSET() to evaluate garbage stack memory
and trigger sporadic TEST_ASSERT failures? Should the return value of
pthread_getaffinity_np() be checked, or should cpuset be initialized before
the call?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707150706.1198541-1-seanjc@google.com?part=2