[PATCH v3 0/4] selftests: shared lib.bpf.mk for building BPF progs and skeletons

[PATCH v3 0/4] selftests: shared lib.bpf.mk for building BPF progs and skeletons

[Ziyang Men]

This series re-factors the bpf-related toolchain for selftests, which
currently duplicated across selftests/{bpf,sched_ext,hid}/. Unify them
into a single includable fragment, the tools/testing/selftests/lib.bpf.mk,
and wires up three consumers. We believe this will simplify the work for
configuring selftests with the bpf, for both the existing and future ones.

Patch 1 adds lib.bpf.mk mentioned above.

Patch 2 adds a new selftest, selftests/cgroup/test_memcg_stat_cross_cpu,
which checks the cgroup flush happens correctly by comparing the bpf
read value matches the file reading. Existing test
bpf/cgroup_iter_memcg only ensures whether the number is non-zero, which
is trivial in some cases.

Patches 3 and 4 replace the duplicated parts in selftests/{hid,
sched_ext} with the lib.bpf.mk

===
Changes since v2:
For shared Makefile:
- The lib.bpf.mk now use the USERCFLAGS (and USERLDFLAGS) to extend the
  CFLAGS and LDFLAGS. 
- OPT_FLAGS/RELEASE and EXTRA_CFLAGS now reach the libbpf and bpftool
  sub-makes as well.
- One rule per BPF source instead of a vpath, which is global and also
  applied to the caller's own %.c rules.
- HOSTCC/HOSTLD come from lib.mk instead of including Makefile.include,
  which also rewrote the caller's AR, LD and CFLAGS.
- A missing vmlinux is reported by the recipe rather than at parse time,
  so "make clean" still works without BTF.

For cgroup selftest:
- Only one cgroup tree is created, one flush is performed, follows by
  bpf read and file read. 
- More common cgroup functions e.g., cg_read_key_long() and
  value_close(), are used in the selftests.
- Use cpu_set_t rather than manually allocated cpu lists.
- Use test_memcontrol.c's anon allocator and moving it into cgroup_util.
- Other suggestion from the review.

Others:
- Bot reviews are fixed.

===
Changes since v1:
  - Generalized lib.bpf.mk (source layout/suffix, extra hdrs/cflags,
    skeleton suffix, subskeletons, configurable output dirs) so it can
    serve hid and sched_ext, not just cgroup.
  - Added patch 3 (hid) and patch 4 (sched_ext), converting those folders
    to the shared fragment.

Ziyang Men (4):
  selftests: add shared lib.bpf.mk to build BPF progs and skeletons
  selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush
  selftests/hid: build the BPF program via the shared lib.bpf.mk
  selftests/sched_ext: build BPF schedulers via the shared lib.bpf.mk

 tools/testing/selftests/cgroup/.gitignore     |   8 +
 tools/testing/selftests/cgroup/Makefile       |  52 ++
 tools/testing/selftests/cgroup/config         |   5 +
 .../selftests/cgroup/lib/cgroup_util.c        |  83 +++
 .../cgroup/lib/include/cgroup_util.h          |   3 +
 .../cgroup/memcg_stat_cross_cpu.bpf.c         |  86 +++
 .../selftests/cgroup/memcg_stat_cross_cpu.h   |  21 +
 .../cgroup/test_memcg_stat_cross_cpu.c        | 596 ++++++++++++++++++
 .../selftests/cgroup/test_memcontrol.c        |  29 +-
 tools/testing/selftests/hid/.gitignore        |   1 +
 tools/testing/selftests/hid/Makefile          | 180 +-----
 tools/testing/selftests/lib.bpf.mk            | 296 +++++++++
 tools/testing/selftests/sched_ext/Makefile    | 146 +----
 13 files changed, 1204 insertions(+), 302 deletions(-)
 create mode 100644 tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
 create mode 100644 tools/testing/selftests/cgroup/memcg_stat_cross_cpu.h
 create mode 100644 tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c
 create mode 100644 tools/testing/selftests/lib.bpf.mk

-- 
2.53.0-Meta

[PATCH v3 1/4] selftests: add shared lib.bpf.mk to build BPF progs and skeletons

[Ziyang Men]

The libbpf + bpftool + vmlinux.h + BPF-object + skeleton build tool-chain
is currently duplicated across tools/testing/selftests/{bpf,sched_ext,
hid}/, each carrying ~100-140 lines of near-identical Makefile.  As more
subsystems grow BPF-based selftests, the duplication scales poorly.

This patch adds tools/testing/selftests/lib.bpf.mk, a single includable
fragment that provides the whole chain end-to-end.

