Re: [PATCH v3 15/19] perf tests: Introduce a 'needs_mlock' suite flag

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf test: missing stdio.h inclusion for musl libc compatibility
--

commit 0cd49c7e0c286b9d1bb7377465b008f3b907a53c
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf tests: Introduce a 'needs_mlock' suite flag

This commit adds a 'needs_mlock' flag to 'struct test_suite' to distinguish
test suites that map ring buffers from those that do not, allowing them to
take turns using the kernel.perf_event_mlock_kb budget. It also introduces
a 'requisites' subcommand to list test requirements.

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 98134b1c74cf8..acffa0d0d33d6 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
[ ... ]
> @@ -283,6 +285,30 @@ static const char *test_description(const struct test_suite *t, int test_case)
>  	return t->desc;
>  }
>  
> +static void requisites__fprintf(FILE *fp, const struct test_suite *t, int test_case)
> +{
> +	bool needs_mlock = t->needs_mlock, exclusive;

[Severity: Low]
Since this newly added function explicitly uses the FILE type and fprintf(),
should <stdio.h> be explicitly included at the top of the file to ensure
compatibility with musl libc?

> +
> +	if (test_case < 0)
> +		test_case = test_suite__num_test_cases(t) == 1 ? 0 : -1;
[ ... ]
> diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c
> index 0370bc7013737..0bc84372eae09 100644
> --- a/tools/perf/tests/tests-scripts.c
> +++ b/tools/perf/tests/tests-scripts.c
[ ... ]
> @@ -187,7 +260,8 @@ static void append_script(int dir_fd, const char *name, char *desc,
>  	struct test_suite *test_suite, **result_tmp;
>  	struct test_case *tests;
>  	ssize_t len;
> -	char *exclusive;
> +	/* Shell tests run perf, that maps ring buffers, so they need the budget. */
> +	bool exclusive = false, needs_mlock = true;
>  
>  	snprintf(link, sizeof(link), "/proc/%d/fd/%d", getpid(), dir_fd);

[Severity: Low]
This isn't a bug in the patch logic, but the code relies on snprintf() here.
Would it make sense to explicitly include <stdio.h> in this file as well to
prevent potential build failures on systems using musl libc?

>  	len = readlink(link, filename, sizeof(filename) - 1);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016001843.71978-1-acme@kernel.org?part=15