Re: [PATCH v1 1/3] perf build: Add libnvme feature detection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The feature check for `libnvme` fails to utilize `pkg-config` during the test compilation, contradicting the commit message and breaking detection when the library is in a non-standard path.
- [Medium] `libnvme` is omitted from `test-all.c`, which causes false positives in fast-path detection and triggers continuous feature-dump flip-flops and rebuilds.
--

commit 1f4a72ba50b99659f0972d94ef171f8be76bc329
Author: Ian Rogers <irogers@google.com>

perf build: Add libnvme feature detection

Introduce a feature check for libnvme so that we can conditionally
compile the NVMe PMU driver. This checks for the availability of the
libnvme headers and library using pkg-config.

> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index ed1374af31c16..bf85032415397 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -73,6 +73,7 @@ FEATURE_TESTS_BASIC :=                  \
>          libbfd                          \
>  	libbfd-threadsafe		\
>          libelf                          \
> +        libnvme                         \

[Severity: Medium]
Without including test-libnvme.c in test-all.c, the fast-path build will
succeed even if libnvme is absent.

This can cause the initial discovery to falsely report libnvme as present,
which then flip-flops when individual checks are run later in
tools/perf/Makefile.config. This mismatch invalidates the build cache and
triggers full feature evaluations and rebuilds on every make invocation.

Since libnvme was added to FEATURE_TESTS_BASIC, does it also need to be
included in tools/build/feature/test-all.c?

>          libelf-getphdrnum               \
>          libelf-gelf_getnote             \
>          libelf-getshdrstrndx            \

[ ... ]

> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 62909a9c799d3..fd00e0be60690 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -147,6 +147,9 @@ $(OUTPUT)test-libcap.bin:
>  $(OUTPUT)test-libelf.bin:
>  	$(BUILD) -lelf
>  
> +$(OUTPUT)test-libnvme.bin:
> +	$(BUILD) -lnvme
> +

[Severity: Medium]
The commit message states the availability of libnvme headers and library are
checked using pkg-config. However, because libnvme is omitted from the
FEATURE_PKG_CONFIG list in tools/build/Makefile.feature, the variables
FEATURE_CHECK_CFLAGS-libnvme and FEATURE_CHECK_LDFLAGS-libnvme are never
initialized.

This rule manually invokes $(BUILD) -lnvme, falling back to standard compiler
paths instead of leveraging pkg-config variables.

If libnvme is installed in a non-standard location discoverable only via
pkg-config, could the feature test fail and incorrectly disable NVMe PMU
support?

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