Re: [PATCH v2 1/2] KVM: selftests: Drop superfluous use of pthread_attr_setaffinity_np()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The assertion intended to verify CPU affinity inheritance can silently pass if `pthread_getaffinity_np` fails, because `cpuset` is not cleared before the call and its return value is ignored.
--

--- Patch [1]: [PATCH v2 1/2] KVM: selftests: Drop superfluous use of pthread_attr_setaffinity_np() ---
commit 4058c4b23733ee3e6fc2f4b8ebc85f63f4492c52
Author: Sean Christopherson <seanjc@google.com>

KVM: selftests: Drop superfluous use of pthread_attr_setaffinity_np()

This commit removes explicit CPU affinity setting for the worker child thread
in the steal time test, relying on inheritance from the parent thread instead.
It adds an assertion to verify the affinity was properly inherited. It also
allows the test to build against non-glibc C libraries lacking
pthread_attr_setaffinity_np().

Link: https://man7.org/linux/man-pages/man3/pthread_setaffinity_np.3.html [1]
Link: https://man7.org/linux/man-pages/man3/pthread_create.3.html [1]
Closes: https://lore.kernel.org/all/20250829142556.72577-4-aqibaf@amazon.com

> diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
> index 76fcdd1fd3cb4..a244bf9f701f7 100644
> --- a/tools/testing/selftests/kvm/steal_time.c
> +++ b/tools/testing/selftests/kvm/steal_time.c

[ ... ]

> @@ -558,7 +555,11 @@ int main(int ac, char **av)
>  
>  		/* Steal time from the VCPU. The steal time thread has the same CPU affinity as the VCPUs. */
>  		run_delay = get_run_delay();
> -		pthread_create(&thread, &attr, do_steal_time, NULL);
> +		pthread_create(&thread, NULL, do_steal_time, NULL);
> +		pthread_getaffinity_np(thread, sizeof(cpuset), &cpuset);

[Severity: Medium]
Can this assertion spuriously pass if pthread_getaffinity_np() fails?

Because cpuset is not cleared before calling pthread_getaffinity_np() in
main(), and the return value isn't checked, cpuset will retain its previous
value if the call fails. This could happen if the child thread completes
its execution before pthread_getaffinity_np() is called.

Since cpuset was already populated with CPU 0 earlier in main(), the
TEST_ASSERT would read the stale data and pass even if the thread did not
inherit the affinity.

> +		TEST_ASSERT(CPU_COUNT(&cpuset) == 1 && CPU_ISSET(0, &cpuset),
> +			    "Worker failed to inherit parent's CPU affinity");
> +
>  		do
>  			sched_yield();
>  		while (get_run_delay() - run_delay < MIN_RUN_DELAY_NS);

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