ne 24. 5. 2026 v 18:24 odesÃlatel Ben Hutchings <[email protected]> napsal: > > Some output files (src/timerlat.bpf.o, src/timerlat.skel.h, > example/timerlat_bpf_action.o, tests/bpf/bpf_action_map.o) are > currently generated in the source tree, preventing a fully out-of-tree > build. To fix this: > > - Add $(OUTPUT) to their filenames in the relevant Makefile rules, and > create subdirectories as needed > - Add $(OUTPUT)src to the include path > - Add ${OUTPUT} to the BPF object filename in tests/timerlat.t > > Fixes: e34293ddcebd ("rtla/timerlat: Add BPF skeleton to collect samples") > Fixes: 0304a3b7ec9a ("rtla/timerlat: Add example for BPF action program") > Fixes: 5525aebd4e0c ("rtla/tests: Test BPF action program") > Signed-off-by: Ben Hutchings <[email protected]> > Reviewed-by: Ian Rogers <[email protected]> > --- > tools/tracing/rtla/Makefile | 31 ++++++++++++++++++----------- > tools/tracing/rtla/tests/timerlat.t | 4 ++-- > 2 files changed, 21 insertions(+), 14 deletions(-) >
It seems that I did take building out-of-tree into account in my latest build changes (in the libsubcmd migration; actually I even fixed some smaller bits in v3 after seeing your patch), but not for earlier generated files. Thanks for catching that. Can you please rebase this on top of linux-trace/tools/for-next [1]? There have been quite a lot of changes queued to the next cycle that conflict now with this patch, namely: 1. changes to the Makefile coming from [3], conflicting around INCLUDES and RTLA_IN rule 2. changes in runtime tests (highlighted below) [1] https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git/log/?h=tools/for-next > diff --git a/tools/tracing/rtla/Makefile b/tools/tracing/rtla/Makefile > index 45690ee14544..f54da7be735d 100644 > --- a/tools/tracing/rtla/Makefile > +++ b/tools/tracing/rtla/Makefile > @@ -66,30 +66,37 @@ ifeq ($(config),1) > include Makefile.config > endif > > +INCLUDES = -I$(OUTPUT)src > + INCLUDES is currently unset by the Makefile, and there are users that pass it to "make", e.g. [2]. Ideally, the name for user-specific includes would be EXTRA_INCLUDES or something like that, and INCLUDES would be used for the generated include files, but the name has already been set since the beginning of rtla, so this has to be named "GEN_INCLUDES" (or similar) instead. Otherwise, a user setting INCLUDES will override the -I$(OUTPUT)src and out-of-tree build would fail. [2] https://gitlab.com/cki-project/kernel-ark/-/blob/4dda3a32c5ef22768f6daa126a595974955e3c10/redhat/kernel.spec.template#L3375 > CFLAGS += $(INCLUDES) $(LIB_INCLUDES) > > export CFLAGS OUTPUT srctree > > ifeq ($(BUILD_BPF_SKEL),1) > -src/timerlat.bpf.o: src/timerlat.bpf.c > +$(OUTPUT)src/timerlat.bpf.o: src/timerlat.bpf.c > + mkdir -p $(@D) The mkdir will always echo when building the file, it'd be better to use $(Q)$(MKDIR) here (and in the other rules below) like in the new LIBSUBCMD_DIR and LIB_DIR rules [3]. Also, could the mkdir be separated into a target here, too? E.g.: SRC_OUTPUT = $(OUTPUT)src/ $(SRC_OUTPUT): $(Q)$(MKDIR) -p $@ and then include it as order-only prerequisite: $(SRC_OUTPUT)timerlat.bpf.o: src/timerlat.bpf.c | $(SRC_OUTPUT) $(QUIET_CLANG)$(CLANG) -g -O2 -target bpf -c $(filter %.c,$^) -o $@ That will remove the need to duplicate the mkdir command, plus the SRC_OUTPUT variable can be used in place of $(OUTPUT)src for all generated / non-build rules. [3] https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git/tree/tools/tracing/rtla/Makefile?h=tools/for-next&id=db956bcf8d681b5a01ebe04c79f6a7b29b9934f9#n144 > $(QUIET_CLANG)$(CLANG) -g -O2 -target bpf -c $(filter %.c,$^) -o $@ > > -src/timerlat.skel.h: src/timerlat.bpf.o > +$(OUTPUT)src/timerlat.skel.h: $(OUTPUT)src/timerlat.bpf.o > + mkdir -p $(@D) > $(QUIET_GENSKEL)$(SYSTEM_BPFTOOL) gen skeleton $< > $@ > > -example/timerlat_bpf_action.o: example/timerlat_bpf_action.c > +$(OUTPUT)example/timerlat_bpf_action.o: example/timerlat_bpf_action.c > + mkdir -p $(@D) > $(QUIET_CLANG)$(CLANG) -g -O2 -target bpf -c $(filter %.c,$^) -o $@ > > -tests/bpf/bpf_action_map.o: tests/bpf/bpf_action_map.c > +$(OUTPUT)tests/bpf/bpf_action_map.o: tests/bpf/bpf_action_map.c > + mkdir -p $(@D) > $(QUIET_CLANG)$(CLANG) -g -O2 -target bpf -c $(filter %.c,$^) -o $@ > else > -src/timerlat.skel.h: > - $(Q)echo '/* BPF skeleton is disabled */' > src/timerlat.skel.h > +$(OUTPUT)src/timerlat.skel.h: > + mkdir -p $(@D) > + $(Q)echo '/* BPF skeleton is disabled */' > $@ > > -example/timerlat_bpf_action.o: example/timerlat_bpf_action.c > +$(OUTPUT)example/timerlat_bpf_action.o: example/timerlat_bpf_action.c > $(Q)echo "BPF skeleton support is disabled, skipping > example/timerlat_bpf_action.o" > > -tests/bpf/bpf_action_map.o: tests/bpf/bpf_action_map.c > +$(OUTPUT)tests/bpf/bpf_action_map.o: tests/bpf/bpf_action_map.c > $(Q)echo "BPF skeleton support is disabled, skipping > tests/bpf/bpf_action_map.o" > endif > > @@ -103,7 +110,7 @@ static: $(RTLA_IN) > rtla.%: fixdep FORCE > make -f $(srctree)/tools/build/Makefile.build dir=. $@ > > -$(RTLA_IN): fixdep FORCE src/timerlat.skel.h > +$(RTLA_IN): fixdep FORCE $(OUTPUT)src/timerlat.skel.h > make $(build)=rtla > > clean: doc_clean fixdep-clean > @@ -111,10 +118,10 @@ clean: doc_clean fixdep-clean > $(Q)find . -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name > '\.*.d' -delete > $(Q)rm -f rtla rtla-static fixdep FEATURE-DUMP rtla-* > $(Q)rm -rf feature > - $(Q)rm -f src/timerlat.bpf.o src/timerlat.skel.h > example/timerlat_bpf_action.o > + $(Q)rm -f $(OUTPUT)src/timerlat.bpf.o $(OUTPUT)src/timerlat.skel.h > $(OUTPUT)example/timerlat_bpf_action.o Maybe instead of adding $(OUTPUT) to all of these files, $(OUTPUT) could be passed to the "find" command on the first line? Like this: $(Q)find $(OUTPUT) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete That will also remove the necessity of manually running "rm" on src/timerlat.bpf.o and src/timerlat.skel.h, only $(OUTPUT)src/timerlat.skel.h has to stay. > $(Q)rm -f $(UNIT_TESTS) This is correct. $(UNIT_TESTS) is defined in Makefile.unit, like this: UNIT_TESTS := $(OUTPUT)unit_tests The cleanup of the newly added LIB_OUTPUT and LIBSUBCMD_OUTPUT should already be implemented correctly in the target lib_clean / libsubcmd_clean, no need to change anything for that, either. > > -check: $(RTLA) tests/bpf/bpf_action_map.o > +check: $(RTLA) $(OUTPUT)tests/bpf/bpf_action_map.o > RTLA=$(RTLA) BPFTOOL=$(SYSTEM_BPFTOOL) prove -o -f -v tests/ > -examples: example/timerlat_bpf_action.o > +examples: $(OUTPUT)example/timerlat_bpf_action.o > .PHONY: FORCE clean check > diff --git a/tools/tracing/rtla/tests/timerlat.t > b/tools/tracing/rtla/tests/timerlat.t > index fd4935fd7b49..e0f3fc4df655 100644 > --- a/tools/tracing/rtla/tests/timerlat.t > +++ b/tools/tracing/rtla/tests/timerlat.t > @@ -74,12 +74,12 @@ then > # Test BPF action program properly in BPF mode > [ -z "$BPFTOOL" ] && BPFTOOL=bpftool > check "hist with BPF action program (BPF mode)" \ > - "timerlat hist -T 2 --bpf-action tests/bpf/bpf_action_map.o > --on-threshold shell,command='$BPFTOOL map dump name rtla_test_map'" \ > + "timerlat hist -T 2 --bpf-action > ${OUTPUT}tests/bpf/bpf_action_map.o --on-threshold shell,command='$BPFTOOL > map dump name rtla_test_map'" \ > 2 '"value": 42' > else > # Test BPF action program failure in non-BPF mode > check "hist with BPF action program (non-BPF mode)" \ > - "timerlat hist -T 2 --bpf-action tests/bpf/bpf_action_map.o" \ > + "timerlat hist -T 2 --bpf-action > ${OUTPUT}tests/bpf/bpf_action_map.o" \ > 1 "BPF actions are not supported in tracefs-only mode" As mentioned above, this conflicts with [4] from linux-trace/tools/for-next, as check was replaced with check_top_q_hist and "timerlat hist" with "timerlat TOOL". Also note that another patch [5] changes the path to use $testdir instead of tests, as tests now run in a temporary directory as CWD, however ${OUTPUT}tests is still the correct path, not $testdir, as it references a built artifact (unlike the other uses of $testdir, which reference auxiliary scripts). [4] https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git/commit/tools/tracing/rtla?h=tools/for-next&id=c15c55c01e48e4639c715370f41d9d4c490f5d23 [5] https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git/commit/tools/tracing/rtla?h=tools/for-next&id=ad5b50a0959fb67100ddb93aeb0ea40201272cb2 > fi > done Thanks, Tomas
