Re: [PATCH v4 03/13] perf tests workloads: Support sub-second durations in noploop and thloop

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Conversion of an unbounded floating-point value to `unsigned int` invokes undefined behavior.
- [Low] Missing explicit inclusion of `<stdio.h>` for `fprintf` and `stderr` usage.
--

commit 7d05873e679bf07cde18f37a2d18403b7405f3c4
Author: Ian Rogers <irogers@google.com>

perf tests workloads: Support sub-second durations in noploop and thloop

This commit updates noploop and thloop workloads to support sub-second sleep
durations by switching from atoi() to atof(). It uses ualarm() for durations
under 1.0 second and falls back to alarm() for longer durations.

> diff --git a/tools/perf/tests/workloads/noploop.c b/tools/perf/tests/workloads/noploop.c
> index 656e472e61882..ca9f871e136f5 100644
> --- a/tools/perf/tests/workloads/noploop.c
> +++ b/tools/perf/tests/workloads/noploop.c
> @@ -15,15 +15,26 @@ static void sighandler(int sig __maybe_unused)
>  
>  static int noploop(int argc, const char **argv)
>  {
> -	int sec = 1;
> +	double sec = 1.0;
>  
>  	pthread_setname_np(pthread_self(), "perf-noploop");
>  	if (argc > 0)
> -		sec = atoi(argv[0]);
> +		sec = atof(argv[0]);
> +
> +	if (!(sec > 0.0)) {
> +		fprintf(stderr, "Error: seconds (%f) must be > 0\n", sec);

[Severity: Low]
Does this code compile cleanly under musl libc? The perf subsystem guidelines
require explicit POSIX header inclusions for all libc functions to prevent
musl compilation failures. The stdio.h header appears to be missing for this
fprintf and stderr usage.

> +		return 1;
> +	}
>  
>  	signal(SIGINT, sighandler);
>  	signal(SIGALRM, sighandler);
> -	alarm(sec);
> +
> +	if (sec < 1.0) {
> +		useconds_t usecs = (useconds_t)(sec * 1000000.0);
> +
> +		ualarm(usecs > 0 ? usecs : 1, 0);
> +	} else
> +		alarm((unsigned int)sec);

[Severity: Medium]
What happens if the parsed duration exceeds UINT_MAX? Converting an unbounded
floating-point value like 1e20 to an unsigned int invokes undefined behavior.
Should sec be bound-checked before this cast?

[ ... ]

> diff --git a/tools/perf/tests/workloads/thloop.c b/tools/perf/tests/workloads/thloop.c
> index bd8168f883fb6..c830d739489fe 100644
> --- a/tools/perf/tests/workloads/thloop.c
> +++ b/tools/perf/tests/workloads/thloop.c
> @@ -31,14 +31,15 @@ static void *thfunc(void *arg)
>  
>  static int thloop(int argc, const char **argv)
>  {
> -	int nt = 2, sec = 1, err = 1;
> +	int nt = 2, err = 1;
> +	double sec = 1.0;
>  	pthread_t *thread_list = NULL;
>  
>  	if (argc > 0)
> -		sec = atoi(argv[0]);
> +		sec = atof(argv[0]);
>  
> -	if (sec <= 0) {
> -		fprintf(stderr, "Error: seconds (%d) must be >= 1\n", sec);
> +	if (!(sec > 0.0)) {
> +		fprintf(stderr, "Error: seconds (%f) must be > 0\n", sec);

[Severity: Low]
Similarly, this usage of fprintf and stderr might cause musl compilation to
fail if stdio.h is not explicitly included in this file.

[ ... ]

> @@ -67,7 +68,12 @@ static int thloop(int argc, const char **argv)
>  			goto out;
>  		}
>  	}
> -	alarm(sec);
> +	if (sec < 1.0) {
> +		useconds_t usecs = (useconds_t)(sec * 1000000.0);
> +
> +		ualarm(usecs > 0 ? usecs : 1, 0);
> +	} else
> +		alarm((unsigned int)sec);

[Severity: Medium]
Is this unbounded cast to unsigned int safe? Just as in noploop(), if sec is
excessively large, this cast could result in undefined behavior.

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