Re: [PATCH v5 03/14] perf test: Add build infra for perf test tools for CoreSight tests

From: Carsten Haitzler
Date: Wed Aug 10 2022 - 13:37:13 EST




On 8/7/22 04:59, Leo Yan wrote:
On Thu, Jul 28, 2022 at 03:52:45PM +0100, carsten.haitzler@xxxxxxxxxxxx wrote:
From: "Carsten Haitzler (Rasterman)" <raster@xxxxxxxxxxxxx>

This adds the initial build infrastructure (makefiles maintainers
information) for adding follow-on tests for CoreSight.

Signed-off-by: Carsten Haitzler <carsten.haitzler@xxxxxxx>
---
MAINTAINERS | 1 +
tools/perf/Makefile.perf | 18 ++++++++++---
tools/perf/tests/shell/coresight/Makefile | 26 +++++++++++++++++++
.../tests/shell/coresight/Makefile.miniconfig | 24 +++++++++++++++++
4 files changed, 66 insertions(+), 3 deletions(-)
create mode 100644 tools/perf/tests/shell/coresight/Makefile
create mode 100644 tools/perf/tests/shell/coresight/Makefile.miniconfig

diff --git a/MAINTAINERS b/MAINTAINERS
index 171563d8dc14..87e4ac463429 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1989,6 +1989,7 @@ F: drivers/hwtracing/coresight/*
F: include/dt-bindings/arm/coresight-cti-dt.h
F: include/linux/coresight*
F: samples/coresight/*
+F: tools/perf/tests/shell/coresight/*
F: tools/perf/arch/arm/util/auxtrace.c
F: tools/perf/arch/arm/util/cs-etm.c
F: tools/perf/arch/arm/util/cs-etm.h
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 8f738e11356d..edb621ace2e2 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -629,7 +629,15 @@ sync_file_range_tbls := $(srctree)/tools/perf/trace/beauty/sync_file_range.sh
$(sync_file_range_arrays): $(linux_uapi_dir)/fs.h $(sync_file_range_tbls)
$(Q)$(SHELL) '$(sync_file_range_tbls)' $(linux_uapi_dir) > $@
-all: shell_compatibility_test $(ALL_PROGRAMS) $(LANG_BINDINGS) $(OTHER_PROGRAMS)
+TESTS_CORESIGHT_DIR := $(srctree)/tools/perf/tests/shell/coresight
+
+tests-coresight-targets: FORCE
+ $(Q)$(MAKE) -C $(TESTS_CORESIGHT_DIR)
+
+tests-coresight-targets-clean:
+ $(Q)$(MAKE) -C $(TESTS_CORESIGHT_DIR) clean
+
+all: shell_compatibility_test $(ALL_PROGRAMS) $(LANG_BINDINGS) $(OTHER_PROGRAMS) tests-coresight-targets
# Create python binding output directory if not already present
_dummy := $(shell [ -d '$(OUTPUT)python' ] || mkdir -p '$(OUTPUT)python')
@@ -1015,7 +1023,10 @@ install-tests: all install-gtk
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/shell'; \
$(INSTALL) tests/shell/*.sh '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/shell'; \
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/shell/lib'; \
- $(INSTALL) tests/shell/lib/*.sh '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/shell/lib'
+ $(INSTALL) tests/shell/lib/*.sh '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/shell/lib'; \
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/shell/coresight'; \
+ $(INSTALL) tests/shell/coresight/*.sh '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/shell/coresight'
+ $(Q)$(MAKE) -C tests/shell/coresight install-tests
install-bin: install-tools install-tests install-traceevent-plugins
@@ -1085,7 +1096,7 @@ endif # BUILD_BPF_SKEL
bpf-skel-clean:
$(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
-clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean
+clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean tests-coresight-targets-clean
$(call QUIET_CLEAN, core-objs) $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS)
$(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
$(Q)$(RM) $(OUTPUT).config-detected
@@ -1143,5 +1154,6 @@ FORCE:
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
.PHONY: .FORCE-PERF-VERSION-FILE TAGS tags cscope FORCE prepare
.PHONY: libtraceevent_plugins archheaders
+.PHONY: $(TESTS_CORESIGHT_TARGETS)

TESTS_CORESIGHT_TARGETS is not used anywhere else, should remove it?

I think this was left over from a previous iteration of this work and I removed it later on and forgot about this.

endif # force_fixdep
diff --git a/tools/perf/tests/shell/coresight/Makefile b/tools/perf/tests/shell/coresight/Makefile
new file mode 100644
index 000000000000..3b816bb4ced3
--- /dev/null
+++ b/tools/perf/tests/shell/coresight/Makefile
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# Carsten Haitzler <carsten.haitzler@xxxxxxx>, 2021
+include ../../../../../tools/scripts/Makefile.include
+include ../../../../../tools/scripts/Makefile.arch
+include ../../../../../tools/scripts/utilities.mak
+
+SUBDIRS =
+
+all: $(SUBDIRS)
+$(SUBDIRS):
+ $(Q)$(MAKE) -C $@
+
+INSTALLDIRS = $(SUBDIRS:%=install-%)
+
+install-tests: $(INSTALLDIRS)
+$(INSTALLDIRS):
+ $(Q)$(MAKE) -C $(@:install-%=%) install-tests

I can see here you add a prefix "install-" for every sub directory
string, and then removed this prefix when invoke "make -C" command.

install-tests is a target to install the tests on the host in a known location. it was already there. i added handling of this. there are other install- taregts like install-doc, install-bin and so on. i passed that through so *IF* there is a doc: or similar rule to generate docs then it runs the doc rule first to ensure docs are built before doing the install. this isn't actually used but it's passing along rules. i.e. make install-doc -> make doc etc.

I know you have reason for doing this way, could you explain why not
directly use $(SUBDIRS) for INSTALLDIRS?

+
+CLEANDIRS = $(SUBDIRS:%=clean-%)
+
+clean: $(CLEANDIRS)
+$(CLEANDIRS):
+ $(Q)$(MAKE) -C $(@:clean-%=%) clean >/dev/null

Same question of using prefix "clean-" for variable CLEANDIRS.

Same answer as above.

+
+.PHONY: all clean $(SUBDIRS) $(CLEANDIRS) $(INSTALLDIRS)
+
diff --git a/tools/perf/tests/shell/coresight/Makefile.miniconfig b/tools/perf/tests/shell/coresight/Makefile.miniconfig
new file mode 100644
index 000000000000..a65482d769ab
--- /dev/null
+++ b/tools/perf/tests/shell/coresight/Makefile.miniconfig
@@ -0,0 +1,24 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# Carsten Haitzler <carsten.haitzler@xxxxxxx>, 2021
+
+ifndef DESTDIR
+prefix ?= $(HOME)
+endif
+
+DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
+perfexecdir = libexec/perf-core
+perfexec_instdir = $(perfexecdir)
+
+ifneq ($(filter /%,$(firstword $(perfexecdir))),)
+perfexec_instdir = $(perfexecdir)
+else
+perfexec_instdir = $(prefix)/$(perfexecdir)
+endif
+
+perfexec_instdir_SQ = $(subst ','\'',$(perfexec_instdir))

I know these variables are copied from Makefile.config, just wandering
if we can directly export these variables from high level makefile, so
the sub-make can inherit the variables.

E.g. we can add below line in Makefile.config:

export perfexec_instdir_SQ

Please let me know if you observe any issue for this.

I can indeed simplify that a bit and remove a few lines.

Thanks,
Leo

+INSTALL = install
+INSTDIR_SUB = tests/shell/coresight
+
+include ../../../../../scripts/Makefile.include
+include ../../../../../scripts/Makefile.arch
+include ../../../../../scripts/utilities.mak
--
2.32.0