To use: set BPF_SRCS and OVERRIDE_TARGETS := 1 before including ../lib.mk
(so lib.mk's default link rule is suppressed), then include ../lib.bpf.mk
and list $(BPF_SKELS) and $(BPFOBJ) as prerequisites of the test binary,
e.g.,:

    BPF_SRCS         := progs/foo.bpf.c
    TEST_GEN_PROGS   := foo_test
    OVERRIDE_TARGETS := 1
    include ../lib.mk
    include ../lib.bpf.mk
    $(OUTPUT)/foo_test: foo_test.c $(BPF_SKELS) $(BPFOBJ)
            $(call bpf_link,$@,$<)

This eases adding BPF-based selftests in other directories, such as
cgroup.

Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Suggested-by: Mykola Lysenko <mykolal@meta.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
---
 tools/testing/selftests/lib.bpf.mk | 296 +++++++++++++++++++++++++++++
 1 file changed, 296 insertions(+)
 create mode 100644 tools/testing/selftests/lib.bpf.mk

diff --git a/tools/testing/selftests/lib.bpf.mk b/tools/testing/selftests/lib.bpf.mk
new file mode 100644
index 000000000000..433a26a36960
--- /dev/null
+++ b/tools/testing/selftests/lib.bpf.mk
@@ -0,0 +1,296 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Build BPF programs and skeleton headers for selftests, then link them into
+# test binaries.
+#
+# Use it from a test Makefile like this:
+#
+#     BPF_SRCS         := foo.bpf.c bar.bpf.c
+#     TEST_GEN_PROGS   := foo_test
+#     OVERRIDE_TARGETS := 1              # set before lib.mk
+#     include ../lib.mk
+#     include ../lib.bpf.mk
+#
+#     $(OUTPUT)/foo_test: foo_test.c $(BPF_SKELS) $(BPFOBJ)
+#         $(call bpf_link,$@,$<)
+#
+# Keep $(BPFOBJ) in the dependencies so make relinks the test when libbpf.a
+# changes.
+#
+# Options to set before including lib.bpf.mk:
+#   BPF_PROG_EXT     - source ending; default .bpf.c. Use .c for progs/foo.c.
+#   BPF_EXTRA_HDRS   - more headers needed by the BPF objects.
+#   BPF_EXTRA_CFLAGS - more flags for compiling BPF programs.
+#   BPF_SKEL_EXT     - skeleton header ending; default .skel.h.
+#   BPF_GEN_SUBSKEL  - also create a subskeleton header when set.
+#   BPF_OBJ_DIR      - folder for BPF objects; default $(OUTPUT).
+#   BPF_SKEL_DIR     - folder for skeleton headers; default $(OUTPUT).
+#
+# You may replace BPF_CFLAGS after including this file. Make reads it when each
+# object is built. Sources may be in subfolders, but generated files use only
+# the source filename and are placed directly in their output folder.
+#
+# Include lib.mk first. This file uses OUTPUT, top_srcdir, CC, CLANG, Q and msg.
+# OVERRIDE_TARGETS stops lib.mk from adding its normal link rule. This file sets
+# BPFOBJ, BPFTOOL, BPF_OBJS, BPF_SKELS and BPF_LDLIBS, and adds to CFLAGS and
+# EXTRA_CLEAN.
+
+include $(top_srcdir)/tools/scripts/Makefile.arch	# ARCH / SRCARCH / HOSTARCH
+
+# Some commands write bpftool output straight to $@. Remove a partial file when
+# a command fails.
+.DELETE_ON_ERROR:
+
+# Use matching host tools for libbpf and bpftool. This keeps an LLVM build from
+# mixing Clang and GCC. Makefile.include is not used because it would also
+# change the test's AR, LD, CXX and CFLAGS.
+ifneq ($(LLVM),)
+HOSTCC ?= $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
+HOSTLD ?= $(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX)
+else
+HOSTCC ?= gcc
+HOSTLD ?= ld
+endif
+CLANG  ?= clang
+
+ifneq ($(V),1)
+submake_extras := feature_display=0
+endif
+
+# Pass RELEASE, OPT_FLAGS and EXTRA_CFLAGS to the libbpf and bpftool builds,
+# matching selftests/bpf/Makefile.
+OPT_FLAGS ?= $(if $(RELEASE),-O2,-O0)
+
+# ---- files and tools ------------------------------------------------------
+# Use one full path. Different forms of the same -I path can change BTF order.
+TOOLSDIR    := $(abspath $(top_srcdir)/tools)
+LIBDIR      := $(TOOLSDIR)/lib
+BPFDIR      := $(LIBDIR)/bpf
+TOOLSINCDIR := $(TOOLSDIR)/include
+BPFTOOLDIR  := $(TOOLSDIR)/bpf/bpftool
+APIDIR      := $(TOOLSINCDIR)/uapi
+
+# Keep private build files under $(OUTPUT) for both in-tree and O= builds.
+SCRATCH_DIR := $(OUTPUT)/tools
+BUILD_DIR   := $(SCRATCH_DIR)/build
+INCLUDE_DIR := $(SCRATCH_DIR)/include
+BPFOBJ      := $(BUILD_DIR)/libbpf/libbpf.a
+
+# bpftool runs on the build host. Build a separate host libbpf when the target
+# architecture is different.
+ifneq ($(CROSS_COMPILE)$(filter-out $(HOSTARCH),$(SRCARCH)),)
+HOST_BUILD_DIR   := $(BUILD_DIR)/host
+HOST_SCRATCH_DIR := $(OUTPUT)/host-tools
+else
+HOST_BUILD_DIR   := $(BUILD_DIR)
+HOST_SCRATCH_DIR := $(SCRATCH_DIR)
+endif
+HOST_BPFOBJ     := $(HOST_BUILD_DIR)/libbpf/libbpf.a
+DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
+BPFTOOL         ?= $(DEFAULT_BPFTOOL)
+
+# When host and target are the same, bpftool uses the target libbpf and needs
+# the same USERCFLAGS. Do not pass USERCFLAGS to separate host tools.
+ifeq ($(BPFOBJ),$(HOST_BPFOBJ))
+HOST_USERCFLAGS := $(USERCFLAGS)
+endif
+
+# ---- find vmlinux BTF -----------------------------------------------------
+VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)				\
+		     $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux)	\
+		     $(top_srcdir)/vmlinux				\
+		     /sys/kernel/btf/vmlinux				\
+		     /boot/vmlinux-$(shell uname -r)
+VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
+# When VMLINUX_H is not set, report a missing vmlinux only while building
+# vmlinux.h. Reporting it here would also make "make clean" fail.
+
+# ---- compiler flags -------------------------------------------------------
+# Find the normal system headers that Clang omits with --target=bpf. Put them
+# last with -idirafter so they cannot replace project headers.
+define get_sys_includes
+$(shell $(1) $(2) -v -E - </dev/null 2>&1 \
+	| sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }') \
+$(shell $(1) $(2) -dM -E - </dev/null | grep '__riscv_xlen ' | awk '{printf("-D__riscv_xlen=%d -D__BITS_PER_LONG=%d", $$3, $$3)}')
+endef
+ifneq ($(CROSS_COMPILE),)
+CLANG_TARGET_ARCH = --target=$(notdir $(CROSS_COMPILE:%-=%))
+endif
+# Find the system include flags once and reuse them.
+CLANG_SYS_INCLUDES := $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
+
+IS_LITTLE_ENDIAN := $(shell $(CC) -dM -E - </dev/null | \
+			grep 'define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__')
+MENDIAN := $(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
+
+# Use BPF CPU v3 when Clang supports it. Otherwise use v2.
+CLANG_BPF_CPU := $(shell $(CLANG) --target=bpf -mcpu=help 2>&1 | grep -q 'v3' \
+			 && echo v3 || echo v2)
+
+# vmlinux.h can contain anonymous struct and union members. Clang needs
+# -fms-extensions to accept them.
+BPF_CFLAGS = -g -Wall -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN)	\
+	     -I$(INCLUDE_DIR) -I$(APIDIR) -I$(TOOLSINCDIR)		\
+	     -std=gnu11							\
+	     -fno-strict-aliasing					\
+	     -fms-extensions -Wno-microsoft-anon-tag			\
+	     -Wno-compare-distinct-pointer-types			\
+	     $(CLANG_SYS_INCLUDES) $(BPF_EXTRA_CFLAGS)
+
+# $1 = source, $2 = object. -MMD -MP records every included header, including
+# headers included by other headers.
+define BPF_BUILD_RULE
+	$(call msg,CLNG-BPF,,$2)
+	$(Q)$(CLANG) $(BPF_CFLAGS) -O2 --target=bpf -mcpu=$(CLANG_BPF_CPU) \
+		-MMD -MP -c $1 -o $2
+endef
+
+# ---- output folders -------------------------------------------------------
+BPF_OBJ_DIR  ?= $(OUTPUT)
+BPF_SKEL_DIR ?= $(OUTPUT)
+
+# ---- build folders --------------------------------------------------------
+MAKE_DIRS := $(sort $(BUILD_DIR)/libbpf $(HOST_BUILD_DIR)/libbpf		\
+		    $(HOST_BUILD_DIR)/bpftool $(INCLUDE_DIR)		\
+		    $(filter-out $(OUTPUT),$(BPF_OBJ_DIR) $(BPF_SKEL_DIR)))
+$(MAKE_DIRS):
+	$(call msg,MKDIR,,$@)
+	$(Q)mkdir -p $@
+
+# ---- target libbpf --------------------------------------------------------
+# Pass ARCH, CROSS_COMPILE and CC because lib.mk does not export CC. -fPIC lets
+# libbpf.a link into PIE test binaries. libbpf reads EXTRA_CFLAGS, so pass
+# USERCFLAGS there too.
+$(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
+	   $(APIDIR)/linux/bpf.h | $(BUILD_DIR)/libbpf
+	$(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
+		    ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) CC="$(CC)"     \
+		    EXTRA_CFLAGS='-g $(OPT_FLAGS) -fPIC $(EXTRA_CFLAGS) $(USERCFLAGS)' \
+		    DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
+
+# ---- host libbpf, only when the target differs ----------------------------
+ifneq ($(BPFOBJ),$(HOST_BPFOBJ))
+$(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
+		$(APIDIR)/linux/bpf.h | $(HOST_BUILD_DIR)/libbpf
+	$(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) ARCH= CROSS_COMPILE=	      \
+		    OUTPUT=$(HOST_BUILD_DIR)/libbpf/ CC="$(HOSTCC)" LD="$(HOSTLD)" \
+		    EXTRA_CFLAGS='-g $(OPT_FLAGS) $(EXTRA_CFLAGS)' \
+		    DESTDIR=$(HOST_SCRATCH_DIR) prefix= all install_headers
+endif
+
+# ---- host bpftool ---------------------------------------------------------
+$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
+		    $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
+	$(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR)		  \
+		    ARCH= CROSS_COMPILE= CC="$(HOSTCC)" LD="$(HOSTLD)" \
+		    EXTRA_CFLAGS='-g $(OPT_FLAGS) $(EXTRA_CFLAGS) $(HOST_USERCFLAGS)' \
+		    EXTRA_LDFLAGS='$(EXTRA_LDFLAGS)'		  \
+		    OUTPUT=$(HOST_BUILD_DIR)/bpftool/		  \
+		    LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/	  \
+		    LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/		  \
+		    prefix= DESTDIR=$(HOST_SCRATCH_DIR)/ install-bin
+
+# ---- build vmlinux.h ------------------------------------------------------
+# Replace vmlinux.h only when its contents change. A new timestamp alone would
+# rebuild every BPF object and skeleton.
+$(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL) | $(INCLUDE_DIR)
+ifeq ($(VMLINUX_H),)
+	$(call msg,GEN,,$@)
+	$(Q)test -n "$(VMLINUX_BTF)" || { \
+		echo "lib.bpf.mk: no vmlinux with BTF at any of \"$(VMLINUX_BTF_PATHS)\"" >&2; \
+		exit 1; }
+	$(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@.tmp
+else
+	$(call msg,CP,,$@)
+	$(Q)cp "$(VMLINUX_H)" $@.tmp
+endif
+	$(Q)cmp -s $@.tmp $@ || mv $@.tmp $@
+	$(Q)rm -f $@.tmp
+
+# ---- BPF objects and skeletons --------------------------------------------
+BPF_PROG_EXT ?= .bpf.c
+# Each source must end with BPF_PROG_EXT so Make can remove that ending.
+bpf_bad_srcs := $(filter-out %$(BPF_PROG_EXT),$(BPF_SRCS))
+ifneq ($(bpf_bad_srcs),)
+$(error lib.bpf.mk: BPF_SRCS entries must end in $(BPF_PROG_EXT): $(bpf_bad_srcs))
+endif
+bpf_stems := $(patsubst %$(BPF_PROG_EXT),%,$(notdir $(BPF_SRCS)))
+# Reject any dot left after removing BPF_PROG_EXT. For example, foo.bpf.c leaves
+# foo.bpf when the ending is set to .c.
+bpf_dotted_stems := $(filter-out $(basename $(bpf_stems)),$(bpf_stems))
+ifneq ($(bpf_dotted_stems),)
+$(error lib.bpf.mk: BPF_SRCS basenames must not contain a dot: $(bpf_dotted_stems))
+endif
+# Generated files use only the source filename. Reject equal filenames instead
+# of letting one replace another.
+ifneq ($(words $(bpf_stems)),$(words $(sort $(bpf_stems))))
+$(error lib.bpf.mk: BPF_SRCS has colliding basenames: $(BPF_SRCS))
+endif
+BPF_SKEL_EXT    ?= .skel.h
+BPF_SUBSKEL_EXT := $(patsubst %skel.h,%subskel.h,$(BPF_SKEL_EXT))
+# BPF_SKEL_EXT must end in skel.h so the subskeleton gets a different name.
+ifneq ($(BPF_GEN_SUBSKEL),)
+ifeq ($(BPF_SUBSKEL_EXT),$(BPF_SKEL_EXT))
+$(error lib.bpf.mk: BPF_GEN_SUBSKEL needs BPF_SKEL_EXT to end in skel.h: $(BPF_SKEL_EXT))
+endif
+endif
+BPF_OBJS  := $(addprefix $(BPF_OBJ_DIR)/,$(addsuffix .bpf.o,$(bpf_stems)))
+BPF_SKELS := $(addprefix $(BPF_SKEL_DIR)/,$(addsuffix $(BPF_SKEL_EXT),$(bpf_stems)))
+
+# Make one rule for each source. A global vpath would also match the test's
+# normal C rules when BPF_PROG_EXT is .c.
+define bpf_obj_rule
+$(BPF_OBJ_DIR)/$(patsubst %$(BPF_PROG_EXT),%,$(notdir $(1))).bpf.o: $(1)	\
+		$(BPF_EXTRA_HDRS) $(INCLUDE_DIR)/vmlinux.h			\
+		| $(BPF_OBJ_DIR) $(BPFOBJ)
+	$$(call BPF_BUILD_RULE,$$<,$$@)
+endef
+$(foreach src,$(BPF_SRCS),$(eval $(call bpf_obj_rule,$(src))))
+
+# Link three times and compare the last two outputs. A correct linker must not
+# change an object that it already linked.
+$(BPF_SKELS): $(BPF_SKEL_DIR)/%$(BPF_SKEL_EXT): $(BPF_OBJ_DIR)/%.bpf.o $(BPFTOOL) | $(BPF_SKEL_DIR)
+	$(call msg,GEN-SKEL,,$@)
+	$(Q)$(BPFTOOL) gen object $(<:.o=.linked1.o) $<
+	$(Q)$(BPFTOOL) gen object $(<:.o=.linked2.o) $(<:.o=.linked1.o)
+	$(Q)$(BPFTOOL) gen object $(<:.o=.linked3.o) $(<:.o=.linked2.o)
+	$(Q)diff $(<:.o=.linked2.o) $(<:.o=.linked3.o)
+	$(Q)$(BPFTOOL) gen skeleton $(<:.o=.linked3.o) name $(notdir $(<:.bpf.o=)) > $@
+ifneq ($(BPF_GEN_SUBSKEL),)
+	$(Q)$(BPFTOOL) gen subskeleton $(<:.o=.linked3.o) name $(notdir $(<:.bpf.o=)) > $(@:$(BPF_SKEL_EXT)=$(BPF_SUBSKEL_EXT))
+endif
+	$(Q)rm -f $(<:.o=.linked1.o) $(<:.o=.linked2.o) $(<:.o=.linked3.o)
+
+# Read the header dependencies written by -MMD.
+-include $(BPF_OBJS:.o=.d)
+
+# ---- values for the test Makefile -----------------------------------------
+# Add paths for generated skeletons and vmlinux.h. Put the new libbpf headers
+# first so an older copy under BPF_SKEL_DIR cannot be used by mistake.
+CFLAGS += -I$(INCLUDE_DIR) -I$(OUTPUT) -I$(BPF_SKEL_DIR)
+
+# Add target zstd when pkg-config finds it. A cross build must use the target
+# pkg-config, not the host one.
+PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config
+BPF_LDLIBS := $(BPFOBJ) -lelf -lz
+ifneq ($(shell $(PKG_CONFIG) --exists libzstd 2>/dev/null && echo y),)
+BPF_LDLIBS += -lzstd
+endif
+
+# lib.mk made its "all" list before this file was included. Add the skeletons
+# here so they are built.
+all: $(BPF_SKELS)
+
+# The skeleton already embeds each BPF object in the test binary. To install the
+# objects separately, add TEST_GEN_FILES += $(BPF_OBJS).
+
+# $1 = binary, $2 = test source. Use the same compile and link flags as lib.mk.
+define bpf_link
+	$(call msg,BINARY,,$1)
+	$(Q)$(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $2 \
+		$(BPF_LDLIBS) $(LDLIBS) -o $1
+endef
+
+EXTRA_CLEAN += $(sort $(SCRATCH_DIR) $(HOST_SCRATCH_DIR))		\
+	       $(addprefix $(BPF_OBJ_DIR)/,*.bpf.o *.bpf.d *.linked*.o)	\
+	       $(addprefix $(BPF_SKEL_DIR)/,*$(BPF_SKEL_EXT) *$(BPF_SUBSKEL_EXT))
-- 
2.53.0-Meta

[PATCH v3 2/4] selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush

[Ziyang Men]

This Patch adds a new selftest, selftests/cgroup/test_memcg_stat_cross_cpu,
which checks the cgroup flush happens correctly by comparing the bpf
read value matches the file reading.

Existing approach in cgroup_iter_memcg only checks whether the read
value is non-zero, rather than compare it with the actually ones, e.g.,

  ASSERT_GT(memcg_query->nr_file_pages, 0, "final file value");
  ASSERT_GT(memcg_query->nr_file_mapped, 0, "final file mapped value");

This ensures the flush does happen and take effect, but it does not
check the flush take effect as expected. And this is why we want to
compare with the file reading: it is a stronger evidence for the
correctness of the flush function.

Besides, another check missed from the cgroup_iter_memcg is that it
contains no accumulation tests for a cgroup subtree. Currently it only
creates one children cgroup and puts the reading task in it. The flush
never have a chance to merge-up the changes in the leaves.

Another missing parts for the cgroup_iter_memcg is that it charges the
memory and then reads the stats on the same cpu, which means a (possible
broken) flush that processed only that CPU would pass the weak non-zero
test. The new tests strengthen the check by pin each child to multiple
CPUs. This increase the code length but greatly improves the test
coverage.

Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
---
 tools/testing/selftests/cgroup/.gitignore     |   8 +
 tools/testing/selftests/cgroup/Makefile       |  52 ++
 tools/testing/selftests/cgroup/config         |   5 +
 .../selftests/cgroup/lib/cgroup_util.c        |  83 +++
 .../cgroup/lib/include/cgroup_util.h          |   3 +
 .../cgroup/memcg_stat_cross_cpu.bpf.c         |  86 +++
 .../selftests/cgroup/memcg_stat_cross_cpu.h   |  21 +
 .../cgroup/test_memcg_stat_cross_cpu.c        | 596 ++++++++++++++++++
 .../selftests/cgroup/test_memcontrol.c        |  29 +-
 9 files changed, 858 insertions(+), 25 deletions(-)
 create mode 100644 tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
 create mode 100644 tools/testing/selftests/cgroup/memcg_stat_cross_cpu.h
 create mode 100644 tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c

diff --git a/tools/testing/selftests/cgroup/.gitignore b/tools/testing/selftests/cgroup/.gitignore
index 952e4448bf07..561a3e891b98 100644
--- a/tools/testing/selftests/cgroup/.gitignore
+++ b/tools/testing/selftests/cgroup/.gitignore
@@ -6,7 +6,15 @@ test_freezer
 test_hugetlb_memcg
 test_kill
 test_kmem
+test_memcg_stat_cross_cpu
 test_memcontrol
 test_pids
 test_zswap
 wait_inotify
+# Artifacts generated by lib.bpf.mk
+/tools
+/host-tools
+*.bpf.o
+*.bpf.d
+*.linked*.o
+*.skel.h
diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
index e01584c2189a..01927511c059 100644
--- a/tools/testing/selftests/cgroup/Makefile
+++ b/tools/testing/selftests/cgroup/Makefile
@@ -20,9 +20,51 @@ TEST_GEN_PROGS += test_zswap
 
 LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
 
+# test_memcg_stat_cross_cpu needs clang and a vmlinux with BTF; build it only
+# where both exist, since every other test here needs just a C compiler.
+# lib.mk settles CLANG from LLVM=, but not before OVERRIDE_TARGETS is needed.
+ifneq ($(filter %/,$(LLVM)),)
+CLANG ?= $(LLVM)clang
+else
+CLANG ?= clang$(filter -%,$(LLVM))
+endif
+READELF ?= $(CROSS_COMPILE)readelf
+VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)				\
+		     $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux)	\
+		     ../../../../vmlinux				\
+		     /sys/kernel/btf/vmlinux				\
+		     /boot/vmlinux-$(shell uname -r)
+# A vmlinux built without CONFIG_DEBUG_INFO_BTF has nothing to dump, and taking
+# it would fail this whole directory rather than just skip the BPF test.  Pick
+# the first candidate that carries BTF; /sys/kernel/btf/vmlinux is raw BTF
+# rather than an ELF file, so it needs no check.
+VMLINUX_BTF ?= $(abspath $(firstword $(foreach v,$(wildcard $(VMLINUX_BTF_PATHS)),\
+		 $(if $(filter /sys/kernel/btf/%,$(v)),$(v),\
+		   $(if $(shell $(READELF) -S "$(v)" 2>/dev/null | grep -F .BTF),$(v))))))
+# Either of the two knobs lib.bpf.mk takes instead of a vmlinux will do.
+HAVE_BPF := $(and $(or $(VMLINUX_H),$(VMLINUX_BTF)),			\
+		  $(shell command -v $(CLANG) 2>/dev/null))
+
+ifneq ($(HAVE_BPF),)
+TEST_GEN_PROGS += test_memcg_stat_cross_cpu
+BPF_SRCS := memcg_stat_cross_cpu.bpf.c
+# Suppresses lib.mk's default C link rule, re-supplied below.
+OVERRIDE_TARGETS := 1
+endif
+
 include ../lib.mk
 include lib/libcgroup.mk
 
+ifneq ($(HAVE_BPF),)
+include ../lib.bpf.mk
+
+# The default link rule for the plain tests, which OVERRIDE_TARGETS removed.
+LOCAL_HDRS += $(selfdir)/kselftest_harness.h $(selfdir)/kselftest.h
+$(OUTPUT)/%: %.c $(LOCAL_HDRS)
+	$(call msg,CC,,$@)
+	$(Q)$(LINK.c) $(filter-out $(LOCAL_HDRS),$^) $(LDLIBS) -o $@
+endif
+
 $(OUTPUT)/test_core: $(LIBCGROUP_O)
 $(OUTPUT)/test_cpu: $(LIBCGROUP_O)
 $(OUTPUT)/test_cpuset: $(LIBCGROUP_O)
@@ -33,3 +75,13 @@ $(OUTPUT)/test_kmem: $(LIBCGROUP_O)
 $(OUTPUT)/test_memcontrol: $(LIBCGROUP_O)
 $(OUTPUT)/test_pids: $(LIBCGROUP_O)
 $(OUTPUT)/test_zswap: $(LIBCGROUP_O)
+
+ifneq ($(HAVE_BPF),)
+# Links cgroup_util and the skeleton against the libbpf lib.bpf.mk built.  The
+# uapi include is private to this binary: the other tests here must keep
+# resolving <linux/*.h> against the system headers.
+$(OUTPUT)/test_memcg_stat_cross_cpu: private CFLAGS += -I$(top_srcdir)/tools/include/uapi
+$(OUTPUT)/test_memcg_stat_cross_cpu: test_memcg_stat_cross_cpu.c \
+					$(BPF_SKELS) $(LIBCGROUP_O) $(BPFOBJ)
+	$(call bpf_link,$@,$< $(LIBCGROUP_O))
+endif
diff --git a/tools/testing/selftests/cgroup/config b/tools/testing/selftests/cgroup/config
index 39f979690dd3..356b6a533d88 100644
--- a/tools/testing/selftests/cgroup/config
+++ b/tools/testing/selftests/cgroup/config
@@ -4,3 +4,8 @@ CONFIG_CGROUP_FREEZER=y
 CONFIG_CGROUP_SCHED=y
 CONFIG_MEMCG=y
 CONFIG_PAGE_COUNTER=y
+CONFIG_BPF=y
+CONFIG_BPF_SYSCALL=y
+CONFIG_DEBUG_INFO=y
+CONFIG_DEBUG_INFO_BTF=y
+CONFIG_DEBUG_INFO_DWARF4=y
diff --git a/tools/testing/selftests/cgroup/lib/cgroup_util.c b/tools/testing/selftests/cgroup/lib/cgroup_util.c
index 2596c12cd864..0ae76e75e7f8 100644
--- a/tools/testing/selftests/cgroup/lib/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/lib/cgroup_util.c
@@ -54,6 +54,89 @@ ssize_t write_text(const char *path, char *buf, ssize_t len)
 	return len < 0 ? -errno : len;
 }
 
+/**
+ * cg_get_id - return the kernfs id of a cgroup directory
+ * @cgroup: absolute path to the cgroup directory
+ *
+ * This is what the kernel gives BPF as cgrp->kn->id, taken from the cgroupfs
+ * file handle.  It is not the directory's st_ino.
+ *
+ * Return: The cgroup id, or 0 on failure.
+ */
+unsigned long long cg_get_id(const char *cgroup)
+{
+	union {
+		unsigned long long id;
+		unsigned char raw[8];
+	} handle;
+	struct file_handle *fhp, *fhp2;
+	int mount_id, fhsize, err;
+	unsigned long long ret = 0;
+
+	fhsize = sizeof(*fhp);
+	fhp = calloc(1, fhsize);
+	if (!fhp)
+		return 0;
+
+	/* The probe fails and reports the size; a cgroupfs handle is 8 bytes. */
+	err = name_to_handle_at(AT_FDCWD, cgroup, fhp, &mount_id, 0);
+	if (err >= 0 || fhp->handle_bytes != 8)
+		goto out;
+
+	fhsize = sizeof(*fhp) + fhp->handle_bytes;
+	fhp2 = realloc(fhp, fhsize);
+	if (!fhp2)
+		goto out;
+	fhp = fhp2;
+
+	if (name_to_handle_at(AT_FDCWD, cgroup, fhp, &mount_id, 0) < 0)
+		goto out;
+
+	memcpy(handle.raw, fhp->f_handle, 8);
+	ret = handle.id;
+out:
+	free(fhp);
+	return ret;
+}
+
+/**
+ * cg_touch_pages - write to every page of a region so it becomes resident
+ * @buf: start of the region
+ * @size: length of the region in bytes
+ *
+ * The pages are charged to the calling task's cgroup, on the cpu it runs on.
+ */
+void cg_touch_pages(char *buf, size_t size)
+{
+	long page_size = sysconf(_SC_PAGESIZE);
+	char *ptr;
+
+	if (page_size <= 0)
+		page_size = BUF_SIZE;
+
+	for (ptr = buf; ptr < buf + size; ptr += page_size)
+		*ptr = 0;
+}
+
+/**
+ * cg_alloc_anon - allocate anonymous memory and fault it in
+ * @size: bytes to allocate
+ *
+ * Return: The region, to be released with free(), or NULL.
+ */
+char *cg_alloc_anon(size_t size)
+{
+	char *buf = malloc(size);
+
+	if (!buf) {
+		fprintf(stderr, "malloc() failed\n");
+		return NULL;
+	}
+
+	cg_touch_pages(buf, size);
+	return buf;
+}
+
 char *cg_name(const char *root, const char *name)
 {
 	size_t len = strlen(root) + strlen(name) + 2;
diff --git a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
index 8ebb2b4d4ec0..640778018780 100644
--- a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
+++ b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
@@ -54,6 +54,9 @@ extern ssize_t write_text(const char *path, char *buf, ssize_t len);
 extern int cg_find_controller_root(char *root, size_t len, const char *controller);
 extern int cg_find_unified_root(char *root, size_t len, bool *nsdelegate);
 extern char *cg_name(const char *root, const char *name);
+extern unsigned long long cg_get_id(const char *cgroup);
+extern void cg_touch_pages(char *buf, size_t size);
+extern char *cg_alloc_anon(size_t size);
 extern char *cg_name_indexed(const char *root, const char *name, int index);
 extern char *cg_control(const char *cgroup, const char *control);
 extern int cg_create(const char *cgroup);
diff --git a/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
new file mode 100644
index 000000000000..3c2acd49c16b
--- /dev/null
+++ b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#define BPF_NO_KFUNC_PROTOTYPES
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
+#include "memcg_stat_cross_cpu.h"
+
+char _license[] SEC("license") = "GPL";
+
+/*
+ * Declared here rather than taken from vmlinux.h, which only has them if the
+ * build host's own kernel does.  A kernel without them is caught at run time.
+ */
+struct mem_cgroup *bpf_get_mem_cgroup(struct cgroup_subsys_state *css) __ksym;
+void bpf_put_mem_cgroup(struct mem_cgroup *memcg) __ksym;
+void bpf_mem_cgroup_flush_stats(struct mem_cgroup *memcg) __ksym;
+unsigned long bpf_mem_cgroup_page_state(struct mem_cgroup *memcg, int idx) __ksym;
+unsigned long bpf_mem_cgroup_vm_events(struct mem_cgroup *memcg,
+				       enum vm_event_item event) __ksym;
+
+/*
+ * Results keyed by cgroup id, which is the same value cg_get_id() returns.
+ * Userspace resizes the map to the subtree before load.
+ */
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(max_entries, 1);
+	__type(key, __u64);
+	__type(value, struct memcg_stat_snapshot);
+} results SEC(".maps");
+
+/*
+ * Flush once at the subtree root, then read each cgroup through the kfuncs.
+ * iter.s because the flush kfunc may sleep.
+ */
+SEC("iter.s/cgroup")
+int cgroup_memcg_stat_cross_cpu(struct bpf_iter__cgroup *ctx)
+{
+	struct cgroup *cgrp = ctx->cgroup;
+	struct memcg_stat_snapshot snap = {};
+	struct cgroup_subsys_state *css;
+	struct mem_cgroup *memcg;
+	int idx_anon, idx_file, idx_shmem, idx_fmapped, idx_pgfault;
+	__u64 cg_id;
+
+	/* The walk ends with a NULL element; return 0 so it runs to the end. */
+	if (!cgrp)
+		return 0;
+
+	css = &cgrp->self;
+	memcg = bpf_get_mem_cgroup(css);
+	if (!memcg)
+		return 0;
+
+	/*
+	 * seq_num 0 is the subtree root in DESCENDANTS_PRE order, and the flush
+	 * is subtree-wide, so one call brings every descendant up to date.
+	 */
+	if (ctx->meta->seq_num == 0)
+		bpf_mem_cgroup_flush_stats(memcg);
+
+	cg_id = BPF_CORE_READ(cgrp, kn, id);
+	snap.cgroup_id = cg_id;
+
+	idx_anon = bpf_core_enum_value(enum node_stat_item, NR_ANON_MAPPED);
+	idx_file = bpf_core_enum_value(enum node_stat_item, NR_FILE_PAGES);
+	idx_shmem = bpf_core_enum_value(enum node_stat_item, NR_SHMEM);
+	idx_fmapped = bpf_core_enum_value(enum node_stat_item, NR_FILE_MAPPED);
+	idx_pgfault = bpf_core_enum_value(enum vm_event_item, PGFAULT);
+
+	snap.anon = bpf_mem_cgroup_page_state(memcg, idx_anon);
+	snap.file = bpf_mem_cgroup_page_state(memcg, idx_file);
+	snap.shmem = bpf_mem_cgroup_page_state(memcg, idx_shmem);
+	snap.file_mapped = bpf_mem_cgroup_page_state(memcg, idx_fmapped);
+	snap.pgfault = bpf_mem_cgroup_vm_events(memcg, idx_pgfault);
+
+	/* page_counter fields need no kfunc; read them off the trusted ptr. */
+	snap.usage_pages = BPF_CORE_READ(memcg, memory.usage.counter);
+	snap.max_pages = BPF_CORE_READ(memcg, memory.max);
+
+	bpf_map_update_elem(&results, &cg_id, &snap, BPF_ANY);
+
+	bpf_put_mem_cgroup(memcg);
+	return 0;
+}
diff --git a/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.h b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.h
new file mode 100644
index 000000000000..7177ab3b9380
--- /dev/null
+++ b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+#ifndef __MEMCG_STAT_CROSS_CPU_H
+#define __MEMCG_STAT_CROSS_CPU_H
+
+/*
+ * One per-cgroup snapshot, written by the BPF program and read by the test, so
+ * the fields are fixed-width rather than the long the other cgroup tests use.
+ */
+struct memcg_stat_snapshot {
+	__u64 cgroup_id;
+	__u64 anon;		/* NR_ANON_MAPPED, bytes */
+	__u64 file;		/* NR_FILE_PAGES, bytes */
+	__u64 shmem;		/* NR_SHMEM, bytes */
+	__u64 file_mapped;	/* NR_FILE_MAPPED, bytes */
+	__u64 pgfault;		/* PGFAULT, count */
+	__u64 usage_pages;	/* page_counter memory.usage, in PAGES */
+	__u64 max_pages;	/* page_counter memory.max, in PAGES */
+};
+
+#endif /* __MEMCG_STAT_CROSS_CPU_H */
diff --git a/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c b/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c
new file mode 100644
index 000000000000..9946c41c7a90
--- /dev/null
+++ b/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c
@@ -0,0 +1,596 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
+
+/*
+ * Compare the memcg BPF kfuncs with memory.stat over a cgroup subtree whose
+ * charge is scattered across many CPUs.
+ *
+ * One child per leaf faults a region on K different CPUs and holds it, then the
+ * subtree is read by a SEC("iter.s/cgroup") program and from memory.stat.  The
+ * two readers must return the same values, which checks the readers.  The flush
+ * is checked by two invariants: each leaf's anon covers the charge it holds, and
+ * the root's equals the sum of the leaves'.  Comparing the readers cannot check
+ * the flush, because a flush clears the pending-update count the next one is
+ * gated on, so the second reader returns what the first one left.
+ *
+ * The comparison is exact, which needs the subtree quiesced: only this test's
+ * children charge it, and they block once their memory is faulted in.  Global
+ * reclaim would move the numbers and the test would report a mismatch.
+ * file, shmem and file_mapped are 0 under this workload; they are compared to
+ * keep the field set complete, not because the workload produces them.
+ */
+#define _GNU_SOURCE
+
+#include <linux/limits.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdint.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/wait.h>
+
+#include <linux/bpf.h>
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+#include <bpf/btf.h>
+
+#include "kselftest.h"
+#include "cgroup_util.h"
+#include "memcg_stat_cross_cpu.h"
+#include "memcg_stat_cross_cpu.skel.h"
+
+#define SUBTREE_NAME		"mcg_xcpu"
+
+#define MEMCG_CHARGE_BATCH	64
+
+#define FLUSH_MARGIN		4
+
+#define CHARGE_WAIT_RETRIES	100
+
+static char root[PATH_MAX];
+static char *subtree_root;
+static long page_size;
+
+/* Values parsed from memory.stat / memory.current, in bytes. */
+struct file_snap {
+	long anon, file, shmem, file_mapped, pgfault;
+	long current;
+};
+
+struct cg_node {
+	char path[PATH_MAX];
+	unsigned long long id;
+	bool is_leaf;			/* holds a charge of its own */
+	struct memcg_stat_snapshot bpf;	/* read through the kfuncs */
+	struct file_snap file;		/* read from memory.stat */
+};
+
+static struct cg_node *nodes;	/* DFS order: a parent precedes its children */
+static int n_nodes;
+static int n_leaves;
+
+/* ---- CPUs this test may run on ----------------------------------------- */
+
+static cpu_set_t allowed_cpus;
+static int n_cpu;
+
+static int nth_cpu(int n)
+{
+	int i, seen = 0;
+
+	for (i = 0; i < CPU_SETSIZE; i++) {
+		if (!CPU_ISSET(i, &allowed_cpus))
+			continue;
+		if (seen++ == n % n_cpu)
+			return i;
+	}
+	return -1;
+}
+
+static int pin_cpu(int cpu)
+{
+	cpu_set_t set;
+
+	if (cpu < 0)
+		return -1;
+
+	CPU_ZERO(&set);
+	CPU_SET(cpu, &set);
+	return sched_setaffinity(0, sizeof(set), &set);
+}
+
+/* ---- tree construction -------------------------------------------------- */
+
+static int add_node(const char *path, bool is_leaf)
+{
+	struct cg_node *n = &nodes[n_nodes];
+
+	if (cg_create(path))
+		return -1;
+
+	strncpy(n->path, path, sizeof(n->path) - 1);
+	n->id = cg_get_id(path);
+	n->is_leaf = is_leaf;
+	if (is_leaf)
+		n_leaves++;
+	n_nodes++;
+	return 0;
+}
+
+/* Create # @levels more levels below @path.
+ * Only the last level is charged.
+ */
+static int build_children(const char *path, int fanout, int levels)
+{
+	char child[PATH_MAX];
+	int i;
+
+	if (levels == 0)
+		return 0;
+
+	/* Enable memory on this interior node so its children get a memcg. */
+	if (cg_write(path, "cgroup.subtree_control", "+memory"))
+		return -1;
+
+	for (i = 0; i < fanout; i++) {
+		snprintf(child, sizeof(child), "%s/c%d", path, i);
+		if (add_node(child, levels == 1))
+			return -1;
+		if (build_children(child, fanout, levels - 1))
+			return -1;
+	}
+	return 0;
+}
+
+static size_t tree_capacity(int fanout, int depth)
+{
+	size_t total = 1, level = 1;
+	int d;
+
+	for (d = 0; d < depth; d++) {
+		level *= fanout;
+		total += level;
+	}
+	return total;
+}
+
+static int build_tree(int fanout, int depth, int *root_fd)
+{
+	n_nodes = 0;
+	n_leaves = 0;
+	nodes = calloc(tree_capacity(fanout, depth), sizeof(*nodes));
+	if (!nodes)
+		return -1;
+
+	if (add_node(subtree_root, depth == 0))
+		return -1;
+
+	*root_fd = open(subtree_root, O_RDONLY);
+	if (*root_fd < 0)
+		return -1;
+
+	return build_children(subtree_root, fanout, depth);
+}
+
+/*
+ * Remove in reverse creation order, so a child always goes before its parent.
+ * cg_destroy() kills the charging children; reap them afterwards.
+ */
+static void destroy_tree(void)
+{
+	int i;
+
+	if (!nodes)
+		return;
+
+	for (i = n_nodes - 1; i >= 0; i--)
+		cg_destroy(nodes[i].path);
+	free(nodes);
+	nodes = NULL;
+
+	while (waitpid(-1, NULL, 0) > 0)
+		;
+}
+
+/* ---- cross-CPU charge (one child per leaf) ------------------------------ */
+
+struct charge_args {
+	size_t bytes;	/* anon this leaf holds */
+	int base;	/* index of the first CPU to fault on */
+	int k;		/* CPUs to spread the charge over */
+};
+
+static int charge_leaf(const char *cgroup, void *arg)
+{
+	const struct charge_args *ca = arg;
+	int ppid = getppid();
+	size_t per, off;
+	char *buf;
+	int j;
+
+	buf = malloc(ca->bytes);
+	if (!buf) {
+		fprintf(stderr, "malloc() failed\n");
+		return -1;
+	}
+
+	/* Whole pages, or several slices would share one page and one CPU. */
+	per = ca->bytes / ca->k / page_size * page_size;
+
+	for (j = 0; j < ca->k; j++) {
+		off = (size_t)j * per;
+		if (pin_cpu(nth_cpu(ca->base + j))) {
+			free(buf);
+			return -1;
+		}
+		cg_touch_pages(buf + off,
+			       j == ca->k - 1 ? ca->bytes - off : per);
+	}
+
+	while (getppid() == ppid)
+		sleep(1);
+
+	free(buf);
+	return 0;
+}
+
+static size_t leaf_charge(size_t want, int k)
+{
+	long online = sysconf(_SC_NPROCESSORS_ONLN);
+	size_t floor;
+
+	floor = (size_t)FLUSH_MARGIN * MEMCG_CHARGE_BATCH * online *
+		page_size / n_leaves;
+	if (want < floor)
+		want = floor;
+	if (want < (size_t)k * page_size)
+		want = (size_t)k * page_size;
+
+	return (want + page_size - 1) / page_size * page_size;
+}
+
+static int start_chargers(int k, size_t bytes)
+{
+	struct charge_args ca = { .bytes = bytes, .k = k };
+	long prev, cur;
+	int i, retries;
+
+	for (i = 0; i < n_nodes; i++) {
+		if (!nodes[i].is_leaf)
+			continue;
+		if (cg_run_nowait(nodes[i].path, charge_leaf, &ca) < 0) {
+			ksft_print_msg("cannot start a charger on %s\n",
+				       nodes[i].path);
+			return -1;
+		}
+		ca.base += k;
+	}
+
+	for (i = 0; i < n_nodes; i++) {
+		if (!nodes[i].is_leaf)
+			continue;
+		/*
+		 * Wait for the charge to both cover the region and stop
+		 * moving.
+		 */
+		prev = -1;
+		for (retries = CHARGE_WAIT_RETRIES; retries; retries--) {
+			cur = cg_read_long(nodes[i].path, "memory.current");
+			if (cur >= (long)bytes && cur == prev)
+				break;
+			prev = cur;
+			usleep(DEFAULT_WAIT_INTERVAL_US / 10);
+		}
+		if (!retries) {
+			ksft_print_msg("%s never reached its charge\n",
+				       nodes[i].path);
+			return -1;
+		}
+	}
+	return 0;
+}
+
+/* ---- the two readers ---------------------------------------------------- */
+
+/*
+ * Collect what the kfuncs report.  The program flushes at the first cgroup it
+ * sees, the subtree root in DESCENDANTS_PRE order.
+ */
+static int read_bpf(int root_fd)
+{
+	DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
+	struct memcg_stat_cross_cpu *skel = NULL;
+	union bpf_iter_link_info linfo = {};
+	struct bpf_link *link = NULL;
+	int ret = -1, i, mfd, fd;
+	char buf[4096];
+	ssize_t r;
+
+	skel = memcg_stat_cross_cpu__open();
+	if (!skel) {
+		ksft_print_msg("skel open failed\n");
+		return -1;
+	}
+	if (bpf_map__set_max_entries(skel->maps.results, n_nodes + 8)) {
+		ksft_print_msg("set max_entries failed\n");
+		goto out;
+	}
+	if (memcg_stat_cross_cpu__load(skel)) {
+		ksft_print_msg("skel load failed\n");
+		goto out;
+	}
+
+	linfo.cgroup.cgroup_fd = root_fd;
+	linfo.cgroup.order = BPF_CGROUP_ITER_DESCENDANTS_PRE;
+	opts.link_info = &linfo;
+	opts.link_info_len = sizeof(linfo);
+
+	link = bpf_program__attach_iter(skel->progs.cgroup_memcg_stat_cross_cpu,
+					&opts);
+	if (!link) {
+		ksft_print_msg("attach iter failed\n");
+		goto out;
+	}
+
+	fd = bpf_iter_create(bpf_link__fd(link));
+	if (fd < 0) {
+		ksft_print_msg("bpf_iter_create failed\n");
+		goto out;
+	}
+	while ((r = read(fd, buf, sizeof(buf))) > 0)
+		;
+	close(fd);
+	if (r) {
+		ksft_print_msg("bpf walk failed\n");
+		goto out;
+	}
+
+	mfd = bpf_map__fd(skel->maps.results);
+	for (i = 0; i < n_nodes; i++)
+		if (bpf_map_lookup_elem(mfd, &nodes[i].id, &nodes[i].bpf)) {
+			ksft_print_msg("no map entry for %s\n", nodes[i].path);
+			goto out;
+		}
+	ret = 0;
+out:
+	bpf_link__destroy(link);
+	memcg_stat_cross_cpu__destroy(skel);
+	return ret;
+}
+
+/* Read the same numbers from cgroupfs, on the same tree, right afterwards. */
+static int read_files(void)
+{
+	int i;
+
+	for (i = 0; i < n_nodes; i++) {
+		const char *path = nodes[i].path;
+		struct file_snap *f = &nodes[i].file;
+
+		f->anon = cg_read_key_long(path, "memory.stat", "anon ");
+		f->file = cg_read_key_long(path, "memory.stat", "file ");
+		f->shmem = cg_read_key_long(path, "memory.stat", "shmem ");
+		f->file_mapped = cg_read_key_long(path, "memory.stat",
+						  "file_mapped ");
+		f->pgfault = cg_read_key_long(path, "memory.stat", "pgfault ");
+		f->current = cg_read_long(path, "memory.current");
+
+		if (f->anon < 0 || f->file < 0 || f->shmem < 0 ||
+		    f->file_mapped < 0 || f->pgfault < 0 || f->current < 0) {
+			ksft_print_msg("reading the stats of %s failed\n", path);
+			return -1;
+		}
+	}
+	return 0;
+}
+
+/* ---- comparison --------------------------------------------------------- */
+
+static void dump_node(const struct cg_node *n)
+{
+	ksft_print_msg("%s bpf : anon=%llu file=%llu shmem=%llu fmapped=%llu pgfault=%llu\n",
+		       n->path, n->bpf.anon, n->bpf.file, n->bpf.shmem,
+		       n->bpf.file_mapped, n->bpf.pgfault);
+	ksft_print_msg("%s file: anon=%ld file=%ld shmem=%ld fmapped=%ld pgfault=%ld\n",
+		       n->path, n->file.anon, n->file.file, n->file.shmem,
+		       n->file.file_mapped, n->file.pgfault);
+}
+
+/* The subtree is quiesced, so the two readers must agree exactly. */
+#define STAT_TOLERANCE_PCT	0
+
+static int check_tree(size_t charged)
+{
+	unsigned long long root_anon = 0, leaf_anon = 0;
+	int i, bad = 0;
+
+	for (i = 0; i < n_nodes; i++) {
+		const struct cg_node *n = &nodes[i];
+		const struct memcg_stat_snapshot *b = &n->bpf;
+
+		if (!values_close(b->anon, n->file.anon, STAT_TOLERANCE_PCT) ||
+		    !values_close(b->file, n->file.file, STAT_TOLERANCE_PCT) ||
+		    !values_close(b->shmem, n->file.shmem, STAT_TOLERANCE_PCT) ||
+		    !values_close(b->file_mapped, n->file.file_mapped,
+				  STAT_TOLERANCE_PCT) ||
+		    !values_close(b->pgfault, n->file.pgfault,
+				  STAT_TOLERANCE_PCT)) {
+			ksft_print_msg("kfuncs disagree with memory.stat\n");
+			dump_node(n);
+			bad++;
+		}
+
+		/*
+		 * Usage is a live page_counter read, not a flushed statistic,
+		 * so it is only a bound.
+		 */
+		if (b->anon > b->usage_pages * (unsigned long long)page_size ||
+		    n->file.anon > n->file.current) {
+			ksft_print_msg("%s: anon above usage: bpf %llu/%llu file %ld/%ld\n",
+				       n->path, b->anon,
+				       b->usage_pages * (unsigned long long)page_size,
+				       n->file.anon, n->file.current);
+			bad++;
+		}
+
+		if (n->is_leaf) {
+			if (b->anon < charged) {
+				ksft_print_msg("%s: flushed anon %llu, charged %zu\n",
+					       n->path, b->anon, charged);
+				bad++;
+			}
+			leaf_anon += b->anon;
+		}
+		if (i == 0)
+			root_anon = b->anon;
+	}
+
+	if (root_anon != leaf_anon) {
+		ksft_print_msg("subtree root anon %llu, sum of the leaves %llu\n",
+			       root_anon, leaf_anon);
+		bad++;
+	}
+	return bad ? -1 : 0;
+}
+
+/*
+ * memory.max is never set, so the counter must read PAGE_COUNTER_MAX.  That is
+ * LONG_MAX on a 32-bit kernel and LONG_MAX / PAGE_SIZE elsewhere, and the width
+ * of a userspace long does not tell us which, so accept either.
+ */
+static int check_unlimited(void)
+{
+	unsigned long long max64 = (unsigned long long)INT64_MAX / page_size;
+	unsigned long long max32 = INT32_MAX;
+
+	if (cg_read_strcmp(nodes[0].path, "memory.max", "max\n"))
+		return 0;
+
+	if (nodes[0].bpf.max_pages != max64 && nodes[0].bpf.max_pages != max32) {
+		ksft_print_msg("memory.max reads max, kfunc reports %llu pages\n",
+			       nodes[0].bpf.max_pages);
+		return -1;
+	}
+	return 0;
+}
+
+/* ---- one case ----------------------------------------------------------- */
+
+struct testcase {
+	const char *name;
+	int fanout;
+	int depth;
+	int cpus_per_leaf;	/* K, or 0 for every CPU */
+	size_t resident_bytes;	/* anon per leaf, raised if too small */
+};
+
+static int run_case(const struct testcase *tc)
+{
+	int root_fd = -1, ret = KSFT_FAIL, k;
+	size_t charged;
+
+	if (build_tree(tc->fanout, tc->depth, &root_fd)) {
+		ksft_print_msg("cannot build the tree\n");
+		goto out;
+	}
+
+	k = tc->cpus_per_leaf;
+	if (k <= 0 || k > n_cpu)
+		k = n_cpu;
+	charged = leaf_charge(tc->resident_bytes, k);
+
+	ksft_print_msg("%s: %d cgroups, %d leaves, %d/%d cpus, %zuKB per leaf\n",
+		       tc->name, n_nodes, n_leaves, k, n_cpu, charged >> 10);
+
+	if (start_chargers(k, charged))
+		goto out;
+
+	/* kfuncs first: their flush is the one that has work to do */
+	if (read_bpf(root_fd) || read_files())
+		goto out;
+
+	if (!check_tree(charged) && !check_unlimited())
+		ret = KSFT_PASS;
+out:
+	if (root_fd >= 0)
+		close(root_fd);
+	destroy_tree();
+	return ret;
+}
+
+static const struct testcase cases[] = {
+	/* name, fanout, depth, K, anon per leaf */
+	{ "single_cpu_small_tree", 4, 2, 1, 2 << 20 },
+	{ "cross_cpu_small_tree", 4, 2, 0, 2 << 20 },
+	{ "single_cpu_large_tree", 10, 3, 1, 256 << 10 },
+	{ "cross_cpu_large_tree", 10, 3, 0, 256 << 10 },
+};
+
+static bool memcg_kfuncs_available(void)
+{
+	struct btf *btf;
+	bool ok;
+
+	btf = btf__load_vmlinux_btf();
+	if (!btf)
+		return false;
+	ok = btf__find_by_name_kind(btf, "bpf_get_mem_cgroup", BTF_KIND_FUNC) > 0;
+	btf__free(btf);
+	return ok;
+}
+
+int main(int argc, char **argv)
+{
+	int i;
+
+	ksft_print_header();
+
+	/* Feature gate first: a read-only BTF probe, no privilege needed. */
+	if (!memcg_kfuncs_available())
+		ksft_exit_skip("memcg BPF kfuncs are not available\n");
+
+	if (cg_find_unified_root(root, sizeof(root), NULL))
+		ksft_exit_skip("cgroup v2 isn't mounted\n");
+
+	if (cg_read_strstr(root, "cgroup.controllers", "memory"))
+		ksft_exit_skip("memory controller isn't available\n");
+
+	if (cg_read_strstr(root, "cgroup.subtree_control", "memory"))
+		if (cg_write(root, "cgroup.subtree_control", "+memory"))
+			ksft_exit_skip("Failed to set memory controller\n");
+
+	CPU_ZERO(&allowed_cpus);
+	if (sched_getaffinity(0, sizeof(allowed_cpus), &allowed_cpus))
+		ksft_exit_skip("cannot read CPU affinity\n");
+	n_cpu = CPU_COUNT(&allowed_cpus);
+	if (n_cpu <= 0)
+		ksft_exit_skip("no CPU to run on\n");
+
+	page_size = sysconf(_SC_PAGESIZE);
+	if (page_size <= 0)
+		page_size = BUF_SIZE;
+
+	subtree_root = cg_name(root, SUBTREE_NAME);
+	if (!subtree_root)
+		ksft_exit_skip("cannot build subtree root path\n");
+
+	/* Set the plan only once nothing can skip the whole run any more. */
+	ksft_set_plan(ARRAY_SIZE(cases));
+
+	for (i = 0; i < ARRAY_SIZE(cases); i++) {
+		switch (run_case(&cases[i])) {
+		case KSFT_PASS:
+			ksft_test_result_pass("%s\n", cases[i].name);
+			break;
+		default:
+			ksft_test_result_fail("%s\n", cases[i].name);
+			break;
+		}
+	}
+
+	free(subtree_root);
+	ksft_finished();
+}
diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index 0ebf796f3cff..15ba46879504 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -26,7 +26,6 @@
 
 static bool has_localevents;
 static bool has_recursiveprot;
-static int page_size;
 
 int get_temp_fd(void)
 {
@@ -56,28 +55,12 @@ int alloc_pagecache(int fd, size_t size)
 	return -1;
 }
 
-static char *alloc_and_populate_anon(size_t size)
-{
-	char *buf, *ptr;
-
-	buf = malloc(size);
-	if (buf == NULL) {
-		fprintf(stderr, "malloc() failed\n");
-		return NULL;
-	}
-
-	for (ptr = buf; ptr < buf + size; ptr += page_size)
-		*ptr = 0;
-
-	return buf;
-}
-
 int alloc_anon(const char *cgroup, void *arg)
 {
 	size_t size = (unsigned long)arg;
 	char *buf;
 
-	buf = alloc_and_populate_anon(size);
+	buf = cg_alloc_anon(size);
 	if (!buf)
 		return -1;
 
@@ -195,7 +178,7 @@ static int alloc_anon_50M_check(const char *cgroup, void *arg)
 	long anon, current;
 	int ret = -1;
 
-	buf = alloc_and_populate_anon(size);
+	buf = cg_alloc_anon(size);
 	if (!buf)
 		return -1;
 
@@ -420,7 +403,7 @@ static int alloc_anon_noexit(const char *cgroup, void *arg)
 	size_t size = (unsigned long)arg;
 	char *buf;
 
-	buf = alloc_and_populate_anon(size);
+	buf = cg_alloc_anon(size);
 	if (!buf)
 		return -1;
 
@@ -1001,7 +984,7 @@ static int alloc_anon_50M_check_swap(const char *cgroup, void *arg)
 	long mem_current, swap_current;
 	int ret = -1;
 
-	buf = alloc_and_populate_anon(size);
+	buf = cg_alloc_anon(size);
 	if (!buf)
 		return -1;
 
@@ -1793,10 +1776,6 @@ int main(int argc, char **argv)
 	char root[PATH_MAX];
 	int i, proc_status;
 
-	page_size = sysconf(_SC_PAGE_SIZE);
-	if (page_size <= 0)
-		page_size = BUF_SIZE;
-
 	ksft_print_header();
 	ksft_set_plan(ARRAY_SIZE(tests));
 	if (cg_find_unified_root(root, sizeof(root), NULL))
-- 
2.53.0-Meta

[PATCH v3 3/4] selftests/hid: build the BPF program via the shared lib.bpf.mk

[Ziyang Men]

hid carries its own ~150 lines of libbpf + bpftool + vmlinux.h +
BPF-object + skeleton build machinery, copied from selftests/bpf.
Replace it with an include of the shared
tools/testing/selftests/lib.bpf.mk, so that the previous ~150 lines of
BPF build configuration can now be achieved in only ~10 lines.

The generated hid.skel.h public API is same before and after, hid_bpf
and hidraw are built and linked the same way.

Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Suggested-by: Mykola Lysenko <mykolal@meta.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
---
 tools/testing/selftests/hid/.gitignore |   1 +
 tools/testing/selftests/hid/Makefile   | 180 +++----------------------
 2 files changed, 20 insertions(+), 161 deletions(-)

diff --git a/tools/testing/selftests/hid/.gitignore b/tools/testing/selftests/hid/.gitignore
index 933f483815b2..69943301dc1d 100644
--- a/tools/testing/selftests/hid/.gitignore
+++ b/tools/testing/selftests/hid/.gitignore
@@ -1,5 +1,6 @@
 bpftool
 *.skel.h
+*.bpf.d
 /host-tools
 /tools
 hid_bpf
diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile
index 2f423de83147..1d7c64ec992c 100644
--- a/tools/testing/selftests/hid/Makefile
+++ b/tools/testing/selftests/hid/Makefile
@@ -58,172 +58,31 @@ override define CLEAN
 	$(Q)$(RM) -r $(EXTRA_CLEAN)
 endef
 
+# progs/hid.c is built into a skeleton by ../lib.bpf.mk.  hid keeps the legacy
+# progs/<name>.c layout, hence BPF_PROG_EXT.  BPF_EXTRA_HDRS is deferred: it
+# uses $(BPFDIR), which lib.bpf.mk defines.
+BPF_SRCS       := $(wildcard progs/*.c)
+BPF_PROG_EXT   := .c
+BPF_EXTRA_HDRS  = $(wildcard progs/*.h) $(wildcard $(BPFDIR)/hid_bpf_*.h) \
+		  $(wildcard $(BPFDIR)/*.bpf.h)
+# hid.c predates the shared -Wall and has a few unused locals.
+BPF_EXTRA_CFLAGS := -Wno-unused-variable
+
 include ../lib.mk
+include ../lib.bpf.mk
 
-TOOLSDIR := $(top_srcdir)/tools
-LIBDIR := $(TOOLSDIR)/lib
-BPFDIR := $(LIBDIR)/bpf
-TOOLSINCDIR := $(TOOLSDIR)/include
-BPFTOOLDIR := $(TOOLSDIR)/bpf/bpftool
-SCRATCH_DIR := $(OUTPUT)/tools
-BUILD_DIR := $(SCRATCH_DIR)/build
-INCLUDE_DIR := $(SCRATCH_DIR)/include
-BPFOBJ := $(BUILD_DIR)/libbpf/libbpf.a
-ifneq ($(CROSS_COMPILE),)
-HOST_BUILD_DIR		:= $(BUILD_DIR)/host
-HOST_SCRATCH_DIR	:= $(OUTPUT)/host-tools
-HOST_INCLUDE_DIR	:= $(HOST_SCRATCH_DIR)/include
-else
-HOST_BUILD_DIR		:= $(BUILD_DIR)
-HOST_SCRATCH_DIR	:= $(SCRATCH_DIR)
-HOST_INCLUDE_DIR	:= $(INCLUDE_DIR)
-endif
-HOST_BPFOBJ := $(HOST_BUILD_DIR)/libbpf/libbpf.a
-RESOLVE_BTFIDS := $(HOST_BUILD_DIR)/resolve_btfids/resolve_btfids
-
-VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)				\
-		     $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux)	\
-		     ../../../../vmlinux				\
-		     /sys/kernel/btf/vmlinux				\
-		     /boot/vmlinux-$(shell uname -r)
-VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
-ifeq ($(VMLINUX_BTF),)
-$(error Cannot find a vmlinux for VMLINUX_BTF at any of "$(VMLINUX_BTF_PATHS)")
-endif
+# Ship what this directory shipped before the conversion.
+TEST_GEN_FILES += $(BPF_OBJS)
+TEST_GEN_PROGS_EXTENDED += $(DEFAULT_BPFTOOL)
 
-# Define simple and short `make test_progs`, `make test_sysctl`, etc targets
-# to build individual tests.
+# Define simple and short `make hid_bpf`, `make hidraw` targets.
 # NOTE: Semicolon at the end is critical to override lib.mk's default static
 # rule for binaries.
 $(notdir $(TEST_GEN_PROGS)): %: $(OUTPUT)/% ;
 
-# sort removes libbpf duplicates when not cross-building
-MAKE_DIRS := $(sort $(BUILD_DIR)/libbpf $(HOST_BUILD_DIR)/libbpf		\
-	       $(HOST_BUILD_DIR)/bpftool $(HOST_BUILD_DIR)/resolve_btfids	\
-	       $(INCLUDE_DIR))
-$(MAKE_DIRS):
-	$(call msg,MKDIR,,$@)
-	$(Q)mkdir -p $@
-
-DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
-
-TEST_GEN_PROGS_EXTENDED += $(DEFAULT_BPFTOOL)
-
-$(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): $(BPFOBJ)
-
-BPFTOOL ?= $(DEFAULT_BPFTOOL)
-$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile)    \
-		    $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
-	$(Q)$(MAKE) $(submake_extras)  -C $(BPFTOOLDIR)			       \
-		    ARCH= CROSS_COMPILE= CC=$(HOSTCC) LD=$(HOSTLD) 	       \
-		    EXTRA_CFLAGS='-g -O0'				       \
-		    OUTPUT=$(HOST_BUILD_DIR)/bpftool/			       \
-		    LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/		       \
-		    LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/			       \
-		    prefix= DESTDIR=$(HOST_SCRATCH_DIR)/ install-bin
-
-$(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile)		       \
-	   | $(BUILD_DIR)/libbpf
-	$(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
-		    EXTRA_CFLAGS='-g -O0'				       \
-		    DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
-
-ifneq ($(BPFOBJ),$(HOST_BPFOBJ))
-$(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile)		       \
-		| $(HOST_BUILD_DIR)/libbpf
-	$(Q)$(MAKE) $(submake_extras) -C $(BPFDIR)                             \
-		    EXTRA_CFLAGS='-g -O0' ARCH= CROSS_COMPILE=		       \
-		    OUTPUT=$(HOST_BUILD_DIR)/libbpf/ CC=$(HOSTCC) LD=$(HOSTLD) \
-		    DESTDIR=$(HOST_SCRATCH_DIR)/ prefix= all install_headers
-endif
-
-$(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL) | $(INCLUDE_DIR)
-ifeq ($(VMLINUX_H),)
-	$(call msg,GEN,,$@)
-	$(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@
-else
-	$(call msg,CP,,$@)
-	$(Q)cp "$(VMLINUX_H)" $@
-endif
-
-$(RESOLVE_BTFIDS): $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/resolve_btfids	\
-		       $(TOOLSDIR)/bpf/resolve_btfids/main.c	\
-		       $(TOOLSDIR)/lib/rbtree.c			\
-		       $(TOOLSDIR)/lib/zalloc.c			\
-		       $(TOOLSDIR)/lib/string.c			\
-		       $(TOOLSDIR)/lib/ctype.c			\
-		       $(TOOLSDIR)/lib/str_error_r.c
-	$(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/resolve_btfids	\
-		CC=$(HOSTCC) LD=$(HOSTLD) AR=$(HOSTAR) \
-		LIBBPF_INCLUDE=$(HOST_INCLUDE_DIR) \
-		OUTPUT=$(HOST_BUILD_DIR)/resolve_btfids/ BPFOBJ=$(HOST_BPFOBJ)
-
-# Get Clang's default includes on this system, as opposed to those seen by
-# '--target=bpf'. This fixes "missing" files on some architectures/distros,
-# such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc.
-#
-# Use '-idirafter': Don't interfere with include mechanics except where the
-# build would have failed anyways.
-define get_sys_includes
-$(shell $(1) -v -E - </dev/null 2>&1 \
-	| sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }') \
-$(shell $(1) -dM -E - </dev/null | grep '__riscv_xlen ' | awk '{printf("-D__riscv_xlen=%d -D__BITS_PER_LONG=%d", $$3, $$3)}')
-endef
-
-# Determine target endianness.
-IS_LITTLE_ENDIAN = $(shell $(CC) -dM -E - </dev/null | \
-			grep 'define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__')
-MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
-
-CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
-BPF_CFLAGS = -g -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) 		\
-	     -Wno-microsoft-anon-tag                                    \
-	     -fms-extensions                                            \
-	     -I$(INCLUDE_DIR)
-
-CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \
-	       -Wno-compare-distinct-pointer-types
-
-# Build BPF object using Clang
-# $1 - input .c file
-# $2 - output .o file
-# $3 - CFLAGS
-define CLANG_BPF_BUILD_RULE
-	$(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
-	$(Q)$(CLANG) $3 -O2 --target=bpf -c $1 -mcpu=v3 -o $2
-endef
-# Similar to CLANG_BPF_BUILD_RULE, but with disabled alu32
-define CLANG_NOALU32_BPF_BUILD_RULE
-	$(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
-	$(Q)$(CLANG) $3 -O2 --target=bpf -c $1 -mcpu=v2 -o $2
-endef
-# Build BPF object using GCC
-define GCC_BPF_BUILD_RULE
-	$(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
-	$(Q)$(BPF_GCC) $3 -O2 -c $1 -o $2
-endef
-
-BPF_PROGS_DIR := progs
-BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
-BPF_SRCS := $(notdir $(wildcard $(BPF_PROGS_DIR)/*.c))
-BPF_OBJS := $(patsubst %.c,$(OUTPUT)/%.bpf.o, $(BPF_SRCS))
-BPF_SKELS := $(patsubst %.c,$(OUTPUT)/%.skel.h, $(BPF_SRCS))
-TEST_GEN_FILES += $(BPF_OBJS)
-
-$(BPF_PROGS_DIR)-bpfobjs := y
-$(BPF_OBJS): $(OUTPUT)/%.bpf.o:				\
-	     $(BPF_PROGS_DIR)/%.c			\
-	     $(wildcard $(BPF_PROGS_DIR)/*.h)		\
-	     $(INCLUDE_DIR)/vmlinux.h				\
-	     $(wildcard $(BPFDIR)/hid_bpf_*.h)			\
-	     $(wildcard $(BPFDIR)/*.bpf.h)			\
-	     | $(OUTPUT) $(BPFOBJ)
-	$(call $(BPF_BUILD_RULE),$<,$@, $(BPF_CFLAGS))
-
-$(BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(OUTPUT)
-	$(call msg,GEN-SKEL,$(BINARY),$@)
-	$(Q)$(BPFTOOL) gen object $(<:.o=.linked1.o) $<
-	$(Q)$(BPFTOOL) gen skeleton $(<:.o=.linked1.o) name $(notdir $(<:.bpf.o=)) > $@
+# Each test binary links against the in-tree static libbpf that lib.bpf.mk
+# built (lib.mk has already prefixed TEST_GEN_PROGS with $(OUTPUT)/).
+$(TEST_GEN_PROGS): $(BPFOBJ)
 
 $(OUTPUT)/%.o: %.c $(BPF_SKELS) hid_common.h
 	$(call msg,CC,,$@)
@@ -233,5 +92,4 @@ $(OUTPUT)/%: $(OUTPUT)/%.o
 	$(call msg,BINARY,,$@)
 	$(Q)$(LINK.c) $^ $(LDLIBS) -o $@
 
-EXTRA_CLEAN := $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) feature bpftool	\
-	$(addprefix $(OUTPUT)/,*.o *.skel.h no_alu32)
+EXTRA_CLEAN += feature bpftool $(addprefix $(OUTPUT)/,*.o no_alu32)
-- 
2.53.0-Meta

[PATCH v3 4/4] selftests/sched_ext: build BPF schedulers via the shared lib.bpf.mk

[Ziyang Men]

sched_ext carried its own ~130 lines of libbpf + bpftool + vmlinux.h +
BPF-object + skeleton build machinery.  Replace it with an include of the
shared tools/testing/selftests/lib.bpf.mk, making sched_ext the third
in-tree consumer of that fragment, after selftests/cgroup and
selftests/hid.

All 28 skeletons and 28 subskeletons keep a same public API before and
after, the runner builds and links the same way.

Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Suggested-by: Mykola Lysenko <mykolal@meta.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Ziyang Men <ziyang.meme@gmail.com>
---
 tools/testing/selftests/sched_ext/Makefile | 146 +++++----------------
 1 file changed, 30 insertions(+), 116 deletions(-)

diff --git a/tools/testing/selftests/sched_ext/Makefile b/tools/testing/selftests/sched_ext/Makefile
index 5d2dffca0e91..620e8cb31e6f 100644
--- a/tools/testing/selftests/sched_ext/Makefile
+++ b/tools/testing/selftests/sched_ext/Makefile
@@ -14,48 +14,32 @@ CURDIR := $(abspath .)
 REPOROOT := $(abspath ../../../..)
 TOOLSDIR := $(REPOROOT)/tools
 LIBDIR := $(TOOLSDIR)/lib
-BPFDIR := $(LIBDIR)/bpf
 TOOLSINCDIR := $(TOOLSDIR)/include
-BPFTOOLDIR := $(TOOLSDIR)/bpf/bpftool
 APIDIR := $(TOOLSINCDIR)/uapi
 GENDIR := $(REPOROOT)/include/generated
 GENHDR := $(GENDIR)/autoconf.h
-SCXTOOLSDIR := $(TOOLSDIR)/sched_ext
 SCXTOOLSINCDIR := $(TOOLSDIR)/sched_ext/include
 
-OUTPUT_DIR := $(OUTPUT)/build
-OBJ_DIR := $(OUTPUT_DIR)/obj
-INCLUDE_DIR := $(OUTPUT_DIR)/include
-BPFOBJ_DIR := $(OBJ_DIR)/libbpf
-SCXOBJ_DIR := $(OBJ_DIR)/sched_ext
-BPFOBJ := $(BPFOBJ_DIR)/libbpf.a
-LIBBPF_OUTPUT := $(OBJ_DIR)/libbpf/libbpf.a
-
-DEFAULT_BPFTOOL := $(OUTPUT_DIR)/host/sbin/bpftool
-HOST_OBJ_DIR := $(OBJ_DIR)/host/bpftool
-HOST_LIBBPF_OUTPUT := $(OBJ_DIR)/host/libbpf/
-HOST_LIBBPF_DESTDIR := $(OUTPUT_DIR)/host/
-HOST_DESTDIR := $(OUTPUT_DIR)/host/
-
-VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)					\
-		     $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux)		\
-		     ../../../../vmlinux					\
-		     /sys/kernel/btf/vmlinux					\
-		     /boot/vmlinux-$(shell uname -r)
-VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
-ifeq ($(VMLINUX_BTF),)
-$(error Cannot find a vmlinux for VMLINUX_BTF at any of "$(VMLINUX_BTF_PATHS)")
-endif
+# The schedulers are built by ../lib.bpf.mk: modern *.bpf.c layout, a
+# .bpf.skel.h suffix, and subskeletons.
+BPF_SRCS        := $(wildcard *.bpf.c)
+BPF_SKEL_EXT    := .bpf.skel.h
+BPF_GEN_SUBSKEL := 1
+# Keep the generated files under build/ as before.  SCXOBJ_DIR (userspace
+# objects) reuses the BPF object dir.
+BPF_OBJ_DIR  := $(OUTPUT)/build/obj/sched_ext
+BPF_SKEL_DIR := $(OUTPUT)/build/include
+SCXOBJ_DIR   := $(BPF_OBJ_DIR)
 
-BPFTOOL ?= $(DEFAULT_BPFTOOL)
+include ../lib.bpf.mk
 
 ifneq ($(wildcard $(GENHDR)),)
   GENFLAGS := -DHAVE_GENHDR
 endif
 
 CFLAGS += -g -O2 -rdynamic -pthread -Wall -Werror $(GENFLAGS)			\
-	  -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR)				\
-	  -I$(TOOLSINCDIR) -I$(APIDIR) -I$(CURDIR)/include -I$(SCXTOOLSINCDIR)
+	  -I$(GENDIR) -I$(LIBDIR) -I$(TOOLSINCDIR) -I$(APIDIR)			\
+	  -I$(CURDIR)/include -I$(SCXTOOLSINCDIR)
 
 # Silence some warnings when compiled with clang
 ifneq ($(LLVM),)
@@ -64,102 +48,30 @@ endif
 
 LDFLAGS = -lelf -lz -lpthread -lzstd
 
-IS_LITTLE_ENDIAN = $(shell $(CC) -dM -E - </dev/null |				\
-			grep 'define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__')
-
-# Get Clang's default includes on this system, as opposed to those seen by
-# '-target bpf'. This fixes "missing" files on some architectures/distros,
-# such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc.
-#
-# Use '-idirafter': Don't interfere with include mechanics except where the
-# build would have failed anyways.
-define get_sys_includes
-$(shell $(1) $(2) -v -E - </dev/null 2>&1 \
-	| sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }') \
-$(shell $(1) $(2) -dM -E - </dev/null | grep '__riscv_xlen ' | awk '{printf("-D__riscv_xlen=%d -D__BITS_PER_LONG=%d", $$3, $$3)}')
-endef
-
-ifneq ($(CROSS_COMPILE),)
-CLANG_TARGET_ARCH = --target=$(notdir $(CROSS_COMPILE:%-=%))
-endif
-
-CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
-
-BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH)					\
-	     $(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)		\
+# The schedulers need scheduler-specific includes, so override BPF_CFLAGS
+# wholesale and keep the flag set this directory used, rather than pin -Werror
+# across clang versions.  The recipe adds -O2/--target/-mcpu.
+BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN)				\
 	     -I$(CURDIR)/include -I$(CURDIR)/include/bpf-compat			\
 	     -I$(INCLUDE_DIR) -I$(APIDIR) -I$(SCXTOOLSINCDIR)			\
 	     -I$(REPOROOT)/include						\
-	     $(CLANG_SYS_INCLUDES) 						\
+	     $(CLANG_SYS_INCLUDES)						\
 	     -Wall -Wno-compare-distinct-pointer-types				\
 	     -Wno-incompatible-function-pointer-types				\
 	     -Wno-microsoft-anon-tag						\
 	     -fms-extensions							\
-	     -O2 -mcpu=v3
-
-# sort removes libbpf duplicates when not cross-building
-MAKE_DIRS := $(sort $(OBJ_DIR)/libbpf $(OBJ_DIR)/libbpf				\
-	       $(OBJ_DIR)/bpftool $(OBJ_DIR)/resolve_btfids			\
-	       $(HOST_OBJ_DIR) $(INCLUDE_DIR) $(SCXOBJ_DIR))
-
-$(MAKE_DIRS):
-	$(call msg,MKDIR,,$@)
-	$(Q)mkdir -p $@
-
-$(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile)			\
-	   $(APIDIR)/linux/bpf.h						\
-	   | $(OBJ_DIR)/libbpf
-	$(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(OBJ_DIR)/libbpf/	\
-		    ARCH=$(ARCH) CC="$(CC)" CROSS_COMPILE=$(CROSS_COMPILE)	\
-		    EXTRA_CFLAGS='-g -O0 -fPIC'					\
-		    DESTDIR=$(OUTPUT_DIR) prefix= all install_headers
-
-$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile)	\
-		    $(LIBBPF_OUTPUT) | $(HOST_OBJ_DIR)
-	$(Q)$(MAKE) $(submake_extras)  -C $(BPFTOOLDIR)				\
-		    ARCH= CROSS_COMPILE= CC=$(HOSTCC) LD=$(HOSTLD)		\
-		    EXTRA_CFLAGS='-g -O0'					\
-		    OUTPUT=$(HOST_OBJ_DIR)/					\
-		    LIBBPF_OUTPUT=$(HOST_LIBBPF_OUTPUT)				\
-		    LIBBPF_DESTDIR=$(HOST_LIBBPF_DESTDIR)			\
-		    prefix= DESTDIR=$(HOST_DESTDIR) install-bin
-
-$(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL) | $(INCLUDE_DIR)
-ifeq ($(VMLINUX_H),)
-	$(call msg,GEN,,$@)
-	$(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@
-else
-	$(call msg,CP,,$@)
-	$(Q)cp "$(VMLINUX_H)" $@
-endif
+	     $(BPF_EXTRA_CFLAGS)
 
-$(SCXOBJ_DIR)/%.bpf.o: %.bpf.c $(INCLUDE_DIR)/vmlinux.h	| $(BPFOBJ) $(SCXOBJ_DIR)
-	$(call msg,CLNG-BPF,,$(notdir $@))
-	$(Q)$(CLANG) $(BPF_CFLAGS) -target bpf -c $< -o $@
-
-$(INCLUDE_DIR)/%.bpf.skel.h: $(SCXOBJ_DIR)/%.bpf.o $(INCLUDE_DIR)/vmlinux.h $(BPFTOOL) | $(INCLUDE_DIR)
-	$(eval sched=$(notdir $@))
-	$(call msg,GEN-SKEL,,$(sched))
-	$(Q)$(BPFTOOL) gen object $(<:.o=.linked1.o) $<
-	$(Q)$(BPFTOOL) gen object $(<:.o=.linked2.o) $(<:.o=.linked1.o)
-	$(Q)$(BPFTOOL) gen object $(<:.o=.linked3.o) $(<:.o=.linked2.o)
-	$(Q)diff $(<:.o=.linked2.o) $(<:.o=.linked3.o)
-	$(Q)$(BPFTOOL) gen skeleton $(<:.o=.linked3.o) name $(subst .bpf.skel.h,,$(sched)) > $@
-	$(Q)$(BPFTOOL) gen subskeleton $(<:.o=.linked3.o) name $(subst .bpf.skel.h,,$(sched)) > $(@:.skel.h=.subskel.h)
+EXTRA_CLEAN += $(OUTPUT)/build
 
 ################
 # C schedulers #
 ################
 
-override define CLEAN
-	rm -rf $(OUTPUT_DIR)
-	rm -f $(TEST_GEN_PROGS)
-endef
-
-# Every testcase takes all of the BPF progs are dependencies by default. This
+# Every testcase takes all of the BPF progs as dependencies by default. This
 # allows testcases to load any BPF scheduler, which is useful for testcases
 # that don't need their own prog to run their test.
-all_test_bpfprogs := $(foreach prog,$(wildcard *.bpf.c),$(INCLUDE_DIR)/$(patsubst %.c,%.skel.h,$(prog)))
+all_test_bpfprogs := $(BPF_SKELS)
 
 auto-test-targets :=			\
 	create_dsq			\
@@ -195,7 +107,8 @@ auto-test-targets :=			\
 testcase-targets := $(addsuffix .o,$(addprefix $(SCXOBJ_DIR)/,$(auto-test-targets)))
 
 $(SCXOBJ_DIR)/runner.o: runner.c | $(SCXOBJ_DIR) $(BPFOBJ)
-	$(CC) $(CFLAGS) -c $< -o $@
+	$(call msg,CC,,$@)
+	$(Q)$(CC) $(CFLAGS) -c $< -o $@
 
 # Create all of the test targets object files, whose testcase objects will be
 # registered into the runner in ELF constructors.
@@ -204,15 +117,16 @@ $(SCXOBJ_DIR)/runner.o: runner.c | $(SCXOBJ_DIR) $(BPFOBJ)
 # compiling BPF object files only if one is present, as the wildcard Make
 # function doesn't support using implicit rules otherwise.
 $(testcase-targets): $(SCXOBJ_DIR)/%.o: %.c $(SCXOBJ_DIR)/runner.o $(all_test_bpfprogs) | $(SCXOBJ_DIR)
-	$(eval test=$(patsubst %.o,%.c,$(notdir $@)))
-	$(CC) $(CFLAGS) -c $< -o $@
+	$(call msg,CC,,$@)
+	$(Q)$(CC) $(CFLAGS) -c $< -o $@
 
 $(SCXOBJ_DIR)/util.o: util.c | $(SCXOBJ_DIR)
-	$(CC) $(CFLAGS) -c $< -o $@
+	$(call msg,CC,,$@)
+	$(Q)$(CC) $(CFLAGS) -c $< -o $@
 
 $(OUTPUT)/runner: $(SCXOBJ_DIR)/runner.o $(SCXOBJ_DIR)/util.o $(BPFOBJ) $(testcase-targets)
-	@echo "$(testcase-targets)"
-	$(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS)
+	$(call msg,BINARY,,$@)
+	$(Q)$(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS)
 
 .DEFAULT_GOAL := all
 
-- 
2.53.0-Meta

Re: [PATCH v3 1/4] selftests: add shared lib.bpf.mk to build BPF progs and skeletons

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] BPF objects will not automatically rebuild when libbpf headers are modified, breaking incremental compilation.
- [Low] Missing Clang system include definitions for LoongArch and MIPS architectures.
--

--- Patch [1]: [PATCH v3 1/4] selftests: add shared lib.bpf.mk to build BPF progs and skeletons ---
commit 73f22930c4ae7eac2caa52ed8b921af4c943e0a7
Author: Ziyang Men <ziyang.meme@gmail.com>

selftests: add shared lib.bpf.mk to build BPF progs and skeletons

This commit adds a shared tools/testing/selftests/lib.bpf.mk file to provide
a common build toolchain for BPF programs and skeletons across different
selftest directories. It aims to reduce duplication of the libbpf, bpftool,
vmlinux.h, and skeleton generation Makefiles currently present in bpf,
sched_ext, and hid selftests.

> diff --git a/tools/testing/selftests/lib.bpf.mk b/tools/testing/selftests/lib.bpf.mk
> new file mode 100644
> index 0000000000000..433a26a36960b
> --- /dev/null
> +++ b/tools/testing/selftests/lib.bpf.mk

[ ... ]

> +# ---- compiler flags -------------------------------------------------------
> +# Find the normal system headers that Clang omits with --target=bpf. Put them
> +# last with -idirafter so they cannot replace project headers.
> +define get_sys_includes
> +$(shell $(1) $(2) -v -E - </dev/null 2>&1 \
> +	| sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }') \
> +$(shell $(1) $(2) -dM -E - </dev/null | grep '__riscv_xlen ' | awk '{printf("-D__riscv_xlen=%d -D__BITS_PER_LONG=%d", $$3, $$3)}')
> +endef

[Severity: Low]
Will BPF programs fail to compile on LoongArch and MIPS architectures?

The original selftests/bpf/Makefile extracts necessary host definitions
like __loongarch_grlen and MIPS macros.

This new macro appears to drop those extractions, which might cause Clang's
--target=bpf to fail to parse system headers on those architectures.

[ ... ]

> +# Make one rule for each source. A global vpath would also match the test's
> +# normal C rules when BPF_PROG_EXT is .c.
> +define bpf_obj_rule
> +$(BPF_OBJ_DIR)/$(patsubst %$(BPF_PROG_EXT),%,$(notdir $(1))).bpf.o: $(1)	\
> +		$(BPF_EXTRA_HDRS) $(INCLUDE_DIR)/vmlinux.h			\
> +		| $(BPF_OBJ_DIR) $(BPFOBJ)

[Severity: Medium]
Will this break incremental compilation when libbpf headers are modified?

Since $(BPFOBJ) is specified only as an order-only prerequisite and there is
no explicit Make rule for the installed headers, Make evaluates their
timestamps before $(BPFOBJ) updates them.

Does this cause BPF objects to be incorrectly skipped during the first make
invocation after modifying a libbpf header?

> +	$$(call BPF_BUILD_RULE,$$<,$$@)
> +endef

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814075054.507089-1-ziyang.meme@gmail.com?part=1

Re: [PATCH v3 2/4] selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Multi-line comment does not follow the correct BPF/kernel style.

Pre-existing issues:
- [High] The kfuncs bpf_mem_cgroup_memory_events and bpf_mem_cgroup_vm_events lack negative bounds checks for their enum parameters, leading to out-of-bounds array reads.
--

--- Patch [2]: [PATCH v3 2/4] selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush ---
commit aa1f4cd8dd58a883c661755d56a15bc86e70795f
Author: Ziyang Men <ziyang.meme@gmail.com>

selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush

This patch adds a new selftest, test_memcg_stat_cross_cpu, to verify
that cgroup flushes happen correctly by ensuring BPF read values match
cgroupfs file readings. It also strengthens test coverage by scattering
charges across multiple CPUs.

> diff --git a/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
> new file mode 100644
> index 0000000000000..3c2acd49c16ba
> --- /dev/null
> +++ b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +#define BPF_NO_KFUNC_PROTOTYPES
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_core_read.h>
> +#include "memcg_stat_cross_cpu.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +/*
> + * Declared here rather than taken from vmlinux.h, which only has them if the
> + * build host's own kernel does.  A kernel without them is caught at run time.
> + */
> +struct mem_cgroup *bpf_get_mem_cgroup(struct cgroup_subsys_state *css) __ksym;
> +void bpf_put_mem_cgroup(struct mem_cgroup *memcg) __ksym;
> +void bpf_mem_cgroup_flush_stats(struct mem_cgroup *memcg) __ksym;
> +unsigned long bpf_mem_cgroup_page_state(struct mem_cgroup *memcg, int idx) __ksym;
> +unsigned long bpf_mem_cgroup_vm_events(struct mem_cgroup *memcg,
> +				       enum vm_event_item event) __ksym;

[Severity: High]
This is a pre-existing issue, but does this kfunc allow an out-of-bounds array
read?

The kfuncs bpf_mem_cgroup_memory_events() and bpf_mem_cgroup_vm_events() lack
negative bounds checks for their enum parameters. A BPF program can pass a
negative integer as the enum parameter to bpf_mem_cgroup_vm_events().

This passes the negative enum to memcg_vm_event_item_valid() in mm/memcontrol.c:

    if (idx >= NR_VM_EVENT_ITEMS)

This validation function omits a negative bounds check (e.g., missing an
unsigned cast). Because the BPF verifier evaluates enum parameters as signed
integers without an explicit lower bound check, negative values bypass the
check. This reaches memcg_events_index() and allows arbitrary memory offsets
to be read relative to the memcg arrays.

[ ... ]

> diff --git a/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c b/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c
> new file mode 100644
> index 0000000000000..9946c41c7a908
> --- /dev/null
> +++ b/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c

[ ... ]

> +static int add_node(const char *path, bool is_leaf)
> +{
> +	struct cg_node *n = &nodes[n_nodes];
> +
> +	if (cg_create(path))
> +		return -1;
> +
> +	strncpy(n->path, path, sizeof(n->path) - 1);
> +	n->id = cg_get_id(path);
> +	n->is_leaf = is_leaf;
> +	if (is_leaf)
> +		n_leaves++;
> +	n_nodes++;
> +	return 0;
> +}
> +
> +/* Create # @levels more levels below @path.
> + * Only the last level is charged.
> + */

[Severity: Low]
Does this multi-line comment style follow the BPF and kernel guidelines?
The preferred style requires the opening /* to be on its own line:

/*
 * Create # @levels more levels below @path.
 * Only the last level is charged.
 */

> +static int build_children(const char *path, int fanout, int levels)
> +{
> +	char child[PATH_MAX];
> +	int i;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814075054.507089-1-ziyang.meme@gmail.com?part=2

Re: [PATCH v3 3/4] selftests/hid: build the BPF program via the shared lib.bpf.mk

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3631 bytes --]

> The generated hid.skel.h public API is same before and after, hid_bpf
> and hidraw are built and linked the same way.

This isn't a bug, but could the changelog mention that the BPF compile
flags change with the shared fragment (added -Wall/-std=gnu11/
-fno-strict-aliasing, added include paths, auto-detected -mcpu instead
of a hardcoded v3) rather than describing the build as unchanged?

The flags used to compile progs/hid.c do change: the shared BPF_CFLAGS
add -Wall, -std=gnu11, -fno-strict-aliasing, -I$(APIDIR)
-I$(TOOLSINCDIR) and the clang system-include set (which the old
Makefile computed into CLANG_SYS_INCLUDES but never passed), and -mcpu
becomes auto-detected (v3/v2) instead of hardcoded -mcpu=v3. The
-Wno-unused-variable line the patch has to add is direct evidence that
the compile flags are not identical.

> diff --git a/tools/testing/selftests/hid/.gitignore b/tools/testing/selftests/hid/.gitignore

[ ... ]

> diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile
> index 2f423de83147..1d7c64ec992c 100644
> --- a/tools/testing/selftests/hid/Makefile
> +++ b/tools/testing/selftests/hid/Makefile

[ ... ]

> +# progs/hid.c is built into a skeleton by ../lib.bpf.mk.  hid keeps the legacy
> +# progs/<name>.c layout, hence BPF_PROG_EXT.  BPF_EXTRA_HDRS is deferred: it
> +# uses $(BPFDIR), which lib.bpf.mk defines.
> +BPF_SRCS       := $(wildcard progs/*.c)
> +BPF_PROG_EXT   := .c
> +BPF_EXTRA_HDRS  = $(wildcard progs/*.h) $(wildcard $(BPFDIR)/hid_bpf_*.h) \
> +		  $(wildcard $(BPFDIR)/*.bpf.h)
> +# hid.c predates the shared -Wall and has a few unused locals.
> +BPF_EXTRA_CFLAGS := -Wno-unused-variable

This isn't a bug, but would dropping the four unused locals in
progs/hid.c (the `int i` in the three hid_user_* syscall programs and
the unused `int ret` in hid_test_hidraw_output_report) let this
directory build without the -Wno-unused-variable override?

The shared lib.bpf.mk compiles BPF objects with -Wall -Werror, and this
patch reacts by permanently disabling -Wunused-variable for every BPF
program in this directory. The trigger is exactly four dead locals:
`int i` in hid_user_raw_request(), hid_user_output_report() and
hid_user_input_report(), plus an `int ret;` in
hid_test_hidraw_output_report() that is never assigned or read.
Deleting those four declarations would let the directory build with the
shared warning set unmodified, and would keep the suppression from
silently hiding unused locals in BPF programs added here later.

[ ... ]

> +EXTRA_CLEAN += feature bpftool $(addprefix $(OUTPUT)/,*.o no_alu32)

This isn't a bug, but since the no_alu32 build rule goes away in this
patch, is the `no_alu32` (and `feature`) entry in EXTRA_CLEAN still
needed, or could this line shrink to just `$(OUTPUT)/*.o`?

The rewritten EXTRA_CLEAN keeps the `no_alu32` entry, but this patch
removes the only thing that could ever have produced such an artifact:
the CLANG_NOALU32_BPF_BUILD_RULE definition (and hid never had the
selftests/bpf per-flavor subdirectory machinery that creates a
no_alu32/ tree in the first place). Now that the flavor rule is gone,
nothing in this directory can create $(OUTPUT)/no_alu32. The `feature`
entry is in a similar position - libbpf/bpftool feature-detection
output lands under $(BUILD_DIR)/libbpf/, which is already covered by
the $(SCRATCH_DIR) entry that lib.bpf.mk adds.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31820214629

Re: [PATCH v3 4/4] selftests/sched_ext: build BPF schedulers via the shared lib.bpf.mk

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 5410 bytes --]

> diff --git a/tools/testing/selftests/sched_ext/Makefile b/tools/testing/selftests/sched_ext/Makefile
> index 3cfe90e0f34f..a2df47b300d0 100644
> --- a/tools/testing/selftests/sched_ext/Makefile
> +++ b/tools/testing/selftests/sched_ext/Makefile
> @@ -14,48 +14,32 @@ CURDIR := $(abspath .)
>  REPOROOT := $(abspath ../../../..)
>  TOOLSDIR := $(REPOROOT)/tools
>  LIBDIR := $(TOOLSDIR)/lib
> -BPFDIR := $(LIBDIR)/bpf
>  TOOLSINCDIR := $(TOOLSDIR)/include
> -BPFTOOLDIR := $(TOOLSDIR)/bpf/bpftool
>  APIDIR := $(TOOLSINCDIR)/uapi
>  GENDIR := $(REPOROOT)/include/generated
>  GENHDR := $(GENDIR)/autoconf.h
> -SCXTOOLSDIR := $(TOOLSDIR)/sched_ext
>  SCXTOOLSINCDIR := $(TOOLSDIR)/sched_ext/include
>
> -OUTPUT_DIR := $(OUTPUT)/build
> -OBJ_DIR := $(OUTPUT_DIR)/obj
> -INCLUDE_DIR := $(OUTPUT_DIR)/include
> -BPFOBJ_DIR := $(OBJ_DIR)/libbpf
> -SCXOBJ_DIR := $(OBJ_DIR)/sched_ext
> -BPFOBJ := $(BPFOBJ_DIR)/libbpf.a
> -LIBBPF_OUTPUT := $(OBJ_DIR)/libbpf/libbpf.a
> -
> -DEFAULT_BPFTOOL := $(OUTPUT_DIR)/host/sbin/bpftool
> -HOST_OBJ_DIR := $(OBJ_DIR)/host/bpftool
> -HOST_LIBBPF_OUTPUT := $(OBJ_DIR)/host/libbpf/
> -HOST_LIBBPF_DESTDIR := $(OUTPUT_DIR)/host/
> -HOST_DESTDIR := $(OUTPUT_DIR)/host/
> -
> -VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)					\
> -		     $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux)		\
> -		     ../../../../vmlinux					\
> -		     /sys/kernel/btf/vmlinux					\
> -		     /boot/vmlinux-$(shell uname -r)
> -VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
> -ifeq ($(VMLINUX_BTF),)
> -$(error Cannot find a vmlinux for VMLINUX_BTF at any of "$(VMLINUX_BTF_PATHS)")
> -endif
> +# The schedulers are built by ../lib.bpf.mk: modern *.bpf.c layout, a
> +# .bpf.skel.h suffix, and subskeletons.
> +BPF_SRCS        := $(wildcard *.bpf.c)
> +BPF_SKEL_EXT    := .bpf.skel.h
> +BPF_GEN_SUBSKEL := 1
> +# Keep the generated files under build/ as before.  SCXOBJ_DIR (userspace
> +# objects) reuses the BPF object dir.
> +BPF_OBJ_DIR  := $(OUTPUT)/build/obj/sched_ext
> +BPF_SKEL_DIR := $(OUTPUT)/build/include
> +SCXOBJ_DIR   := $(BPF_OBJ_DIR)

The comment says generated files stay 'under build/ as before', but only
two of the five generated-output locations do. lib.bpf.mk hardcodes its own
scratch tree from $(OUTPUT):

tools/testing/selftests/lib.bpf.mk defines:
  SCRATCH_DIR := $(OUTPUT)/tools
  BUILD_DIR   := $(SCRATCH_DIR)/build
  INCLUDE_DIR := $(SCRATCH_DIR)/include
  BPFOBJ      := $(BUILD_DIR)/libbpf/libbpf.a
  HOST_SCRATCH_DIR := $(OUTPUT)/host-tools
  DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool

So relative to the pre-conversion layout:

  build/obj/sched_ext/         BPF + userspace objects   unchanged
  build/include/               skeletons and subskeletons unchanged
  build/obj/libbpf/libbpf.a -> tools/build/libbpf/libbpf.a       moved out
  build/include/vmlinux.h   -> tools/include/vmlinux.h           moved out
  build/include/bpf/*.h     -> tools/include/bpf/*.h             moved out
  build/host/sbin/bpftool   -> tools/sbin/bpftool or host-tools/sbin/bpftool

The layout change itself is fine and is not a leak: lib.bpf.mk already adds
$(SCRATCH_DIR) and $(HOST_SCRATCH_DIR) to EXTRA_CLEAN, so 'make clean'
still removes everything the deleted 'override define CLEAN' used to
remove.

Could the comment be more precise by naming what actually stays put, namely
the BPF objects and the skeleton headers?

[ ... ]

>
>  ################
>  # C schedulers #
>  ################
>
> -override define CLEAN
> -	rm -rf $(OUTPUT_DIR)
> -	rm -f $(TEST_GEN_PROGS)
> -endef
> -
> -# Every testcase takes all of the BPF progs are dependencies by default. This
> +# Every testcase takes all of the BPF progs as dependencies by default. This

[ ... ]

>
>  .DEFAULT_GOAL := all


Separately, the commit message states:

>     sched_ext carried its own ~130 lines of libbpf + bpftool + vmlinux.h +
>     BPF-object + skeleton build machinery.  Replace it with an include of the
>     shared tools/testing/selftests/lib.bpf.mk, making sched_ext the third
>     in-tree consumer of that fragment, after selftests/cgroup and
>     selftests/hid.
>
>     All 28 skeletons and 28 subskeletons keep a same public API before and
>     after, the runner builds and links the same way.

The '28 skeletons and 28 subskeletons' count looks off. The directory holds
29 BPF sources at this commit, so the conversion generates 29 skeletons and
29 subskeletons, not 28:

  $ git ls-tree --name-only f71f601c6ade:tools/testing/selftests/sched_ext \
      | grep '\.bpf\.c$' | wc -l
  29

Both the old wildcard rule and the new BPF_SRCS enumerate the same 29 files,
and the skeleton rule emits one .bpf.skel.h plus one .bpf.subskel.h per
file. The two easy-to-miss sources are maybe_null_fail_dsp.bpf.c and
maybe_null_fail_yld.bpf.c, which have no matching userspace .c file and so
do not appear in auto-test-targets. A count taken from the test list rather
than from *.bpf.c comes out one or two short.

Should this read '29 skeletons and 29 subskeletons'? Also, the same sentence
reads oddly: 'keep a same public API' should be 'keep the same public API',
and the two clauses after it look like a comma splice.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31820214629

Re: [PATCH v3 1/4] selftests: add shared lib.bpf.mk to build BPF progs and skeletons

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 12507 bytes --]

> diff --git a/tools/testing/selftests/lib.bpf.mk b/tools/testing/selftests/lib.bpf.mk
> new file mode 100644
> index 000000000000..433a26a36960
> --- /dev/null
> +++ b/tools/testing/selftests/lib.bpf.mk
> @@ -0,0 +1,296 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Build BPF programs and skeleton headers for selftests, then link them into
> +# test binaries.
> +#
> +# Use it from a test Makefile like this:
> +#
> +#     BPF_SRCS         := foo.bpf.c bar.bpf.c
> +#     TEST_GEN_PROGS   := foo_test
> +#     OVERRIDE_TARGETS := 1              # set before lib.mk
> +#     include ../lib.mk
> +#     include ../lib.bpf.mk
> +#
> +#     $(OUTPUT)/foo_test: foo_test.c $(BPF_SKELS) $(BPFOBJ)
> +#         $(call bpf_link,$@,$<)
> +#
> +# Keep $(BPFOBJ) in the dependencies so make relinks the test when libbpf.a
> +# changes.
> +#
> +# Options to set before including lib.bpf.mk:
> +#   BPF_PROG_EXT     - source ending; default .bpf.c. Use .c for progs/foo.c.
> +#   BPF_EXTRA_HDRS   - more headers needed by the BPF objects.
> +#   BPF_EXTRA_CFLAGS - more flags for compiling BPF programs.
> +#   BPF_SKEL_EXT     - skeleton header ending; default .skel.h.
> +#   BPF_GEN_SUBSKEL  - also create a subskeleton header when set.
> +#   BPF_OBJ_DIR      - folder for BPF objects; default $(OUTPUT).
> +#   BPF_SKEL_DIR     - folder for skeleton headers; default $(OUTPUT).
> +#
> +# You may replace BPF_CFLAGS after including this file. Make reads it when each
> +# object is built. Sources may be in subfolders, but generated files use only
> +# the source filename and are placed directly in their output folder.
> +#
> +# Include lib.mk first. This file uses OUTPUT, top_srcdir, CC, CLANG, Q and msg.
> +# OVERRIDE_TARGETS stops lib.mk from adding its normal link rule. This file sets
> +# BPFOBJ, BPFTOOL, BPF_OBJS, BPF_SKELS and BPF_LDLIBS, and adds to CFLAGS and
> +# EXTRA_CLEAN.

The documented contract lists five variables that this file sets (BPFOBJ,
BPFTOOL, BPF_OBJS, BPF_SKELS, BPF_LDLIBS), but the fragment unconditionally
assigns about twenty more with ':=', several having generic names that an
existing test Makefile might already use: TOOLSDIR, LIBDIR, BPFDIR,
TOOLSINCDIR, BPFTOOLDIR, APIDIR, SCRATCH_DIR, BUILD_DIR, INCLUDE_DIR,
HOST_BUILD_DIR, HOST_SCRATCH_DIR, HOST_BPFOBJ, DEFAULT_BPFTOOL, MAKE_DIRS,
BPF_CFLAGS, CLANG_SYS_INCLUDES, IS_LITTLE_ENDIAN, MENDIAN, CLANG_BPF_CPU,
BPF_SUBSKEL_EXT, submake_extras.

It also defines rules for $(MAKE_DIRS), appends to the phony 'all', and via
'include $(top_srcdir)/tools/scripts/Makefile.arch' it sets ARCH (with the
'override' keyword when ARCH is empty), SRCARCH, HOSTARCH, LP64 and
IS_64_BIT.

The sched_ext conversion already had to rework its own INCLUDE_DIR,
OBJ_DIR, SCXOBJ_DIR and MAKE_DIRS precisely because the names collide.

There's also an asymmetry in assignment operators: BPF_PROG_EXT,
BPF_SKEL_EXT, BPF_OBJ_DIR, BPFTOOL and VMLINUX_BTF* use '?=' so a caller
can influence them, but SCRATCH_DIR, BUILD_DIR and INCLUDE_DIR use ':=', so
a caller cannot relocate the libbpf/bpftool scratch tree even though it
lands inside the test's own directory.

Could the 'This file sets ...' list be completed (and the collision risk
called out), or should the internal names be prefixed to reduce the
collision surface?

>
> +include $(top_srcdir)/tools/scripts/Makefile.arch	# ARCH / SRCARCH / HOSTARCH

[ ... ]

> +# vmlinux.h can contain anonymous struct and union members. Clang needs
> +# -fms-extensions to accept them.
> +BPF_CFLAGS = -g -Wall -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN)	\
> +	     -I$(INCLUDE_DIR) -I$(APIDIR) -I$(TOOLSINCDIR)		\
> +	     -std=gnu11							\
> +	     -fno-strict-aliasing					\
> +	     -fms-extensions -Wno-microsoft-anon-tag			\
> +	     -Wno-compare-distinct-pointer-types			\
> +	     $(CLANG_SYS_INCLUDES) $(BPF_EXTRA_CFLAGS)

A subsystem pattern flags this as potentially concerning: -I$(TOOLSINCDIR)
(tools/include) is added to the BPF-program include path. None of the three
Makefiles this fragment replaces do that, and selftests/bpf/Makefile
deliberately does not either.

It has -I$(TOOLSINCDIR) only in COMMON_CFLAGS for the userspace test
binaries, while its BPF_CFLAGS includes only -I$(INCLUDE_DIR) -I$(CURDIR)
-I$(APIDIR) with no tools/include.

tools/include/linux/ holds 81 host-tool shim headers (err.h, filter.h,
kernel.h, mm.h, module.h, mutex.h, atomic.h, compiler.h, bitops.h, list.h)
whose contents are meant for userspace tools and conflict with vmlinux.h.

Because $(CLANG_SYS_INCLUDES) uses -idirafter, these shims also sort ahead
of /usr/include, so a BPF program that includes any non-uapi <linux/...>
name silently picks up a host shim instead of failing or getting the real
header.

This is currently harmless for all three consumers: -I$(APIDIR) precedes
-I$(TOOLSINCDIR), and the only <linux/...> names actually reached today are
<linux/const.h> (hid/progs/hid_bpf_helpers.h, present in
tools/include/uapi/linux/) and <linux/errno.h> (tools/lib/bpf/*.bpf.h,
present in neither, so it resolves via -idirafter). sched_ext escapes
entirely because it replaces BPF_CFLAGS after the include.

So this is a latent hazard for future consumers rather than a current
breakage, but since no consumer needs it and it diverges from the file's
own stated model (selftests/bpf/Makefile), would dropping -I$(TOOLSINCDIR)
from BPF_CFLAGS be safer?

>
> +# $1 = source, $2 = object. -MMD -MP records every included header, including
> +# headers included by other headers.

[ ... ]

> +# ---- build vmlinux.h ------------------------------------------------------
> +# Replace vmlinux.h only when its contents change. A new timestamp alone would
> +# rebuild every BPF object and skeleton.
> +$(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL) | $(INCLUDE_DIR)
> +ifeq ($(VMLINUX_H),)
> +	$(call msg,GEN,,$@)
> +	$(Q)test -n "$(VMLINUX_BTF)" || { \
> +		echo "lib.bpf.mk: no vmlinux with BTF at any of \"$(VMLINUX_BTF_PATHS)\"" >&2; \
> +		exit 1; }
> +	$(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@.tmp
> +else
> +	$(call msg,CP,,$@)
> +	$(Q)cp "$(VMLINUX_H)" $@.tmp
> +endif
> +	$(Q)cmp -s $@.tmp $@ || mv $@.tmp $@
> +	$(Q)rm -f $@.tmp

A subsystem pattern flags this as potentially concerning: when the dumped
BTF is byte-identical to the existing vmlinux.h, the recipe leaves $@
untouched so its mtime stays older than $(VMLINUX_BTF), and make re-runs
the whole recipe on every subsequent invocation.

'bpftool btf dump file <vmlinux> format c' on a full kernel is a
multi-second, ~100k-line dump, so once the BTF source is newer than the
generated header (after any kernel rebuild, or a fresh
/boot/vmlinux-$(uname -r)) every no-op 'make' in the test directory pays
that cost, permanently.

Both hid and sched_ext previously redirected straight into $@, so the dump
ran once and then the target was up to date. The idiom is inherited from
selftests/bpf/Makefile and the comment correctly explains why the content
check exists, but would 'cmp -s $@.tmp $@ && touch $@ || mv $@.tmp $@' keep
the no-rebuild property while also stopping the repeated dump?

>
> +# ---- BPF objects and skeletons --------------------------------------------
> +BPF_PROG_EXT ?= .bpf.c
> +# Each source must end with BPF_PROG_EXT so Make can remove that ending.
> +bpf_bad_srcs := $(filter-out %$(BPF_PROG_EXT),$(BPF_SRCS))
> +ifneq ($(bpf_bad_srcs),)
> +$(error lib.bpf.mk: BPF_SRCS entries must end in $(BPF_PROG_EXT): $(bpf_bad_srcs))
> +endif
> +bpf_stems := $(patsubst %$(BPF_PROG_EXT),%,$(notdir $(BPF_SRCS)))
> +# Reject any dot left after removing BPF_PROG_EXT. For example, foo.bpf.c leaves
> +# foo.bpf when the ending is set to .c.
> +bpf_dotted_stems := $(filter-out $(basename $(bpf_stems)),$(bpf_stems))
> +ifneq ($(bpf_dotted_stems),)
> +$(error lib.bpf.mk: BPF_SRCS basenames must not contain a dot: $(bpf_dotted_stems))
> +endif
> +# Generated files use only the source filename. Reject equal filenames instead
> +# of letting one replace another.
> +ifneq ($(words $(bpf_stems)),$(words $(sort $(bpf_stems))))
> +$(error lib.bpf.mk: BPF_SRCS has colliding basenames: $(BPF_SRCS))
> +endif

[ ... ]

> +BPF_OBJS  := $(addprefix $(BPF_OBJ_DIR)/,$(addsuffix .bpf.o,$(bpf_stems)))
> +BPF_SKELS := $(addprefix $(BPF_SKEL_DIR)/,$(addsuffix $(BPF_SKEL_EXT),$(bpf_stems)))

[ ... ]

> +# Link three times and compare the last two outputs. A correct linker must not
> +# change an object that it already linked.
> +$(BPF_SKELS): $(BPF_SKEL_DIR)/%$(BPF_SKEL_EXT): $(BPF_OBJ_DIR)/%.bpf.o $(BPFTOOL) | $(BPF_SKEL_DIR)

BPF_SRCS is the fragment's one mandatory input, and the validation above
checks three different ways it can be malformed (wrong extension, dotted
basename, colliding basenames) with a clear $(error) message. The one
remaining malformed value - empty - is not checked, and it is the case that
degrades worst.

With an empty BPF_SRCS, bpf_stems, BPF_OBJS and BPF_SKELS are all empty,
and the static pattern rule above is read by make as a rule whose target
list is empty - a rule with nothing before the first colon. GNU make
rejects this outright, so the *include* of ../lib.bpf.mk aborts with a
generic parse error naming lib.bpf.mk:252 rather than with one of the
fragment's own diagnostics, and it does so for 'make clean' and 'make
install' too. This is the failure mode the author deliberately avoided for
the missing-vmlinux case (see the comment at lines 105-106 and the runtime
guard at line 199).

This is reachable through the way the fragment is actually being adopted:
two of the three consumers in this series derive BPF_SRCS from a wildcard
whose expansion depends on the working tree - 'BPF_SRCS := $(wildcard
progs/*.c)' (selftests/hid) and 'BPF_SRCS := $(wildcard *.bpf.c)'
(selftests/sched_ext). Any tree where that glob comes back empty (a partial
copy, a directory whose programs were removed or renamed, or a future user
who sets BPF_SRCS from a variable that is not set) turns a
should-be-obvious configuration mistake into an unexplained make failure in
a shared file.

Would a fourth guard next to the existing three, e.g.

    ifeq ($(BPF_SRCS),)
    $(error lib.bpf.mk: BPF_SRCS is empty; set it before including lib.bpf.mk)
    endif

make it self-describing and consistent with the checks immediately above?

> +	$(call msg,GEN-SKEL,,$@)

[ ... ]

> +# Read the header dependencies written by -MMD.
> +-include $(BPF_OBJS:.o=.d)
> +
> +# ---- values for the test Makefile -----------------------------------------
> +# Add paths for generated skeletons and vmlinux.h. Put the new libbpf headers
> +# first so an older copy under BPF_SKEL_DIR cannot be used by mistake.
> +CFLAGS += -I$(INCLUDE_DIR) -I$(OUTPUT) -I$(BPF_SKEL_DIR)

The comment does not describe what the line does, and the stated hazard
cannot occur.

BPF_SKEL_DIR is the directory this fragment writes skeleton headers into -
the only things placed there are *$(BPF_SKEL_EXT) and *$(BPF_SUBSKEL_EXT).
libbpf's headers are installed elsewhere: 'install_headers' in
tools/lib/bpf/Makefile writes only to $(DESTDIR)$(prefix)/include/bpf/,
which with 'DESTDIR=$(SCRATCH_DIR) prefix=' is $(INCLUDE_DIR)/bpf/.
Nothing in this fragment can ever put a libbpf header under BPF_SKEL_DIR,
so there is no stale copy for the ordering to protect against. The ordering
rationale is not inherited from the code being consolidated either -
selftests/bpf/Makefile has the opposite order, '-I$(CURDIR)
-I$(INCLUDE_DIR) ...'.

The comment also explains a two-element ordering while the line adds three
paths, and silently skips the middle one. -I$(OUTPUT) is unexplained and
does no work in either configuration: with the default 'BPF_SKEL_DIR ?=
$(OUTPUT)' it is an exact duplicate of the -I$(BPF_SKEL_DIR) that follows
it, and when a consumer overrides BPF_SKEL_DIR (sched_ext sets
'$(OUTPUT)/build/include') nothing this fragment generates lives in
$(OUTPUT) for it to find.

For a new shared file whose comments are the interface documentation for
future consumers, would it be worth either dropping -I$(OUTPUT) and the
invented rationale, or replacing the comment with the real reason the order
was chosen?

>
> +# Add target zstd when pkg-config finds it. A cross build must use the target
> +# pkg-config, not the host one.

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31820214629

Re: [PATCH v3 2/4] selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 13101 bytes --]

> diff --git a/tools/testing/selftests/cgroup/config b/tools/testing/selftests/cgroup/config
> index 39f979690dd3..356b6a533d88 100644
> --- a/tools/testing/selftests/cgroup/config
> +++ b/tools/testing/selftests/cgroup/config
> @@ -4,3 +4,8 @@ CONFIG_CGROUP_FREEZER=y
>  CONFIG_CGROUP_SCHED=y
>  CONFIG_MEMCG=y
>  CONFIG_PAGE_COUNTER=y
> +CONFIG_BPF=y
> +CONFIG_BPF_SYSCALL=y
> +CONFIG_DEBUG_INFO=y
> +CONFIG_DEBUG_INFO_BTF=y
> +CONFIG_DEBUG_INFO_DWARF4=y

A subsystem pattern flags this as potentially concerning: CONFIG_BPF_EVENTS=y
is missing. The new program is SEC("iter.s/cgroup"), which is
BPF_PROG_TYPE_TRACING, and that program type is only registered inside
'#ifdef CONFIG_BPF_EVENTS' in include/linux/bpf_types.h:38. Without it
find_prog_type() returns -EINVAL and the skeleton load fails at runtime
even though the build succeeded.

CONFIG_BPF_EVENTS is 'default y' but has 'depends on (KPROBE_EVENTS ||
UPROBE_EVENTS) && PERF_EVENTS' (kernel/trace/Kconfig:853), so a kselftest
kernel configured only from the merged fragments can easily end up without
it. tools/testing/selftests/bpf/config:5 sets CONFIG_BPF_EVENTS=y
explicitly for exactly this reason.

Note also that CONFIG_DEBUG_INFO is not promptable (it is selected by the
DEBUG_INFO_* choice), so that line is a no-op.

> diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
> index e01584c2189a..01927511c059 100644
> --- a/tools/testing/selftests/cgroup/Makefile
> +++ b/tools/testing/selftests/cgroup/Makefile

[ ... ]

> @@ -33,3 +75,13 @@ $(OUTPUT)/test_kmem: $(LIBCGROUP_O)
>  $(OUTPUT)/test_memcontrol: $(LIBCGROUP_O)
>  $(OUTPUT)/test_pids: $(LIBCGROUP_O)
>  $(OUTPUT)/test_zswap: $(LIBCGROUP_O)
> +
> +ifneq ($(HAVE_BPF),)
> +# Links cgroup_util and the skeleton against the libbpf lib.bpf.mk built.  The
> +# uapi include is private to this binary: the other tests here must keep
> +# resolving <linux/*.h> against the system headers.
> +$(OUTPUT)/test_memcg_stat_cross_cpu: private CFLAGS += -I$(top_srcdir)/tools/include/uapi
> +$(OUTPUT)/test_memcg_stat_cross_cpu: test_memcg_stat_cross_cpu.c \
> +					$(BPF_SKELS) $(LIBCGROUP_O) $(BPFOBJ)
> +	$(call bpf_link,$@,$< $(LIBCGROUP_O))
> +endif

A subsystem pattern flags this as potentially concerning:
memcg_stat_cross_cpu.h is not listed as a prerequisite of the userspace
binary, and BPF_EXTRA_HDRS (documented by the new
tools/testing/selftests/lib.bpf.mk as 'more headers needed by the BPF
objects') is left unset, so the BPF object does not depend on it either.

That header defines struct memcg_stat_snapshot, which is the wire format
shared between the BPF program and the test binary. Editing it therefore
rebuilds neither side, and an incremental build can silently produce a
binary whose struct layout disagrees with the loaded skeleton.

The sibling conversion commit ef5ceab9d440 (selftests/hid) does set
BPF_EXTRA_HDRS for its shared header, so the mechanism was available.
LOCAL_HDRS would cover the C side.

> diff --git a/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
> new file mode 100644
> index 000000000000..3c2acd49c16b
> --- /dev/null
> +++ b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c

[ ... ]

> +/*
> + * Flush once at the subtree root, then read each cgroup through the kfuncs.
> + * iter.s because the flush kfunc may sleep.
> + */
> +SEC("iter.s/cgroup")
> +int cgroup_memcg_stat_cross_cpu(struct bpf_iter__cgroup *ctx)
> +{
> +	struct cgroup *cgrp = ctx->cgroup;
> +	struct memcg_stat_snapshot snap = {};
> +	struct cgroup_subsys_state *css;
> +	struct mem_cgroup *memcg;
> +	int idx_anon, idx_file, idx_shmem, idx_fmapped, idx_pgfault;
> +	__u64 cg_id;
> +
> +	/* The walk ends with a NULL element; return 0 so it runs to the end. */
> +	if (!cgrp)
> +		return 0;

A subsystem pattern flags this as potentially concerning: the comment
misstates the mechanism and will mislead the next reader. The NULL element
is the epilogue: cgroup_iter_seq_stop() invokes
__cgroup_iter_seq_show(seq, NULL, true) only after cgroup_unlock() and
only once the walk has already finished, and it sets p->visited_all = true
immediately afterwards.

__cgroup_iter_seq_show() converts a non-zero return into p->terminate =
true, but nothing reads p->terminate after that point, so returning 0
rather than non-zero for the NULL element has no effect on how far the
walk gets. The pre-existing comparable program
tools/testing/selftests/bpf/progs/cgroup_iter_memcg.c returns 1 for the
NULL element. The code is functionally correct; only the stated rationale
is wrong.

> diff --git a/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c b/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c
> new file mode 100644
> index 000000000000..9946c41c7a90
> --- /dev/null
> +++ b/tools/testing/selftests/cgroup/test_memcg_stat_cross_cpu.c

[ ... ]

> +static size_t leaf_charge(size_t want, int k)
> +{
> +	long online = sysconf(_SC_NPROCESSORS_ONLN);
> +	size_t floor;
> +
> +	floor = (size_t)FLUSH_MARGIN * MEMCG_CHARGE_BATCH * online *
> +		page_size / n_leaves;
> +	if (want < floor)
> +		want = floor;
> +	if (want < (size_t)k * page_size)
> +		want = (size_t)k * page_size;
> +
> +	return (want + page_size - 1) / page_size * page_size;
> +}

Does this handle the sysconf(_SC_NPROCESSORS_ONLN) failure case correctly?
The return value is not checked, and the expression it feeds is unsigned.
Because of the leading (size_t) cast, online is converted to size_t, so a
-1 return makes floor ~= (2^64 - 1048576)/n_leaves instead of a small byte
count.

want is then clamped up to that value and returned, and start_chargers()
passes it to charge_leaf(), where malloc(ca->bytes) fails, the charger
exits, and the test reports the misleading "%s never reached its charge"
failure. The same file already guards the analogous call: main() does
'page_size = sysconf(_SC_PAGESIZE); if (page_size <= 0) page_size =
BUF_SIZE;' (lines 947-949), and this is the only unchecked sysconf() in
the whole cgroup selftest directory.

> +static int start_chargers(int k, size_t bytes)
> +{
> +	struct charge_args ca = { .bytes = bytes, .k = k };
> +	long prev, cur;
> +	int i, retries;
> +
> +	for (i = 0; i < n_nodes; i++) {
> +		if (!nodes[i].is_leaf)
> +			continue;
> +		if (cg_run_nowait(nodes[i].path, charge_leaf, &ca) < 0) {
> +			ksft_print_msg("cannot start a charger on %s\n",
> +				       nodes[i].path);
> +			return -1;
> +		}
> +		ca.base += k;
> +	}

Can the per-leaf charge sizing in leaf_charge() cause memory pressure? The
per-leaf charge is raised to at least k * page_size, and run_case() sets k
= n_cpu for the 'cross_cpu' cases, so the total anon the test pins is
n_leaves * n_cpu * page_size with no ceiling. For the two large-tree cases
(fanout 10, depth 3) n_leaves is 1000, which is 1000 * n_cpu * 4K: 256 MB
on a 64-CPU host, 1 GB at 256 CPUs, 4 GB at 1024 CPUs, held simultaneously
by 1000 live processes.

The file's own header comment states the requirement this violates: "The
comparison is exact, which needs the subtree quiesced ... Global reclaim
would move the numbers and the test would report a mismatch", and
check_tree() runs with STAT_TOLERANCE_PCT 0, so any reclaim during the run
turns into a hard KSFT_FAIL rather than a skip. Nothing in the test
compares the computed footprint against MemAvailable or caps it.

The same sizing also feeds the charge-completion deadline in
start_chargers(): CHARGE_WAIT_RETRIES (100) * DEFAULT_WAIT_INTERVAL_US/10
(10 ms) gives each leaf only ~1 s to both reach bytes and read the same
memory.current twice in a row, while up to 1000 chargers are concurrently
performing k sched_setaffinity() migrations each.

> +	for (i = 0; i < n_nodes; i++) {
> +		if (!nodes[i].is_leaf)
> +			continue;
> +		/*
> +		 * Wait for the charge to both cover the region and stop
> +		 * moving.
> +		 */
> +		prev = -1;
> +		for (retries = CHARGE_WAIT_RETRIES; retries; retries--) {
> +			cur = cg_read_long(nodes[i].path, "memory.current");
> +			if (cur >= (long)bytes && cur == prev)
> +				break;
> +			prev = cur;
> +			usleep(DEFAULT_WAIT_INTERVAL_US / 10);
> +		}
> +		if (!retries) {
> +			ksft_print_msg("%s never reached its charge\n",
> +				       nodes[i].path);
> +			return -1;
> +		}
> +	}
> +	return 0;
> +}

A subsystem pattern flags this as potentially concerning: start_chargers()
open-codes a retry/poll loop over a cgroup control file instead of using
(or extending) the cgroup selftest library's polling helpers.

lib/include/cgroup_util.h already exports cg_read_key_long_poll(cgroup,
control, key, expected, retries, wait_interval_us),
cg_wait_for()/cg_prepare_for_wait() and cg_wait_for_proc_count(), and this
loop also silently treats cg_read_long()'s -1 error return as "not charged
yet" and burns a retry on it, which the shared helper does not do (it
returns the error).

The existing helper is not a drop-in substitute: cg_read_key_long_poll()
polls a memory.stat key for exact equality against expected and returns on
the first read error, whereas this loop needs 'memory.current >= bytes AND
unchanged across two reads' on a different file. So the semantics genuinely
differ and a bespoke loop is defensible; the guide's underlying point still
applies in that this series already extends the shared library
(cg_get_id/cg_touch_pages/cg_alloc_anon), so a 'wait until >= X and stable'
helper could live there too and pick up the missing error handling for
free.

> +/*
> + * Collect what the kfuncs report.  The program flushes at the first cgroup it
> + * sees, the subtree root in DESCENDANTS_PRE order.
> + */
> +static int read_bpf(int root_fd)
> +{
> +	DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
> +	struct memcg_stat_cross_cpu *skel = NULL;
> +	union bpf_iter_link_info linfo = {};
> +	struct bpf_link *link = NULL;
> +	int ret = -1, i, mfd, fd;
> +	char buf[4096];
> +	ssize_t r;
> +
> +	skel = memcg_stat_cross_cpu__open();
> +	if (!skel) {
> +		ksft_print_msg("skel open failed\n");
> +		return -1;
> +	}
> +	if (bpf_map__set_max_entries(skel->maps.results, n_nodes + 8)) {
> +		ksft_print_msg("set max_entries failed\n");
> +		goto out;
> +	}
> +	if (memcg_stat_cross_cpu__load(skel)) {
> +		ksft_print_msg("skel load failed\n");
> +		goto out;
> +	}
> +
> +	linfo.cgroup.cgroup_fd = root_fd;
> +	linfo.cgroup.order = BPF_CGROUP_ITER_DESCENDANTS_PRE;
> +	opts.link_info = &linfo;
> +	opts.link_info_len = sizeof(linfo);
> +
> +	link = bpf_program__attach_iter(skel->progs.cgroup_memcg_stat_cross_cpu,
> +					&opts);
> +	if (!link) {
> +		ksft_print_msg("attach iter failed\n");
> +		goto out;
> +	}
> +
> +	fd = bpf_iter_create(bpf_link__fd(link));
> +	if (fd < 0) {
> +		ksft_print_msg("bpf_iter_create failed\n");
> +		goto out;
> +	}
> +	while ((r = read(fd, buf, sizeof(buf))) > 0)
> +		;
> +	close(fd);
> +	if (r) {
> +		ksft_print_msg("bpf walk failed\n");
> +		goto out;
> +	}

A subsystem pattern flags this as potentially concerning: every BPF
capability shortfall in read_bpf() is reported as a test FAILURE rather
than a SKIP.

The feature gate is narrow. memcg_kfuncs_available() (line 907) probes
exactly one symbol:

  ok = btf__find_by_name_kind(btf, "bpf_get_mem_cgroup", BTF_KIND_FUNC) > 0;

but memcg_stat_cross_cpu.bpf.c additionally requires bpf_put_mem_cgroup,
bpf_mem_cgroup_flush_stats, bpf_mem_cgroup_page_state and
bpf_mem_cgroup_vm_events, plus kernel support for a sleepable cgroup
iterator (SEC("iter.s/cgroup")). On a kernel that exports
bpf_get_mem_cgroup but not the rest of the set (a partial backport, or a
future kernel that renames/retires one of them),
memcg_stat_cross_cpu__load() fails with a capability-absent errno (-ENOENT
for an unresolvable __ksym kfunc, -EOPNOTSUPP/-EINVAL for an unsupported
sleepable iter). The same applies to bpf_program__attach_iter() and
bpf_iter_create(), which also fail with -EOPNOTSUPP/-EPERM when the
capability or privilege is absent.

All three paths take 'goto out' and return -1. run_case() initialises 'ret
= KSFT_FAIL' and only ever upgrades it to KSFT_PASS, so it has no
KSFT_SKIP or ksft_test_result_skip() path at all. The result is a hard
"not ok" in the TAP output for a machine that simply lacks the
prerequisite. The diagnostics also make this indistinguishable from a
genuine failure: none of the four messages ("skel load failed", "attach
iter failed", "bpf_iter_create failed", "bpf walk failed") carry errno or
strerror(errno), so a CI operator cannot tell a missing-capability
environment from a real flush regression.

> diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
> index 0ebf796f3cff..15ba46879504 100644
> --- a/tools/testing/selftests/cgroup/test_memcontrol.c
> +++ b/tools/testing/selftests/cgroup/test_memcontrol.c

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31820214629