Re: [PATCH v5 09/19] perf test: Add named_threads workload

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The test workload loop is optimized away by the compiler, contains a C11 data race on a shared variable, and contradicts its commit message.
--

commit dcd726ac63da1e487f029b2a3e5128fec278becd
Author: James Clark <james.clark@linaro.org>

perf test: Add named_threads workload

This commit adds a new workload to perf tests that runs multiple threads,
each executing a unique function that loops and sets its thread name. It
is intended to test processor trace decoding for concurrent threads.

> perf test: Add named_threads workload
> 
> Add a workload that runs X threads that run a unique function named
> "named_threads_thread[x]" which performs a multiplication in a loop for

[Severity: Low]
The commit message mentions performing a multiplication, but the
implementation uses addition. Is this the intended behavior?

> Y loops. Each thread sets its name to "thread[x]".

[ ... ]

> diff --git a/tools/perf/tests/workloads/Build b/tools/perf/tests/workloads/Build
> index 90f2d8aa4941f..75b377934a0e6 100644
> --- a/tools/perf/tests/workloads/Build
> +++ b/tools/perf/tests/workloads/Build
> @@ -2,6 +2,7 @@
>  
>  perf-test-y += noploop.o
>  perf-test-y += thloop.o
> +perf-test-y += named_threads.o

[Severity: Medium]
In tools/perf/tests/workloads/Build, unlike other loop workloads in this
directory, there is no CFLAGS_named_threads.o = -O0 override.

Without this, will the compiler optimize the loop away into a single
arithmetic operation and remove the branch instructions this test relies on?

>  perf-test-y += leafloop.o

[ ... ]

> diff --git a/tools/perf/tests/workloads/named_threads.c b/tools/perf/tests/workloads/named_threads.c
> new file mode 100644
> index 0000000000000..d051d41a3cfe4
> --- /dev/null
> +++ b/tools/perf/tests/workloads/named_threads.c

[ ... ]

> +#define DEFINE_THREAD(n)						\
> +noinline void *named_threads_thread##n(void *arg __maybe_unused)	\
> +{									\
> +	pthread_setname_np(pthread_self(), "thread" #n);		\
> +	for (int i = 0; i < iterations; i++)				\
> +		named_threads_work += 3;				\

[Severity: Medium]
In DEFINE_THREAD() within tools/perf/tests/workloads/named_threads.c,
named_threads_work is a shared global variable accessed concurrently by
multiple threads.

Without atomic operations or locks, does this introduce a data race?

This might trigger ThreadSanitizer test failures during execution.

> +									\
> +	return NULL;							\
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-james-cs-context-tracking-fix-v5-0-d53a7d096a19@linaro.org?part=9