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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The libnvme feature test hardcodes the `-lnvme` compiler flag, allowing the test to pass even if `pkg-config` is missing or fails to locate `libnvme.pc` (provided the library is in a standard system path). However, `tools/perf/Makefile.config` strictly relies on the output of `pkg-config --libs-only-l libnvme` to populate `EXTLIBS`. This mismatch causes the build system to enable NVMe PMU support based on the successful feature test, but then fail during the final linking stage of the `perf` executable due to missing `-lnvme` flags, resulting in undefined references to libnvme functions.
--

commit a99ed0c67b6c2d89a762f7be2b57899a90210460
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/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: Low]
Does hardcoding the -lnvme flag here bypass the pkg-config requirement?

If libnvme is installed in standard paths but its pkg-config file is missing
or pkg-config is absent, this feature test will still pass.

> +
>  $(OUTPUT)test-eventfd.bin:
>  	$(BUILD)

[ ... ]

> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 6e7b15fab2ecc..79c4ad10bc181 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config

[ ... ]

> @@ -1060,6 +1060,18 @@ ifndef NO_CAPSTONE
>    endif
>  endif
>  
> +ifndef NO_LIBNVME
> +  $(call feature_check,libnvme)
> +  ifeq ($(feature-libnvme), 1)
> +    CFLAGS += -DHAVE_LIBNVME_SUPPORT $(shell $(PKG_CONFIG) --cflags libnvme)
> +    LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libnvme)
> +    EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libnvme)

[Severity: Low]
Since EXTLIBS relies strictly on pkg-config here, will the build fail during
final linking if the feature test succeeded using the hardcoded -lnvme flag?

If pkg-config doesn't return the required flags, the perf executable would
encounter undefined references instead of gracefully falling back to a build
without NVMe PMU support.

> +    $(call detected,CONFIG_LIBNVME)
> +  else
> +    msg := $(warning No libnvme found, disables NVMe PMU support, please install libnvme-dev/libnvme-devel);
> +  endif
> +endif

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