Re: [RFC PATCH v1 2/3] perf build: Prefer zlib-rs over zlib when available

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf build: RUST_FLAGS set after Makefile.feature breaks cross feature-dump
--

commit 71bfdd124d6b614e2d4b506eb746a963a59097da
Author: Ian Rogers <irogers@google.com>

perf build: Prefer zlib-rs over zlib when available

This commit updates the perf build system to detect and prefer zlib-rs over C
zlib unless disabled via NO_ZLIB_RS=1, NO_ZLIB=1, or NO_RUST=1. It moves Rust
compiler detection ahead of zlib detection in Makefile.config, builds
libz_rs.a in $(OUTPUT)libz_rs/, and exposes the zlib-rs feature in perf check.

> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index b4764f69f47f4..4313fca53aa7c 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -532,13 +532,57 @@ ifdef CORESIGHT
>    endif
>  endif
>  
> -ifndef NO_ZLIB
> -  ifeq ($(feature-zlib), 1)
> -    CFLAGS += -DHAVE_ZLIB_SUPPORT
> -    EXTLIBS += -lz
> -    $(call detected,CONFIG_ZLIB)
> +ifndef NO_RUST
> +  $(call feature_check,rust)
> +  ifneq ($(feature-rust), 1)
> +    $(warning Rust is not found. Test workloads with rust are disabled.)
> +    NO_RUST := 1
>    else
> -    NO_ZLIB := 1
> +    NO_RUST := 0
> +    CFLAGS += -DHAVE_RUST_SUPPORT
> +    $(call detected,CONFIG_RUST_SUPPORT)
> +  endif
> +
> +  ifneq ($(CROSS_COMPILE),)
> +    ifeq ($(RUST_TARGET_FLAGS_$(ARCH)),)
> +      $(error Unknown rust cross compilation architecture $(ARCH))
> +    endif
> +
> +    RUST_FLAGS += --target=$(RUST_TARGET_FLAGS_$(ARCH))
> +  endif
> +endif
> +
> +# Building zlib-rs from Cargo crate sources requires a working rustc.
> +ifneq ($(NO_RUST), 0)
> +  NO_ZLIB_RS := 1
> +endif
> +
> +# Prefer zlib-rs over C zlib when available, unless disabled via NO_ZLIB=1 or
> +# NO_ZLIB_RS=1. Both provide the standard zlib C API (HAVE_ZLIB_SUPPORT), while
> +# HAVE_ZLIB_RS_SUPPORT records that zlib-rs is the implementation in use.
> +ifndef NO_ZLIB
> +  ifndef NO_ZLIB_RS
> +    $(call feature_check,zlib-rs)

[Severity: Medium]
When cross-compiling with the feature-dump target (or FEATURE_TESTS=all),
does setting RUST_FLAGS here happen too late for tools/build/Makefile.feature?

In tools/perf/Makefile.perf, building the feature-dump target sets
FEATURE_TESTS := all before including Makefile.config:

tools/perf/Makefile.perf:
ifeq ($(config),1)
ifdef MAKECMDGOALS
ifeq ($(filter feature-dump,$(MAKECMDGOALS)),feature-dump)
FEATURE_TESTS := all
endif
endif
include Makefile.config
endif

Earlier in tools/perf/Makefile.config, tools/build/Makefile.feature is
included before the NO_RUST block:

tools/perf/Makefile.config:
ifeq ($(FEATURES_DUMP),)
  FEATURE_DISPLAY_DEFERRED := 1
include $(srctree)/tools/build/Makefile.feature
else
include $(FEATURES_DUMP)
endif

When FEATURE_TESTS is set to all, Makefile.feature expands FEATURE_TESTS to
include FEATURE_TESTS_EXTRA (which contains zlib-rs) and runs
$(call feature_check,zlib-rs) while RUST_FLAGS still lacks
--target=$(RUST_TARGET_FLAGS_$(ARCH)).

That initial feature check invokes tools/build/Makefile.rust with an empty
RUST_FLAGS, compiling $(OUTPUT)libz_rs/libz_rs.a for the host architecture.
Linking test-zlib-rs.bin with the cross-compiler $(CC) against the host
libz_rs.a then fails and records feature-zlib-rs=0 in $(OUTPUT)FEATURE-DUMP.

Furthermore, when execution reaches the second $(call feature_check,zlib-rs)
above after RUST_FLAGS is populated, $(OUTPUT)libz_rs/libz_rs.a already
exists on disk from the host build, so Make considers it up-to-date and links
against the stale host archive again.

Should the CROSS_COMPILE configuration of RUST_FLAGS be moved above the
inclusion of tools/build/Makefile.feature alongside the CC and CLANG_FLAGS
cross-compilation setup?

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