linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/2] Introduce perf-stat -b for BPF programs
@ 2020-11-19  4:50 Song Liu
  2020-11-19  4:50 ` [RFC 1/2] perf: support build BPF skeletons with perf Song Liu
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Song Liu @ 2020-11-19  4:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, jolsa, namhyung, Song Liu

This set introduces perf-stat -b option to count events for BPF programs.
This is similar to bpftool-prog-profile. But perf-stat makes it much more
flexible.

Sending as RFC because I would like comments on some key design choices:
  1. We are using BPF skeletons here, which is by far the easiest way to
     write and ship BPF programs. However, this requires bpftool, which
     makes building perf slower.
  2. A Makefile is added to tools/perf/util/bpf_skel/ to build bpftool,
     and BPF skeletons. This keeps main perf Makefiles simple. But we may
     not like it for some reason?

Some known limitations (or work to be done):
  1. Only counting events for one BPF program at a time.
  2. Need extra logic in target__validate().

Song Liu (2):
  perf: support build BPF skeletons with perf
  perf-stat: enable counting events for BPF programs

 tools/build/Makefile.feature                  |   3 +-
 tools/perf/Makefile.config                    |   8 +
 tools/perf/Makefile.perf                      |  15 +-
 tools/perf/builtin-stat.c                     |  59 ++++-
 tools/perf/util/Build                         |   1 +
 tools/perf/util/bpf_counter.c                 | 215 ++++++++++++++++++
 tools/perf/util/bpf_counter.h                 |  71 ++++++
 tools/perf/util/bpf_skel/.gitignore           |   3 +
 tools/perf/util/bpf_skel/Makefile             |  71 ++++++
 .../util/bpf_skel/bpf_prog_profiler.bpf.c     |  96 ++++++++
 tools/perf/util/bpf_skel/dummy.bpf.c          |  19 ++
 tools/perf/util/evsel.c                       |  10 +
 tools/perf/util/evsel.h                       |   5 +
 tools/perf/util/target.h                      |   6 +
 14 files changed, 571 insertions(+), 11 deletions(-)
 create mode 100644 tools/perf/util/bpf_counter.c
 create mode 100644 tools/perf/util/bpf_counter.h
 create mode 100644 tools/perf/util/bpf_skel/.gitignore
 create mode 100644 tools/perf/util/bpf_skel/Makefile
 create mode 100644 tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c
 create mode 100644 tools/perf/util/bpf_skel/dummy.bpf.c

--
2.24.1

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-19  4:50 [RFC 0/2] Introduce perf-stat -b for BPF programs Song Liu
@ 2020-11-19  4:50 ` Song Liu
  2020-11-22 23:27   ` Jiri Olsa
                     ` (4 more replies)
  2020-11-19  4:50 ` [RFC 2/2] perf-stat: enable counting events for BPF programs Song Liu
  2020-11-19 14:52 ` [RFC 0/2] Introduce perf-stat -b " Arnaldo Carvalho de Melo
  2 siblings, 5 replies; 23+ messages in thread
From: Song Liu @ 2020-11-19  4:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, jolsa, namhyung, Song Liu

BPF programs are useful in perf to profile BPF programs. BPF skeleton is
by far the easiest way to write BPF tools. Enable building BPF skeletons
in util/bpf_skel. A dummy bpf skeleton is added. More bpf skeletons will
be added for different use cases.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/build/Makefile.feature         |  3 +-
 tools/perf/Makefile.config           |  7 +++
 tools/perf/Makefile.perf             | 15 +++++-
 tools/perf/util/bpf_skel/.gitignore  |  3 ++
 tools/perf/util/bpf_skel/Makefile    | 71 ++++++++++++++++++++++++++++
 tools/perf/util/bpf_skel/dummy.bpf.c | 19 ++++++++
 6 files changed, 115 insertions(+), 3 deletions(-)
 create mode 100644 tools/perf/util/bpf_skel/.gitignore
 create mode 100644 tools/perf/util/bpf_skel/Makefile
 create mode 100644 tools/perf/util/bpf_skel/dummy.bpf.c

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 97cbfb31b7625..95a58b5564218 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -70,7 +70,8 @@ FEATURE_TESTS_BASIC :=                  \
         libaio				\
         libzstd				\
         disassembler-four-args		\
-        file-handle
+        file-handle			\
+        clang-bpf-co-re
 
 # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
 # of all feature tests
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index ce8516e4de34f..1c2c0f4badd85 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -619,6 +619,13 @@ ifndef NO_LIBBPF
     msg := $(warning BPF API too old. Please install recent kernel headers. BPF support in 'perf record' is disabled.)
     NO_LIBBPF := 1
   endif
+
+  ifndef NO_BPF_SKEL
+    ifeq ($(feature-clang-bpf-co-re), 1)
+      BUILD_BPF_SKEL := 1
+      $(call detected,CONFIG_PERF_BPF_SKEL)
+    endif
+  endif
 endif
 
 dwarf-post-unwind := 1
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 7ce3f2e8b9c74..9a9fc71e2ffa4 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -126,6 +126,8 @@ include ../scripts/utilities.mak
 #
 # Define NO_LIBDEBUGINFOD if you do not want support debuginfod
 #
+# Define NO_BPF_SKEL if you do not want build BPF skeletons
+#
 
 # As per kernel Makefile, avoid funny character set dependencies
 unexport LC_ALL
@@ -735,7 +737,8 @@ prepare: $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h archheaders $(drm_ioc
 	$(x86_arch_prctl_code_array) \
 	$(rename_flags_array) \
 	$(arch_errno_name_array) \
-	$(sync_file_range_arrays)
+	$(sync_file_range_arrays) \
+	bpf-skel
 
 $(OUTPUT)%.o: %.c prepare FORCE
 	$(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=$(build-dir) $@
@@ -1008,7 +1011,15 @@ config-clean:
 python-clean:
 	$(python-clean)
 
+ifdef BUILD_BPF_SKEL
+bpf-skel:
+	$(Q)$(MAKE) -C util/bpf_skel
+else
+bpf-skel:
+endif
+
 clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBPERF)-clean config-clean fixdep-clean python-clean
+	$(call descend,util/bpf_skel,clean)
 	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-with-kcore $(LANG_BINDINGS)
 	$(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
 	$(Q)$(RM) $(OUTPUT).config-detected
@@ -1071,7 +1082,7 @@ endif
 
 FORCE:
 
-.PHONY: all install clean config-clean strip install-gtk
+.PHONY: all install clean config-clean strip install-gtk bpf-skel
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
 .PHONY: $(GIT-HEAD-PHONY) TAGS tags cscope FORCE prepare
 .PHONY: libtraceevent_plugins archheaders
diff --git a/tools/perf/util/bpf_skel/.gitignore b/tools/perf/util/bpf_skel/.gitignore
new file mode 100644
index 0000000000000..5263e9e6c5d83
--- /dev/null
+++ b/tools/perf/util/bpf_skel/.gitignore
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+.tmp
+*.skel.h
\ No newline at end of file
diff --git a/tools/perf/util/bpf_skel/Makefile b/tools/perf/util/bpf_skel/Makefile
new file mode 100644
index 0000000000000..853bece088f4b
--- /dev/null
+++ b/tools/perf/util/bpf_skel/Makefile
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+SKEL_OUTPUT := $(abspath .)
+TMP_OUTPUT := $(abspath .tmp)
+CLANG ?= clang
+LLC ?= llc
+LLVM_STRIP ?= llvm-strip
+DEFAULT_BPFTOOL := $(TMP_OUTPUT)/sbin/bpftool
+BPFTOOL ?= $(DEFAULT_BPFTOOL)
+LIBBPF_SRC := $(abspath ../../../lib/bpf)
+BPFOBJ := $(TMP_OUTPUT)/libbpf.a
+BPF_INCLUDE := $(TMP_OUTPUT)
+INCLUDES := -I$(TMP_OUTPUT) -I$(BPF_INCLUDE) -I$(abspath ../../../lib)        \
+       -I$(abspath ../../../include/uapi)
+CFLAGS := -g -Wall
+
+# Try to detect best kernel BTF source
+KERNEL_REL := $(shell uname -r)
+VMLINUX_BTF_PATHS := /sys/kernel/btf/vmlinux /boot/vmlinux-$(KERNEL_REL)
+VMLINUX_BTF_PATH := $(or $(VMLINUX_BTF),$(firstword			       \
+					  $(wildcard $(VMLINUX_BTF_PATHS))))
+ifeq ($(V),1)
+Q =
+msg =
+else
+Q = @
+msg = @printf '  %-8s %s%s\n' "$(1)" "$(notdir $(2))" "$(if $(3), $(3))";
+MAKEFLAGS += --no-print-directory
+submake_extras := feature_display=0
+endif
+
+SKELETONS := $(SKEL_OUTPUT)/dummy.skel.h
+
+.DELETE_ON_ERROR:
+
+.PHONY: all clean
+all: $(SKELETONS)
+
+clean:
+	$(call msg,CLEAN,bpf_skel)
+	$(Q)rm -rf $(TMP_OUTPUT) $(SKELETONS)
+
+$(SKEL_OUTPUT)/%.skel.h: $(TMP_OUTPUT)/%.bpf.o | $(BPFTOOL)
+	$(call msg,GEN-SKEL,$@)
+	$(Q)$(BPFTOOL) gen skeleton $< > $@
+
+$(TMP_OUTPUT)/%.bpf.o: %.bpf.c $(TMP_OUTPUT)/vmlinux.h $(BPFOBJ) | $(TMP_OUTPUT)
+	$(call msg,BPF,$@)
+	$(Q)$(CLANG) -g -O2 -target bpf $(INCLUDES)			      \
+		 -c $(filter %.c,$^) -o $@ &&				      \
+	$(LLVM_STRIP) -g $@
+
+$(TMP_OUTPUT):
+	$(call msg,MKDIR,$@)
+	$(Q)mkdir -p $(TMP_OUTPUT)
+
+$(TMP_OUTPUT)/vmlinux.h: $(VMLINUX_BTF_PATH) | $(TMP_OUTPUT) $(BPFTOOL)
+	$(call msg,GEN,$@)
+	$(Q)if [ ! -e "$(VMLINUX_BTF_PATH)" ] ; then \
+		echo "Couldn't find kernel BTF; set VMLINUX_BTF to"	       \
+			"specify its location." >&2;			       \
+		exit 1;\
+	fi
+	$(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF_PATH) format c > $@
+
+$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(TMP_OUTPUT)
+	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC)			       \
+		    OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
+
+$(DEFAULT_BPFTOOL): | $(TMP_OUTPUT)
+	$(Q)$(MAKE) $(submake_extras) -C ../../../bpf/bpftool		      \
+		    prefix= OUTPUT=$(TMP_OUTPUT)/ DESTDIR=$(TMP_OUTPUT) install
diff --git a/tools/perf/util/bpf_skel/dummy.bpf.c b/tools/perf/util/bpf_skel/dummy.bpf.c
new file mode 100644
index 0000000000000..085fcee1f52cf
--- /dev/null
+++ b/tools/perf/util/bpf_skel/dummy.bpf.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+// Copyright (c) 2020 Facebook
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
+	__uint(key_size, sizeof(u32));
+	__uint(value_size, sizeof(u64));
+} counts SEC(".maps");
+
+SEC("fentry/dummy")
+int BPF_PROG(fentry_dummy)
+{
+	return 0;
+}
+
+char LICENSE[] SEC("license") = "Dual BSD/GPL";
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFC 2/2] perf-stat: enable counting events for BPF programs
  2020-11-19  4:50 [RFC 0/2] Introduce perf-stat -b for BPF programs Song Liu
  2020-11-19  4:50 ` [RFC 1/2] perf: support build BPF skeletons with perf Song Liu
@ 2020-11-19  4:50 ` Song Liu
  2020-11-23 23:47   ` Jiri Olsa
  2020-11-24 19:51   ` Jiri Olsa
  2020-11-19 14:52 ` [RFC 0/2] Introduce perf-stat -b " Arnaldo Carvalho de Melo
  2 siblings, 2 replies; 23+ messages in thread
From: Song Liu @ 2020-11-19  4:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, jolsa, namhyung, Song Liu

Introduce perf-stat -b option, which counts events for BPF programs, like:

[root@localhost ~]# ~/perf stat -e ref-cycles,cycles -b 254 -I 1000
     1.487903822            115,200      ref-cycles
     1.487903822             86,012      cycles
     2.489147029             80,560      ref-cycles
     2.489147029             73,784      cycles
     3.490341825             60,720      ref-cycles
     3.490341825             37,797      cycles
     4.491540887             37,120      ref-cycles
     4.491540887             31,963      cycles

The example above counts cycles and ref-cycles of BPF program of id 254.
This is similar to bpftool-prog-profile command, but more flexible.

perf-stat -b creates per-cpu perf_event and loads fentry/fexit BPF
programs (monitor-progs) to the target BPF program (target-prog). The
monitor-progs read perf_event before and after the target-prog, and
aggregate the difference in a BPF map. Then the user space reads data
from these maps.

A new struct bpf_counter is introduced to provide common interface that
uses BPF programs/maps to count perf events.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/perf/Makefile.config                    |   1 +
 tools/perf/builtin-stat.c                     |  59 ++++-
 tools/perf/util/Build                         |   1 +
 tools/perf/util/bpf_counter.c                 | 215 ++++++++++++++++++
 tools/perf/util/bpf_counter.h                 |  71 ++++++
 tools/perf/util/bpf_skel/Makefile             |   2 +-
 .../util/bpf_skel/bpf_prog_profiler.bpf.c     |  96 ++++++++
 tools/perf/util/evsel.c                       |  10 +
 tools/perf/util/evsel.h                       |   5 +
 tools/perf/util/target.h                      |   6 +
 10 files changed, 457 insertions(+), 9 deletions(-)
 create mode 100644 tools/perf/util/bpf_counter.c
 create mode 100644 tools/perf/util/bpf_counter.h
 create mode 100644 tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 1c2c0f4badd85..60bcaa5af4d4c 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -624,6 +624,7 @@ ifndef NO_LIBBPF
     ifeq ($(feature-clang-bpf-co-re), 1)
       BUILD_BPF_SKEL := 1
       $(call detected,CONFIG_PERF_BPF_SKEL)
+      CFLAGS += -DBUILD_BPF_SKEL
     endif
   endif
 endif
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f15b2f8aa14d8..d6df04cc24073 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -67,6 +67,7 @@
 #include "util/top.h"
 #include "util/affinity.h"
 #include "util/pfm.h"
+#include "util/bpf_counter.h"
 #include "asm/bug.h"
 
 #include <linux/time64.h>
@@ -409,12 +410,31 @@ static int read_affinity_counters(struct timespec *rs)
 	return 0;
 }
 
+static int read_bpf_map_counters(void)
+{
+	struct evsel *counter;
+	int err;
+
+	evlist__for_each_entry(evsel_list, counter) {
+		err = bpf_counter__read(counter);
+		if (err)
+			return err;
+	}
+	return 0;
+}
+
 static void read_counters(struct timespec *rs)
 {
 	struct evsel *counter;
+	int err;
 
-	if (!stat_config.stop_read_counter && (read_affinity_counters(rs) < 0))
-		return;
+	if (!stat_config.stop_read_counter) {
+		err = read_bpf_map_counters();
+		if (err == -EAGAIN)
+			err = read_affinity_counters(rs);
+		if (err < 0)
+			return;
+	}
 
 	evlist__for_each_entry(evsel_list, counter) {
 		if (counter->err)
@@ -496,11 +516,20 @@ static bool handle_interval(unsigned int interval, int *times)
 	return false;
 }
 
-static void enable_counters(void)
+static int enable_counters(void)
 {
+	struct evsel *evsel;
+	int err;
+
+	evlist__for_each_entry(evsel_list, evsel) {
+		err = bpf_counter__enable(evsel);
+		if (err)
+			return err;
+	}
+
 	if (stat_config.initial_delay < 0) {
 		pr_info(EVLIST_DISABLED_MSG);
-		return;
+		return 0;
 	}
 
 	if (stat_config.initial_delay > 0) {
@@ -518,6 +547,7 @@ static void enable_counters(void)
 		if (stat_config.initial_delay > 0)
 			pr_info(EVLIST_ENABLED_MSG);
 	}
+	return 0;
 }
 
 static void disable_counters(void)
@@ -720,7 +750,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
 	const bool forks = (argc > 0);
 	bool is_pipe = STAT_RECORD ? perf_stat.data.is_pipe : false;
 	struct affinity affinity;
-	int i, cpu;
+	int i, cpu, err;
 	bool second_pass = false;
 
 	if (forks) {
@@ -738,6 +768,11 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
 	if (affinity__setup(&affinity) < 0)
 		return -1;
 
+	evlist__for_each_entry(evsel_list, counter) {
+		if (bpf_counter__load(counter, &target))
+			return -1;
+	}
+
 	evlist__for_each_cpu (evsel_list, i, cpu) {
 		affinity__set(&affinity, cpu);
 
@@ -851,7 +886,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
 	}
 
 	if (STAT_RECORD) {
-		int err, fd = perf_data__fd(&perf_stat.data);
+		int fd = perf_data__fd(&perf_stat.data);
 
 		if (is_pipe) {
 			err = perf_header__write_pipe(perf_data__fd(&perf_stat.data));
@@ -877,7 +912,9 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
 
 	if (forks) {
 		perf_evlist__start_workload(evsel_list);
-		enable_counters();
+		err = enable_counters();
+		if (err)
+			return -1;
 
 		if (interval || timeout || evlist__ctlfd_initialized(evsel_list))
 			status = dispatch_events(forks, timeout, interval, &times);
@@ -896,7 +933,9 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
 		if (WIFSIGNALED(status))
 			psignal(WTERMSIG(status), argv[0]);
 	} else {
-		enable_counters();
+		err = enable_counters();
+		if (err)
+			return -1;
 		status = dispatch_events(forks, timeout, interval, &times);
 	}
 
@@ -1087,6 +1126,10 @@ static struct option stat_options[] = {
 		   "stat events on existing process id"),
 	OPT_STRING('t', "tid", &target.tid, "tid",
 		   "stat events on existing thread id"),
+#ifdef BUILD_BPF_SKEL
+	OPT_STRING('b', "bpf-prog", &target.bpf_prog_id, "bpf-prog-id",
+		   "stat events on existing bpf program id"),
+#endif
 	OPT_BOOLEAN('a', "all-cpus", &target.system_wide,
 		    "system-wide collection from all CPUs"),
 	OPT_BOOLEAN('g', "group", &group,
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index e2563d0154eb6..188521f343470 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -135,6 +135,7 @@ perf-y += clockid.o
 
 perf-$(CONFIG_LIBBPF) += bpf-loader.o
 perf-$(CONFIG_LIBBPF) += bpf_map.o
+perf-$(CONFIG_PERF_BPF_SKEL) += bpf_counter.o
 perf-$(CONFIG_BPF_PROLOGUE) += bpf-prologue.o
 perf-$(CONFIG_LIBELF) += symbol-elf.o
 perf-$(CONFIG_LIBELF) += probe-file.o
diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
new file mode 100644
index 0000000000000..25456a179af26
--- /dev/null
+++ b/tools/perf/util/bpf_counter.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/* Copyright (c) 2019 Facebook */
+
+#include <limits.h>
+#include <sys/time.h>
+#include <sys/resource.h>
+#include <linux/err.h>
+#include <bpf/bpf.h>
+#include <bpf/btf.h>
+#include <bpf/libbpf.h>
+
+#include "bpf_counter.h"
+#include "counts.h"
+#include "debug.h"
+#include "evsel.h"
+#include "target.h"
+
+#include "bpf_skel/bpf_prog_profiler.skel.h"
+
+static inline void *u64_to_ptr(__u64 ptr)
+{
+	return (void *)(unsigned long)ptr;
+}
+
+static char *bpf_target_prog_name(int tgt_fd)
+{
+	struct bpf_prog_info_linear *info_linear;
+	struct bpf_func_info *func_info;
+	const struct btf_type *t;
+	char *name = NULL;
+	struct btf *btf;
+
+	info_linear = bpf_program__get_prog_info_linear(
+		tgt_fd, 1UL << BPF_PROG_INFO_FUNC_INFO);
+	if (IS_ERR_OR_NULL(info_linear)) {
+		pr_debug2("failed to get info_linear for prog FD %d", tgt_fd);
+		return NULL;
+	}
+
+	if (info_linear->info.btf_id == 0 ||
+	    btf__get_from_id(info_linear->info.btf_id, &btf)) {
+		pr_debug2("prog FD %d doesn't have valid btf", tgt_fd);
+		goto out;
+	}
+
+	func_info = u64_to_ptr(info_linear->info.func_info);
+	t = btf__type_by_id(btf, func_info[0].type_id);
+	if (!t) {
+		pr_debug2("btf %d doesn't have type %d",
+		      info_linear->info.btf_id, func_info[0].type_id);
+		goto out;
+	}
+	name = strdup(btf__name_by_offset(btf, t->name_off));
+out:
+	free(info_linear);
+	return name;
+}
+
+static void set_max_rlimit(void)
+{
+	struct rlimit rinf = { RLIM_INFINITY, RLIM_INFINITY };
+
+	setrlimit(RLIMIT_MEMLOCK, &rinf);
+}
+
+static int bpf_program_profiler__load(struct evsel *evsel, struct target *target)
+{
+	struct bpf_prog_profiler_bpf *skel;
+	struct bpf_program *prog;
+	char *prog_name;
+	char *end_ptr;
+	u32 prog_id;
+	int prog_fd;
+	int err;
+
+	prog_id = strtoul(target->bpf_prog_id, &end_ptr, 10);
+	if (prog_id == 0 || prog_id == UINT_MAX || *end_ptr != '\0') {
+		pr_debug("Failed to parse bpf prog id %s\n", target->bpf_prog_id);
+		return -1;
+	}
+	prog_fd = bpf_prog_get_fd_by_id(prog_id);
+	if (prog_fd < 0) {
+		pr_debug("Failed to open fd for bpf prog %u\n", prog_id);
+		return -1;
+	}
+
+	skel = bpf_prog_profiler_bpf__open();
+	if (!skel) {
+		pr_debug("Failed to load bpf skeleton\n");
+		return -1;
+	}
+	skel->rodata->num_cpu = evsel__nr_cpus(evsel);
+
+	bpf_map__resize(skel->maps.events, evsel__nr_cpus(evsel));
+	bpf_map__resize(skel->maps.fentry_readings, 1);
+	bpf_map__resize(skel->maps.accum_readings, 1);
+
+	prog_name = bpf_target_prog_name(prog_fd);
+
+	bpf_object__for_each_program(prog, skel->obj) {
+		err = bpf_program__set_attach_target(prog, prog_fd, prog_name);
+		if (err)
+			pr_debug("bpf_program__set_attach_target failed\n");
+	}
+	set_max_rlimit();
+	err = bpf_prog_profiler_bpf__load(skel);
+	if (err)
+		pr_debug("bpf_prog_profiler_bpf__load failed\n");
+
+	evsel->bpf_counter.skel = skel;
+	return 0;
+}
+
+static int bpf_program_profiler__enable(struct evsel *evsel)
+{
+	struct bpf_prog_profiler_bpf *skel = evsel->bpf_counter.skel;
+
+	return bpf_prog_profiler_bpf__attach(skel);
+}
+
+static int bpf_program_profiler__read(struct evsel *evsel)
+{
+	struct bpf_prog_profiler_bpf *skel = evsel->bpf_counter.skel;
+	int num_cpu = evsel__nr_cpus(evsel);
+	struct bpf_perf_event_value values[num_cpu];
+	int reading_map_fd;
+	__u32 key = 0;
+	int err, cpu;
+
+	if (!skel)
+		return -EAGAIN;
+
+	reading_map_fd = bpf_map__fd(skel->maps.accum_readings);
+
+	err = bpf_map_lookup_elem(reading_map_fd, &key, values);
+	if (err) {
+		fprintf(stderr, "failed to read value\n");
+		return err;
+	}
+
+	for (cpu = 0; cpu < num_cpu; cpu++) {
+		perf_counts(evsel->counts, cpu, 0)->val = values[cpu].counter;
+		perf_counts(evsel->counts, cpu, 0)->ena = values[cpu].enabled;
+		perf_counts(evsel->counts, cpu, 0)->run = values[cpu].running;
+	}
+
+	return 0;
+}
+
+static int bpf_program_profiler__destroy(struct evsel *evsel)
+{
+	struct bpf_prog_profiler_bpf *skel = evsel->bpf_counter.skel;
+
+	bpf_prog_profiler_bpf__destroy(skel);
+	return 0;
+}
+
+static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu,
+					    int fd)
+{
+	struct bpf_prog_profiler_bpf *skel = evsel->bpf_counter.skel;
+
+	return bpf_map_update_elem(bpf_map__fd(skel->maps.events),
+				   &cpu, &fd, BPF_ANY);
+}
+
+struct bpf_counter_ops bpf_program_profiler_ops = {
+	.load       = bpf_program_profiler__load,
+	.enable	    = bpf_program_profiler__enable,
+	.read       = bpf_program_profiler__read,
+	.destroy    = bpf_program_profiler__destroy,
+	.install_pe = bpf_program_profiler__install_pe,
+};
+
+int bpf_counter__install_pe(struct evsel *evsel, int cpu, int fd)
+{
+	if (!evsel->bpf_counter.skel)
+		return 0;
+	return evsel->bpf_counter.ops->install_pe(evsel, cpu, fd);
+}
+
+int bpf_counter__load(struct evsel *evsel, struct target *target)
+{
+	if (target__has_bpf(target))
+		evsel->bpf_counter.ops = &bpf_program_profiler_ops;
+
+	if (evsel->bpf_counter.ops)
+		return evsel->bpf_counter.ops->load(evsel, target);
+	return 0;
+}
+
+int bpf_counter__enable(struct evsel *evsel)
+{
+	if (!evsel->bpf_counter.skel)
+		return 0;
+	return evsel->bpf_counter.ops->enable(evsel);
+}
+
+int bpf_counter__read(struct evsel *evsel)
+{
+	if (!evsel->bpf_counter.skel)
+		return -EAGAIN;
+	return evsel->bpf_counter.ops->read(evsel);
+}
+
+int bpf_counter__destroy(struct evsel *evsel)
+{
+	if (!evsel->bpf_counter.skel)
+		return 0;
+	evsel->bpf_counter.ops->destroy(evsel);
+	evsel->bpf_counter.skel = NULL;
+	evsel->bpf_counter.ops = NULL;
+	return 0;
+}
diff --git a/tools/perf/util/bpf_counter.h b/tools/perf/util/bpf_counter.h
new file mode 100644
index 0000000000000..9f8f9bd3ec6e2
--- /dev/null
+++ b/tools/perf/util/bpf_counter.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PERF_BPF_COUNTER_H
+#define __PERF_BPF_COUNTER_H 1
+
+struct evsel;
+struct target;
+struct bpf_counter;
+
+typedef int (*bpf_counter_evsel_op)(struct evsel *evsel);
+typedef int (*bpf_counter_evsel_target_op)(struct evsel *evsel,
+					   struct target *target);
+typedef int (*bpf_counter_evsel_install_pe_op)(struct evsel *evsel,
+					       int cpu,
+					       int fd);
+
+struct bpf_counter_ops {
+	bpf_counter_evsel_target_op load;
+	bpf_counter_evsel_op enable;
+	bpf_counter_evsel_op read;
+	bpf_counter_evsel_op destroy;
+	bpf_counter_evsel_install_pe_op install_pe;
+};
+
+struct bpf_counter {
+	void *skel;
+	struct bpf_counter_ops *ops;
+};
+
+#ifdef BUILD_BPF_SKEL
+
+int bpf_counter__load(struct evsel *evsel, struct target *target);
+int bpf_counter__enable(struct evsel *evsel);
+int bpf_counter__read(struct evsel *evsel);
+int bpf_counter__destroy(struct evsel *evsel);
+int bpf_counter__install_pe(struct evsel *evsel, int cpu, int fd);
+
+#else
+
+#include<linux/err.h>
+
+static inline int bpf_counter__load(struct evsel *evsel __maybe_unused,
+				    struct target *target __maybe_unused)
+{
+	return 0;
+}
+
+static inline int bpf_counter__enable(struct evsel *evsel __maybe_unused)
+{
+	return 0;
+}
+
+static inline int bpf_counter__read(struct evsel *evsel __maybe_unused)
+{
+	return -EAGAIN;
+}
+
+static inline int bpf_counter__destroy(struct evsel *evsel __maybe_unused)
+{
+	return 0;
+}
+
+static inline int bpf_counter__install_pe(struct evsel *evsel __maybe_unused,
+					  int cpu __maybe_unused,
+					  int fd __maybe_unused)
+{
+	return 0;
+}
+
+#endif /* BUILD_BPF_SKEL */
+
+#endif /* __PERF_BPF_COUNTER_H */
diff --git a/tools/perf/util/bpf_skel/Makefile b/tools/perf/util/bpf_skel/Makefile
index 853bece088f4b..9e942f280c713 100644
--- a/tools/perf/util/bpf_skel/Makefile
+++ b/tools/perf/util/bpf_skel/Makefile
@@ -28,7 +28,7 @@ MAKEFLAGS += --no-print-directory
 submake_extras := feature_display=0
 endif
 
-SKELETONS := $(SKEL_OUTPUT)/dummy.skel.h
+SKELETONS := $(SKEL_OUTPUT)/dummy.skel.h $(SKEL_OUTPUT)/bpf_prog_profiler.skel.h
 
 .DELETE_ON_ERROR:
 
diff --git a/tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c b/tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c
new file mode 100644
index 0000000000000..41714bc436656
--- /dev/null
+++ b/tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+// Copyright (c) 2020 Facebook
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+/* map of perf event fds, num_cpu * num_metric entries */
+struct {
+	__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
+	__uint(key_size, sizeof(u32));
+	__uint(value_size, sizeof(int));
+} events SEC(".maps");
+
+/* readings at fentry */
+struct {
+	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
+	__uint(key_size, sizeof(u32));
+	__uint(value_size, sizeof(struct bpf_perf_event_value));
+	__uint(max_entries, 1);
+} fentry_readings SEC(".maps");
+
+/* accumulated readings */
+struct {
+	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
+	__uint(key_size, sizeof(u32));
+	__uint(value_size, sizeof(struct bpf_perf_event_value));
+	__uint(max_entries, 1);
+} accum_readings SEC(".maps");
+
+const volatile __u32 num_cpu = 1;
+
+SEC("fentry/XXX")
+int BPF_PROG(fentry_XXX)
+{
+	u32 key = bpf_get_smp_processor_id();
+	struct bpf_perf_event_value reading;
+	struct bpf_perf_event_value *ptr;
+	u32 zero = 0;
+	long err;
+
+	/* look up before reading, to reduce error */
+	ptr = bpf_map_lookup_elem(&fentry_readings, &zero);
+	if (!ptr)
+		return 0;
+
+	err = bpf_perf_event_read_value(&events, key, &reading,
+					sizeof(reading));
+	if (err)
+		return 0;
+
+	*ptr = reading;
+	return 0;
+}
+
+static inline void
+fexit_update_maps(struct bpf_perf_event_value *after)
+{
+	struct bpf_perf_event_value *before, diff, *accum;
+	u32 zero = 0;
+
+	before = bpf_map_lookup_elem(&fentry_readings, &zero);
+	/* only account samples with a valid fentry_reading */
+	if (before && before->counter) {
+		struct bpf_perf_event_value *accum;
+
+		diff.counter = after->counter - before->counter;
+		diff.enabled = after->enabled - before->enabled;
+		diff.running = after->running - before->running;
+
+		accum = bpf_map_lookup_elem(&accum_readings, &zero);
+		if (accum) {
+			accum->counter += diff.counter;
+			accum->enabled += diff.enabled;
+			accum->running += diff.running;
+		}
+	}
+}
+
+SEC("fexit/XXX")
+int BPF_PROG(fexit_XXX)
+{
+	struct bpf_perf_event_value reading;
+	u32 cpu = bpf_get_smp_processor_id();
+	u32 one = 1, zero = 0;
+	int err;
+
+	/* read all events before updating the maps, to reduce error */
+	err = bpf_perf_event_read_value(&events, cpu, &reading, sizeof(reading));
+	if (err)
+		return 0;
+
+	fexit_update_maps(&reading);
+	return 0;
+}
+
+char LICENSE[] SEC("license") = "Dual BSD/GPL";
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 1cad6051d8b08..6376b8db58e09 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -25,6 +25,7 @@
 #include <stdlib.h>
 #include <perf/evsel.h>
 #include "asm/bug.h"
+#include "bpf_counter.h"
 #include "callchain.h"
 #include "cgroup.h"
 #include "counts.h"
@@ -51,6 +52,10 @@
 #include <internal/lib.h>
 
 #include <linux/ctype.h>
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+#include <bpf/btf.h>
+#include "rlimit.h"
 
 struct perf_missing_features perf_missing_features;
 
@@ -1365,6 +1370,7 @@ void evsel__exit(struct evsel *evsel)
 {
 	assert(list_empty(&evsel->core.node));
 	assert(evsel->evlist == NULL);
+	bpf_counter__destroy(evsel);
 	evsel__free_counts(evsel);
 	perf_evsel__free_fd(&evsel->core);
 	perf_evsel__free_id(&evsel->core);
@@ -1770,6 +1776,8 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 		evsel->core.attr.sample_id_all = 0;
 
 	display_attr(&evsel->core.attr);
+	if (evsel->bpf_counter.skel)
+		evsel->core.attr.inherit = 0;
 
 	for (cpu = start_cpu; cpu < end_cpu; cpu++) {
 
@@ -1788,6 +1796,8 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 
 			FD(evsel, cpu, thread) = fd;
 
+			bpf_counter__install_pe(evsel, cpu, fd);
+
 			if (unlikely(test_attr__enabled)) {
 				test_attr__open(&evsel->core.attr, pid, cpus->map[cpu],
 						fd, group_fd, flags);
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 79a860d8e3eef..3a44f7b25726c 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -10,6 +10,7 @@
 #include <internal/evsel.h>
 #include <perf/evsel.h>
 #include "symbol_conf.h"
+#include "bpf_counter.h"
 #include <internal/cpumap.h>
 
 struct bpf_object;
@@ -17,6 +18,8 @@ struct cgroup;
 struct perf_counts;
 struct perf_stat_evsel;
 union perf_event;
+struct bpf_counter;
+struct target;
 
 typedef int (evsel__sb_cb_t)(union perf_event *event, void *data);
 
@@ -127,6 +130,7 @@ struct evsel {
 	 * See also evsel__has_callchain().
 	 */
 	__u64			synth_sample_type;
+	struct bpf_counter	bpf_counter;
 };
 
 struct perf_missing_features {
@@ -423,4 +427,5 @@ static inline bool evsel__is_dummy_event(struct evsel *evsel)
 struct perf_env *evsel__env(struct evsel *evsel);
 
 int evsel__store_ids(struct evsel *evsel, struct evlist *evlist);
+
 #endif /* __PERF_EVSEL_H */
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index 6ef01a83b24e9..cdaa7510f918b 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -10,6 +10,7 @@ struct target {
 	const char   *tid;
 	const char   *cpu_list;
 	const char   *uid_str;
+	const char   *bpf_prog_id;
 	uid_t	     uid;
 	bool	     system_wide;
 	bool	     uses_mmap;
@@ -59,6 +60,11 @@ static inline bool target__has_cpu(struct target *target)
 	return target->system_wide || target->cpu_list;
 }
 
+static inline bool target__has_bpf(struct target *target)
+{
+	return target->bpf_prog_id;
+}
+
 static inline bool target__none(struct target *target)
 {
 	return !target__has_task(target) && !target__has_cpu(target);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [RFC 0/2] Introduce perf-stat -b for BPF programs
  2020-11-19  4:50 [RFC 0/2] Introduce perf-stat -b for BPF programs Song Liu
  2020-11-19  4:50 ` [RFC 1/2] perf: support build BPF skeletons with perf Song Liu
  2020-11-19  4:50 ` [RFC 2/2] perf-stat: enable counting events for BPF programs Song Liu
@ 2020-11-19 14:52 ` Arnaldo Carvalho de Melo
  2 siblings, 0 replies; 23+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-11-19 14:52 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-kernel, kernel-team, peterz, mingo, mark.rutland,
	alexander.shishkin, jolsa, namhyung

Em Wed, Nov 18, 2020 at 08:50:44PM -0800, Song Liu escreveu:
> This set introduces perf-stat -b option to count events for BPF programs.
> This is similar to bpftool-prog-profile. But perf-stat makes it much more
> flexible.
> 
> Sending as RFC because I would like comments on some key design choices:
>   1. We are using BPF skeletons here, which is by far the easiest way to
>      write and ship BPF programs. However, this requires bpftool, which
>      makes building perf slower.
>   2. A Makefile is added to tools/perf/util/bpf_skel/ to build bpftool,
>      and BPF skeletons. This keeps main perf Makefiles simple. But we may
>      not like it for some reason?

I'll review it in detail, but before that: thanks a lot for working on
this! Looks super cool from a first quick look. :-)

- Arnaldo
 
> Some known limitations (or work to be done):
>   1. Only counting events for one BPF program at a time.
>   2. Need extra logic in target__validate().

 
> Song Liu (2):
>   perf: support build BPF skeletons with perf
>   perf-stat: enable counting events for BPF programs
> 
>  tools/build/Makefile.feature                  |   3 +-
>  tools/perf/Makefile.config                    |   8 +
>  tools/perf/Makefile.perf                      |  15 +-
>  tools/perf/builtin-stat.c                     |  59 ++++-
>  tools/perf/util/Build                         |   1 +
>  tools/perf/util/bpf_counter.c                 | 215 ++++++++++++++++++
>  tools/perf/util/bpf_counter.h                 |  71 ++++++
>  tools/perf/util/bpf_skel/.gitignore           |   3 +
>  tools/perf/util/bpf_skel/Makefile             |  71 ++++++
>  .../util/bpf_skel/bpf_prog_profiler.bpf.c     |  96 ++++++++
>  tools/perf/util/bpf_skel/dummy.bpf.c          |  19 ++
>  tools/perf/util/evsel.c                       |  10 +
>  tools/perf/util/evsel.h                       |   5 +
>  tools/perf/util/target.h                      |   6 +
>  14 files changed, 571 insertions(+), 11 deletions(-)
>  create mode 100644 tools/perf/util/bpf_counter.c
>  create mode 100644 tools/perf/util/bpf_counter.h
>  create mode 100644 tools/perf/util/bpf_skel/.gitignore
>  create mode 100644 tools/perf/util/bpf_skel/Makefile
>  create mode 100644 tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c
>  create mode 100644 tools/perf/util/bpf_skel/dummy.bpf.c
> 
> --
> 2.24.1

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-19  4:50 ` [RFC 1/2] perf: support build BPF skeletons with perf Song Liu
@ 2020-11-22 23:27   ` Jiri Olsa
  2020-11-24 23:47     ` Song Liu
  2020-11-22 23:32   ` Jiri Olsa
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 23+ messages in thread
From: Jiri Olsa @ 2020-11-22 23:27 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-kernel, kernel-team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:

SNIP

>  # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
>  # of all feature tests
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index ce8516e4de34f..1c2c0f4badd85 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -619,6 +619,13 @@ ifndef NO_LIBBPF
>      msg := $(warning BPF API too old. Please install recent kernel headers. BPF support in 'perf record' is disabled.)
>      NO_LIBBPF := 1
>    endif
> +
> +  ifndef NO_BPF_SKEL
> +    ifeq ($(feature-clang-bpf-co-re), 1)
> +      BUILD_BPF_SKEL := 1
> +      $(call detected,CONFIG_PERF_BPF_SKEL)
> +    endif
> +  endif
>  endif
>  
>  dwarf-post-unwind := 1
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 7ce3f2e8b9c74..9a9fc71e2ffa4 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -126,6 +126,8 @@ include ../scripts/utilities.mak
>  #
>  # Define NO_LIBDEBUGINFOD if you do not want support debuginfod
>  #
> +# Define NO_BPF_SKEL if you do not want build BPF skeletons

we will need to make this the other way round and disable it
by default before we figure out al lthe dependencies,
my build's failing on:

	[jolsa@krava perf]$ make JOBS=1
	  BUILD:   Doing 'make -j1' parallel build
	  CC       /home/jolsa/kernel/linux-perf/tools/perf/util/bpf_skel/.tmp/prog.o
	  CLANG    /home/jolsa/kernel/linux-perf/tools/perf/util/bpf_skel/.tmp/pid_iter.bpf.o
	In file included from skeleton/pid_iter.bpf.c:4:
	In file included from /home/jolsa/kernel/linux-perf/tools/lib/bpf/bpf_helpers.h:11:
	/home/jolsa/kernel/linux-perf/tools/lib/bpf/bpf_helper_defs.h:3560:57: warning: declaration of 'struct bpf_redir_neigh' will not be visible outside of this function [-Wvisibility]
	static long (*bpf_redirect_neigh)(__u32 ifindex, struct bpf_redir_neigh *params, int plen, __u64 flags) = (void *) 152;
								^
	skeleton/pid_iter.bpf.c:35:48: error: no member named 'id' in 'struct bpf_link'
			return BPF_CORE_READ((struct bpf_link *)ent, id);
			       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
	/home/jolsa/kernel/linux-perf/tools/lib/bpf/bpf_core_read.h:339:18: note: expanded from macro 'BPF_CORE_READ'
			___type((src), a, ##__VA_ARGS__) __r;                       \
	...


jirka


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-19  4:50 ` [RFC 1/2] perf: support build BPF skeletons with perf Song Liu
  2020-11-22 23:27   ` Jiri Olsa
@ 2020-11-22 23:32   ` Jiri Olsa
  2020-11-24 23:51     ` Song Liu
  2020-11-22 23:35   ` Jiri Olsa
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 23+ messages in thread
From: Jiri Olsa @ 2020-11-22 23:32 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-kernel, kernel-team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:

SNIP

> diff --git a/tools/perf/util/bpf_skel/.gitignore b/tools/perf/util/bpf_skel/.gitignore
> new file mode 100644
> index 0000000000000..5263e9e6c5d83
> --- /dev/null
> +++ b/tools/perf/util/bpf_skel/.gitignore
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +.tmp
> +*.skel.h
> \ No newline at end of file
> diff --git a/tools/perf/util/bpf_skel/Makefile b/tools/perf/util/bpf_skel/Makefile
> new file mode 100644
> index 0000000000000..853bece088f4b
> --- /dev/null
> +++ b/tools/perf/util/bpf_skel/Makefile
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> +SKEL_OUTPUT := $(abspath .)
> +TMP_OUTPUT := $(abspath .tmp)
> +CLANG ?= clang
> +LLC ?= llc
> +LLVM_STRIP ?= llvm-strip
> +DEFAULT_BPFTOOL := $(TMP_OUTPUT)/sbin/bpftool
> +BPFTOOL ?= $(DEFAULT_BPFTOOL)
> +LIBBPF_SRC := $(abspath ../../../lib/bpf)
> +BPFOBJ := $(TMP_OUTPUT)/libbpf.a
> +BPF_INCLUDE := $(TMP_OUTPUT)
> +INCLUDES := -I$(TMP_OUTPUT) -I$(BPF_INCLUDE) -I$(abspath ../../../lib)        \
> +       -I$(abspath ../../../include/uapi)
> +CFLAGS := -g -Wall
> +
> +# Try to detect best kernel BTF source
> +KERNEL_REL := $(shell uname -r)
> +VMLINUX_BTF_PATHS := /sys/kernel/btf/vmlinux /boot/vmlinux-$(KERNEL_REL)
> +VMLINUX_BTF_PATH := $(or $(VMLINUX_BTF),$(firstword			       \
> +					  $(wildcard $(VMLINUX_BTF_PATHS))))
> +ifeq ($(V),1)
> +Q =
> +msg =
> +else
> +Q = @
> +msg = @printf '  %-8s %s%s\n' "$(1)" "$(notdir $(2))" "$(if $(3), $(3))";
> +MAKEFLAGS += --no-print-directory
> +submake_extras := feature_display=0
> +endif

some of the above should already be defined in the base Makefile.perf
we should use that, I think it'd fit better in the Makefile.perf itself

jirka


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-19  4:50 ` [RFC 1/2] perf: support build BPF skeletons with perf Song Liu
  2020-11-22 23:27   ` Jiri Olsa
  2020-11-22 23:32   ` Jiri Olsa
@ 2020-11-22 23:35   ` Jiri Olsa
  2020-11-24 23:52     ` Song Liu
  2020-11-24 19:51   ` Jiri Olsa
  2020-11-24 19:51   ` Jiri Olsa
  4 siblings, 1 reply; 23+ messages in thread
From: Jiri Olsa @ 2020-11-22 23:35 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-kernel, kernel-team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:
> BPF programs are useful in perf to profile BPF programs. BPF skeleton is
> by far the easiest way to write BPF tools. Enable building BPF skeletons
> in util/bpf_skel. A dummy bpf skeleton is added. More bpf skeletons will
> be added for different use cases.

I was just in a place adding bpf program to perf as well,
so this will save me some time ;-) thanks!

jirka

> 
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
>  tools/build/Makefile.feature         |  3 +-
>  tools/perf/Makefile.config           |  7 +++
>  tools/perf/Makefile.perf             | 15 +++++-
>  tools/perf/util/bpf_skel/.gitignore  |  3 ++
>  tools/perf/util/bpf_skel/Makefile    | 71 ++++++++++++++++++++++++++++
>  tools/perf/util/bpf_skel/dummy.bpf.c | 19 ++++++++
>  6 files changed, 115 insertions(+), 3 deletions(-)
>  create mode 100644 tools/perf/util/bpf_skel/.gitignore
>  create mode 100644 tools/perf/util/bpf_skel/Makefile
>  create mode 100644 tools/perf/util/bpf_skel/dummy.bpf.c
> 
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 97cbfb31b7625..95a58b5564218 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -70,7 +70,8 @@ FEATURE_TESTS_BASIC :=                  \
>          libaio				\
>          libzstd				\
>          disassembler-four-args		\
> -        file-handle
> +        file-handle			\
> +        clang-bpf-co-re
>  
>  # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
>  # of all feature tests
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index ce8516e4de34f..1c2c0f4badd85 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -619,6 +619,13 @@ ifndef NO_LIBBPF
>      msg := $(warning BPF API too old. Please install recent kernel headers. BPF support in 'perf record' is disabled.)
>      NO_LIBBPF := 1
>    endif
> +
> +  ifndef NO_BPF_SKEL
> +    ifeq ($(feature-clang-bpf-co-re), 1)
> +      BUILD_BPF_SKEL := 1
> +      $(call detected,CONFIG_PERF_BPF_SKEL)
> +    endif
> +  endif
>  endif
>  
>  dwarf-post-unwind := 1
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 7ce3f2e8b9c74..9a9fc71e2ffa4 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -126,6 +126,8 @@ include ../scripts/utilities.mak
>  #
>  # Define NO_LIBDEBUGINFOD if you do not want support debuginfod
>  #
> +# Define NO_BPF_SKEL if you do not want build BPF skeletons
> +#
>  
>  # As per kernel Makefile, avoid funny character set dependencies
>  unexport LC_ALL
> @@ -735,7 +737,8 @@ prepare: $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h archheaders $(drm_ioc
>  	$(x86_arch_prctl_code_array) \
>  	$(rename_flags_array) \
>  	$(arch_errno_name_array) \
> -	$(sync_file_range_arrays)
> +	$(sync_file_range_arrays) \
> +	bpf-skel
>  
>  $(OUTPUT)%.o: %.c prepare FORCE
>  	$(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=$(build-dir) $@
> @@ -1008,7 +1011,15 @@ config-clean:
>  python-clean:
>  	$(python-clean)
>  
> +ifdef BUILD_BPF_SKEL
> +bpf-skel:
> +	$(Q)$(MAKE) -C util/bpf_skel
> +else
> +bpf-skel:
> +endif
> +
>  clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBPERF)-clean config-clean fixdep-clean python-clean
> +	$(call descend,util/bpf_skel,clean)
>  	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-with-kcore $(LANG_BINDINGS)
>  	$(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
>  	$(Q)$(RM) $(OUTPUT).config-detected
> @@ -1071,7 +1082,7 @@ endif
>  
>  FORCE:
>  
> -.PHONY: all install clean config-clean strip install-gtk
> +.PHONY: all install clean config-clean strip install-gtk bpf-skel
>  .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
>  .PHONY: $(GIT-HEAD-PHONY) TAGS tags cscope FORCE prepare
>  .PHONY: libtraceevent_plugins archheaders
> diff --git a/tools/perf/util/bpf_skel/.gitignore b/tools/perf/util/bpf_skel/.gitignore
> new file mode 100644
> index 0000000000000..5263e9e6c5d83
> --- /dev/null
> +++ b/tools/perf/util/bpf_skel/.gitignore
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +.tmp
> +*.skel.h
> \ No newline at end of file
> diff --git a/tools/perf/util/bpf_skel/Makefile b/tools/perf/util/bpf_skel/Makefile
> new file mode 100644
> index 0000000000000..853bece088f4b
> --- /dev/null
> +++ b/tools/perf/util/bpf_skel/Makefile
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> +SKEL_OUTPUT := $(abspath .)
> +TMP_OUTPUT := $(abspath .tmp)
> +CLANG ?= clang
> +LLC ?= llc
> +LLVM_STRIP ?= llvm-strip
> +DEFAULT_BPFTOOL := $(TMP_OUTPUT)/sbin/bpftool
> +BPFTOOL ?= $(DEFAULT_BPFTOOL)
> +LIBBPF_SRC := $(abspath ../../../lib/bpf)
> +BPFOBJ := $(TMP_OUTPUT)/libbpf.a
> +BPF_INCLUDE := $(TMP_OUTPUT)
> +INCLUDES := -I$(TMP_OUTPUT) -I$(BPF_INCLUDE) -I$(abspath ../../../lib)        \
> +       -I$(abspath ../../../include/uapi)
> +CFLAGS := -g -Wall
> +
> +# Try to detect best kernel BTF source
> +KERNEL_REL := $(shell uname -r)
> +VMLINUX_BTF_PATHS := /sys/kernel/btf/vmlinux /boot/vmlinux-$(KERNEL_REL)
> +VMLINUX_BTF_PATH := $(or $(VMLINUX_BTF),$(firstword			       \
> +					  $(wildcard $(VMLINUX_BTF_PATHS))))
> +ifeq ($(V),1)
> +Q =
> +msg =
> +else
> +Q = @
> +msg = @printf '  %-8s %s%s\n' "$(1)" "$(notdir $(2))" "$(if $(3), $(3))";
> +MAKEFLAGS += --no-print-directory
> +submake_extras := feature_display=0
> +endif
> +
> +SKELETONS := $(SKEL_OUTPUT)/dummy.skel.h
> +
> +.DELETE_ON_ERROR:
> +
> +.PHONY: all clean
> +all: $(SKELETONS)
> +
> +clean:
> +	$(call msg,CLEAN,bpf_skel)
> +	$(Q)rm -rf $(TMP_OUTPUT) $(SKELETONS)
> +
> +$(SKEL_OUTPUT)/%.skel.h: $(TMP_OUTPUT)/%.bpf.o | $(BPFTOOL)
> +	$(call msg,GEN-SKEL,$@)
> +	$(Q)$(BPFTOOL) gen skeleton $< > $@
> +
> +$(TMP_OUTPUT)/%.bpf.o: %.bpf.c $(TMP_OUTPUT)/vmlinux.h $(BPFOBJ) | $(TMP_OUTPUT)
> +	$(call msg,BPF,$@)
> +	$(Q)$(CLANG) -g -O2 -target bpf $(INCLUDES)			      \
> +		 -c $(filter %.c,$^) -o $@ &&				      \
> +	$(LLVM_STRIP) -g $@
> +
> +$(TMP_OUTPUT):
> +	$(call msg,MKDIR,$@)
> +	$(Q)mkdir -p $(TMP_OUTPUT)
> +
> +$(TMP_OUTPUT)/vmlinux.h: $(VMLINUX_BTF_PATH) | $(TMP_OUTPUT) $(BPFTOOL)
> +	$(call msg,GEN,$@)
> +	$(Q)if [ ! -e "$(VMLINUX_BTF_PATH)" ] ; then \
> +		echo "Couldn't find kernel BTF; set VMLINUX_BTF to"	       \
> +			"specify its location." >&2;			       \
> +		exit 1;\
> +	fi
> +	$(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF_PATH) format c > $@
> +
> +$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(TMP_OUTPUT)
> +	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC)			       \
> +		    OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
> +
> +$(DEFAULT_BPFTOOL): | $(TMP_OUTPUT)
> +	$(Q)$(MAKE) $(submake_extras) -C ../../../bpf/bpftool		      \
> +		    prefix= OUTPUT=$(TMP_OUTPUT)/ DESTDIR=$(TMP_OUTPUT) install
> diff --git a/tools/perf/util/bpf_skel/dummy.bpf.c b/tools/perf/util/bpf_skel/dummy.bpf.c
> new file mode 100644
> index 0000000000000..085fcee1f52cf
> --- /dev/null
> +++ b/tools/perf/util/bpf_skel/dummy.bpf.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +// Copyright (c) 2020 Facebook
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
> +	__uint(key_size, sizeof(u32));
> +	__uint(value_size, sizeof(u64));
> +} counts SEC(".maps");
> +
> +SEC("fentry/dummy")
> +int BPF_PROG(fentry_dummy)
> +{
> +	return 0;
> +}
> +
> +char LICENSE[] SEC("license") = "Dual BSD/GPL";
> -- 
> 2.24.1
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 2/2] perf-stat: enable counting events for BPF programs
  2020-11-19  4:50 ` [RFC 2/2] perf-stat: enable counting events for BPF programs Song Liu
@ 2020-11-23 23:47   ` Jiri Olsa
  2020-11-24 23:31     ` Song Liu
  2020-11-24 19:51   ` Jiri Olsa
  1 sibling, 1 reply; 23+ messages in thread
From: Jiri Olsa @ 2020-11-23 23:47 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-kernel, kernel-team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

On Wed, Nov 18, 2020 at 08:50:46PM -0800, Song Liu wrote:
> Introduce perf-stat -b option, which counts events for BPF programs, like:
> 
> [root@localhost ~]# ~/perf stat -e ref-cycles,cycles -b 254 -I 1000
>      1.487903822            115,200      ref-cycles
>      1.487903822             86,012      cycles
>      2.489147029             80,560      ref-cycles
>      2.489147029             73,784      cycles
>      3.490341825             60,720      ref-cycles
>      3.490341825             37,797      cycles
>      4.491540887             37,120      ref-cycles
>      4.491540887             31,963      cycles
> 
> The example above counts cycles and ref-cycles of BPF program of id 254.
> This is similar to bpftool-prog-profile command, but more flexible.
> 
> perf-stat -b creates per-cpu perf_event and loads fentry/fexit BPF
> programs (monitor-progs) to the target BPF program (target-prog). The
> monitor-progs read perf_event before and after the target-prog, and
> aggregate the difference in a BPF map. Then the user space reads data
> from these maps.
> 
> A new struct bpf_counter is introduced to provide common interface that
> uses BPF programs/maps to count perf events.
> 
> Signed-off-by: Song Liu <songliubraving@fb.com>

I still need to review this deeply, but so far I'm getting this error:

	# ./perf stat -b 40
	libbpf: elf: skipping unrecognized data section(9) .eh_frame
	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
	libbpf: XXX is not found in vmlinux BTF
	libbpf: failed to load object 'bpf_prog_profiler_bpf'
	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
	libbpf: elf: skipping unrecognized data section(9) .eh_frame
	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
	libbpf: XXX is not found in vmlinux BTF
	libbpf: failed to load object 'bpf_prog_profiler_bpf'
	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
	libbpf: elf: skipping unrecognized data section(9) .eh_frame
	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
	libbpf: XXX is not found in vmlinux BTF
	libbpf: failed to load object 'bpf_prog_profiler_bpf'
	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
	libbpf: elf: skipping unrecognized data section(9) .eh_frame
	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
	libbpf: XXX is not found in vmlinux BTF
	libbpf: failed to load object 'bpf_prog_profiler_bpf'
	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
	libbpf: elf: skipping unrecognized data section(9) .eh_frame
	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
	libbpf: XXX is not found in vmlinux BTF
	libbpf: failed to load object 'bpf_prog_profiler_bpf'
	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
	libbpf: elf: skipping unrecognized data section(9) .eh_frame
	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
	libbpf: XXX is not found in vmlinux BTF
	libbpf: failed to load object 'bpf_prog_profiler_bpf'
	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
	libbpf: elf: skipping unrecognized data section(9) .eh_frame
	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
	libbpf: XXX is not found in vmlinux BTF
	libbpf: failed to load object 'bpf_prog_profiler_bpf'
	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
	libbpf: elf: skipping unrecognized data section(9) .eh_frame
	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
	libbpf: XXX is not found in vmlinux BTF
	libbpf: failed to load object 'bpf_prog_profiler_bpf'
	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
	libbpf: Can't get the 0th fd from program fentry_XXX: only -1 instances
	libbpf: prog 'fentry_XXX': can't attach before loaded
	libbpf: failed to auto-attach program 'fentry_XXX': -22

jirka


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 2/2] perf-stat: enable counting events for BPF programs
  2020-11-19  4:50 ` [RFC 2/2] perf-stat: enable counting events for BPF programs Song Liu
  2020-11-23 23:47   ` Jiri Olsa
@ 2020-11-24 19:51   ` Jiri Olsa
  2020-11-24 23:43     ` Song Liu
  1 sibling, 1 reply; 23+ messages in thread
From: Jiri Olsa @ 2020-11-24 19:51 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-kernel, kernel-team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

On Wed, Nov 18, 2020 at 08:50:46PM -0800, Song Liu wrote:

SNIP

> +static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu,
> +					    int fd)
> +{
> +	struct bpf_prog_profiler_bpf *skel = evsel->bpf_counter.skel;
> +
> +	return bpf_map_update_elem(bpf_map__fd(skel->maps.events),
> +				   &cpu, &fd, BPF_ANY);
> +}
> +
> +struct bpf_counter_ops bpf_program_profiler_ops = {
> +	.load       = bpf_program_profiler__load,
> +	.enable	    = bpf_program_profiler__enable,
> +	.read       = bpf_program_profiler__read,
> +	.destroy    = bpf_program_profiler__destroy,
> +	.install_pe = bpf_program_profiler__install_pe,
> +};

hum, what's the point of this ops? you plan some other ops?
we could just define stat callbacks right?

> +SEC("fentry/XXX")
> +int BPF_PROG(fentry_XXX)
> +{
> +	u32 key = bpf_get_smp_processor_id();
> +	struct bpf_perf_event_value reading;
> +	struct bpf_perf_event_value *ptr;
> +	u32 zero = 0;
> +	long err;
> +
> +	/* look up before reading, to reduce error */
> +	ptr = bpf_map_lookup_elem(&fentry_readings, &zero);
> +	if (!ptr)
> +		return 0;
> +
> +	err = bpf_perf_event_read_value(&events, key, &reading,
> +					sizeof(reading));
> +	if (err)
> +		return 0;
> +
> +	*ptr = reading;
> +	return 0;
> +}

so currently it's one extra bpf program for each event,
but it seems bad for multiple events stat.. could we
just have one program that would read and process all events?

thanks,
jirka


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-19  4:50 ` [RFC 1/2] perf: support build BPF skeletons with perf Song Liu
                     ` (2 preceding siblings ...)
  2020-11-22 23:35   ` Jiri Olsa
@ 2020-11-24 19:51   ` Jiri Olsa
  2020-11-24 23:59     ` Song Liu
  2020-11-24 19:51   ` Jiri Olsa
  4 siblings, 1 reply; 23+ messages in thread
From: Jiri Olsa @ 2020-11-24 19:51 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-kernel, kernel-team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:

SNIP

> +
> +$(TMP_OUTPUT):
> +	$(call msg,MKDIR,$@)
> +	$(Q)mkdir -p $(TMP_OUTPUT)
> +
> +$(TMP_OUTPUT)/vmlinux.h: $(VMLINUX_BTF_PATH) | $(TMP_OUTPUT) $(BPFTOOL)
> +	$(call msg,GEN,$@)
> +	$(Q)if [ ! -e "$(VMLINUX_BTF_PATH)" ] ; then \
> +		echo "Couldn't find kernel BTF; set VMLINUX_BTF to"	       \
> +			"specify its location." >&2;			       \
> +		exit 1;\
> +	fi
> +	$(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF_PATH) format c > $@
> +
> +$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(TMP_OUTPUT)
> +	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC)			       \
> +		    OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
> +
> +$(DEFAULT_BPFTOOL): | $(TMP_OUTPUT)
> +	$(Q)$(MAKE) $(submake_extras) -C ../../../bpf/bpftool		      \
> +		    prefix= OUTPUT=$(TMP_OUTPUT)/ DESTDIR=$(TMP_OUTPUT) install

could we build just the bootstrap version of bpftool?
should be enough for skeleton and vmlinux.h dump, no?

thanks,
jirka


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-19  4:50 ` [RFC 1/2] perf: support build BPF skeletons with perf Song Liu
                     ` (3 preceding siblings ...)
  2020-11-24 19:51   ` Jiri Olsa
@ 2020-11-24 19:51   ` Jiri Olsa
  2020-11-24 23:53     ` Song Liu
  4 siblings, 1 reply; 23+ messages in thread
From: Jiri Olsa @ 2020-11-24 19:51 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-kernel, kernel-team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:

SNIP

> +	$(Q)$(BPFTOOL) gen skeleton $< > $@
> +
> +$(TMP_OUTPUT)/%.bpf.o: %.bpf.c $(TMP_OUTPUT)/vmlinux.h $(BPFOBJ) | $(TMP_OUTPUT)
> +	$(call msg,BPF,$@)
> +	$(Q)$(CLANG) -g -O2 -target bpf $(INCLUDES)			      \
> +		 -c $(filter %.c,$^) -o $@ &&				      \
> +	$(LLVM_STRIP) -g $@
> +
> +$(TMP_OUTPUT):
> +	$(call msg,MKDIR,$@)
> +	$(Q)mkdir -p $(TMP_OUTPUT)
> +
> +$(TMP_OUTPUT)/vmlinux.h: $(VMLINUX_BTF_PATH) | $(TMP_OUTPUT) $(BPFTOOL)

please add support to specify VMLINUX_H as it is in selftests
or bpftool, we will need it in out building setup

thanks,
jirka

> +	$(call msg,GEN,$@)
> +	$(Q)if [ ! -e "$(VMLINUX_BTF_PATH)" ] ; then \
> +		echo "Couldn't find kernel BTF; set VMLINUX_BTF to"	       \
> +			"specify its location." >&2;			       \
> +		exit 1;\
> +	fi
> +	$(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF_PATH) format c > $@
> +
> +$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(TMP_OUTPUT)
> +	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC)			       \
> +		    OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
> +
> +$(DEFAULT_BPFTOOL): | $(TMP_OUTPUT)
> +	$(Q)$(MAKE) $(submake_extras) -C ../../../bpf/bpftool		      \
> +		    prefix= OUTPUT=$(TMP_OUTPUT)/ DESTDIR=$(TMP_OUTPUT) install
> diff --git a/tools/perf/util/bpf_skel/dummy.bpf.c b/tools/perf/util/bpf_skel/dummy.bpf.c
> new file mode 100644
> index 0000000000000..085fcee1f52cf
> --- /dev/null
> +++ b/tools/perf/util/bpf_skel/dummy.bpf.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +// Copyright (c) 2020 Facebook
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
> +	__uint(key_size, sizeof(u32));
> +	__uint(value_size, sizeof(u64));
> +} counts SEC(".maps");
> +
> +SEC("fentry/dummy")
> +int BPF_PROG(fentry_dummy)
> +{
> +	return 0;
> +}
> +
> +char LICENSE[] SEC("license") = "Dual BSD/GPL";
> -- 
> 2.24.1
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 2/2] perf-stat: enable counting events for BPF programs
  2020-11-23 23:47   ` Jiri Olsa
@ 2020-11-24 23:31     ` Song Liu
  2020-11-25 16:42       ` Jiri Olsa
  0 siblings, 1 reply; 23+ messages in thread
From: Song Liu @ 2020-11-24 23:31 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: open list, Kernel Team, Peter Ziljstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	namhyung



> On Nov 23, 2020, at 3:47 PM, Jiri Olsa <jolsa@redhat.com> wrote:

[...]

> 
> I still need to review this deeply, but so far I'm getting this error:
> 
> 	# ./perf stat -b 40
> 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> 	libbpf: XXX is not found in vmlinux BTF
> 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> 	libbpf: XXX is not found in vmlinux BTF
> 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> 	libbpf: XXX is not found in vmlinux BTF
> 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> 	libbpf: XXX is not found in vmlinux BTF
> 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> 	libbpf: XXX is not found in vmlinux BTF
> 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> 	libbpf: XXX is not found in vmlinux BTF
> 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> 	libbpf: XXX is not found in vmlinux BTF
> 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> 	libbpf: XXX is not found in vmlinux BTF
> 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> 	libbpf: Can't get the 0th fd from program fentry_XXX: only -1 instances
> 	libbpf: prog 'fentry_XXX': can't attach before loaded
> 	libbpf: failed to auto-attach program 'fentry_XXX': -22

I cannot reproduce this. Is 40 a valid BPF program ID? Could you please share 
more information about it with "bpftool prog show id 40"?

Thanks,
Song

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 2/2] perf-stat: enable counting events for BPF programs
  2020-11-24 19:51   ` Jiri Olsa
@ 2020-11-24 23:43     ` Song Liu
  2020-11-25  0:02       ` Song Liu
  0 siblings, 1 reply; 23+ messages in thread
From: Song Liu @ 2020-11-24 23:43 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Kernel Team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung



> On Nov 24, 2020, at 11:51 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Nov 18, 2020 at 08:50:46PM -0800, Song Liu wrote:
> 
> SNIP
> 
>> +static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu,
>> +					    int fd)
>> +{
>> +	struct bpf_prog_profiler_bpf *skel = evsel->bpf_counter.skel;
>> +
>> +	return bpf_map_update_elem(bpf_map__fd(skel->maps.events),
>> +				   &cpu, &fd, BPF_ANY);
>> +}
>> +
>> +struct bpf_counter_ops bpf_program_profiler_ops = {
>> +	.load       = bpf_program_profiler__load,
>> +	.enable	    = bpf_program_profiler__enable,
>> +	.read       = bpf_program_profiler__read,
>> +	.destroy    = bpf_program_profiler__destroy,
>> +	.install_pe = bpf_program_profiler__install_pe,
>> +};
> 
> hum, what's the point of this ops? you plan some other ops?
> we could just define stat callbacks right?

I do have other ideas using BPF program to aggregate perf event 
counts. This ops provides common interface for different BPF 
extensions to evsel. 

> 
>> +SEC("fentry/XXX")
>> +int BPF_PROG(fentry_XXX)
>> +{
>> +	u32 key = bpf_get_smp_processor_id();
>> +	struct bpf_perf_event_value reading;
>> +	struct bpf_perf_event_value *ptr;
>> +	u32 zero = 0;
>> +	long err;
>> +
>> +	/* look up before reading, to reduce error */
>> +	ptr = bpf_map_lookup_elem(&fentry_readings, &zero);
>> +	if (!ptr)
>> +		return 0;
>> +
>> +	err = bpf_perf_event_read_value(&events, key, &reading,
>> +					sizeof(reading));
>> +	if (err)
>> +		return 0;
>> +
>> +	*ptr = reading;
>> +	return 0;
>> +}
> 
> so currently it's one extra bpf program for each event,
> but it seems bad for multiple events stat.. could we
> just have one program that would read and process all events?

Multiple fentry programs should not be too expensive. Current design
extends evsel, so it is a cleaner implementation. We can evaluate the 
difference of these two designs by comparing this with 
"bpftool prog profile".

Thanks,
Song




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-22 23:27   ` Jiri Olsa
@ 2020-11-24 23:47     ` Song Liu
  2020-11-25 16:38       ` Jiri Olsa
  0 siblings, 1 reply; 23+ messages in thread
From: Song Liu @ 2020-11-24 23:47 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: open list, Kernel Team, Peter Ziljstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	namhyung



> On Nov 22, 2020, at 3:27 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:
> 
> SNIP
> 
>> # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
>> # of all feature tests
>> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
>> index ce8516e4de34f..1c2c0f4badd85 100644
>> --- a/tools/perf/Makefile.config
>> +++ b/tools/perf/Makefile.config
>> @@ -619,6 +619,13 @@ ifndef NO_LIBBPF
>>     msg := $(warning BPF API too old. Please install recent kernel headers. BPF support in 'perf record' is disabled.)
>>     NO_LIBBPF := 1
>>   endif
>> +
>> +  ifndef NO_BPF_SKEL
>> +    ifeq ($(feature-clang-bpf-co-re), 1)
>> +      BUILD_BPF_SKEL := 1
>> +      $(call detected,CONFIG_PERF_BPF_SKEL)
>> +    endif
>> +  endif
>> endif
>> 
>> dwarf-post-unwind := 1
>> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
>> index 7ce3f2e8b9c74..9a9fc71e2ffa4 100644
>> --- a/tools/perf/Makefile.perf
>> +++ b/tools/perf/Makefile.perf
>> @@ -126,6 +126,8 @@ include ../scripts/utilities.mak
>> #
>> # Define NO_LIBDEBUGINFOD if you do not want support debuginfod
>> #
>> +# Define NO_BPF_SKEL if you do not want build BPF skeletons
> 
> we will need to make this the other way round and disable it
> by default before we figure out al lthe dependencies,
> my build's failing on:
> 
> 	[jolsa@krava perf]$ make JOBS=1
> 	  BUILD:   Doing 'make -j1' parallel build
> 	  CC       /home/jolsa/kernel/linux-perf/tools/perf/util/bpf_skel/.tmp/prog.o
> 	  CLANG    /home/jolsa/kernel/linux-perf/tools/perf/util/bpf_skel/.tmp/pid_iter.bpf.o
> 	In file included from skeleton/pid_iter.bpf.c:4:
> 	In file included from /home/jolsa/kernel/linux-perf/tools/lib/bpf/bpf_helpers.h:11:
> 	/home/jolsa/kernel/linux-perf/tools/lib/bpf/bpf_helper_defs.h:3560:57: warning: declaration of 'struct bpf_redir_neigh' will not be visible outside of this function [-Wvisibility]
> 	static long (*bpf_redirect_neigh)(__u32 ifindex, struct bpf_redir_neigh *params, int plen, __u64 flags) = (void *) 152;
> 								^
> 	skeleton/pid_iter.bpf.c:35:48: error: no member named 'id' in 'struct bpf_link'
> 			return BPF_CORE_READ((struct bpf_link *)ent, id);
> 			       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
> 	/home/jolsa/kernel/linux-perf/tools/lib/bpf/bpf_core_read.h:339:18: note: expanded from macro 'BPF_CORE_READ'
> 			___type((src), a, ##__VA_ARGS__) __r;                       \
> 	...

I guess this was caused by older vmlinux.h. Could you please try build 
vmlinux first? 

Thanks,
Song

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-22 23:32   ` Jiri Olsa
@ 2020-11-24 23:51     ` Song Liu
  0 siblings, 0 replies; 23+ messages in thread
From: Song Liu @ 2020-11-24 23:51 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: open list, Kernel Team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung



> On Nov 22, 2020, at 3:32 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:
> 
> SNIP
> 
>> diff --git a/tools/perf/util/bpf_skel/.gitignore b/tools/perf/util/bpf_skel/.gitignore
>> new file mode 100644
>> index 0000000000000..5263e9e6c5d83
>> --- /dev/null
>> +++ b/tools/perf/util/bpf_skel/.gitignore
>> @@ -0,0 +1,3 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +.tmp
>> +*.skel.h
>> \ No newline at end of file
>> diff --git a/tools/perf/util/bpf_skel/Makefile b/tools/perf/util/bpf_skel/Makefile
>> new file mode 100644
>> index 0000000000000..853bece088f4b
>> --- /dev/null
>> +++ b/tools/perf/util/bpf_skel/Makefile
>> @@ -0,0 +1,71 @@
>> +# SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
>> +SKEL_OUTPUT := $(abspath .)
>> +TMP_OUTPUT := $(abspath .tmp)
>> +CLANG ?= clang
>> +LLC ?= llc
>> +LLVM_STRIP ?= llvm-strip
>> +DEFAULT_BPFTOOL := $(TMP_OUTPUT)/sbin/bpftool
>> +BPFTOOL ?= $(DEFAULT_BPFTOOL)
>> +LIBBPF_SRC := $(abspath ../../../lib/bpf)
>> +BPFOBJ := $(TMP_OUTPUT)/libbpf.a
>> +BPF_INCLUDE := $(TMP_OUTPUT)
>> +INCLUDES := -I$(TMP_OUTPUT) -I$(BPF_INCLUDE) -I$(abspath ../../../lib)        \
>> +       -I$(abspath ../../../include/uapi)
>> +CFLAGS := -g -Wall
>> +
>> +# Try to detect best kernel BTF source
>> +KERNEL_REL := $(shell uname -r)
>> +VMLINUX_BTF_PATHS := /sys/kernel/btf/vmlinux /boot/vmlinux-$(KERNEL_REL)
>> +VMLINUX_BTF_PATH := $(or $(VMLINUX_BTF),$(firstword			       \
>> +					  $(wildcard $(VMLINUX_BTF_PATHS))))
>> +ifeq ($(V),1)
>> +Q =
>> +msg =
>> +else
>> +Q = @
>> +msg = @printf '  %-8s %s%s\n' "$(1)" "$(notdir $(2))" "$(if $(3), $(3))";
>> +MAKEFLAGS += --no-print-directory
>> +submake_extras := feature_display=0
>> +endif
> 
> some of the above should already be defined in the base Makefile.perf
> we should use that, I think it'd fit better in the Makefile.perf itself

Let me try that. OTOH, I think Makefile.perf is a little too big (1k+ lines)..

Thanks,
Song

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-22 23:35   ` Jiri Olsa
@ 2020-11-24 23:52     ` Song Liu
  2020-11-25 16:40       ` Jiri Olsa
  0 siblings, 1 reply; 23+ messages in thread
From: Song Liu @ 2020-11-24 23:52 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: open list, Kernel Team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung



> On Nov 22, 2020, at 3:35 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:
>> BPF programs are useful in perf to profile BPF programs. BPF skeleton is
>> by far the easiest way to write BPF tools. Enable building BPF skeletons
>> in util/bpf_skel. A dummy bpf skeleton is added. More bpf skeletons will
>> be added for different use cases.
> 
> I was just in a place adding bpf program to perf as well,
> so this will save me some time ;-) thanks!

I'd love to learn about your plan. Maybe we have some similar ideas, 
and could collaborate on them. 

Thanks,
Song


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-24 19:51   ` Jiri Olsa
@ 2020-11-24 23:53     ` Song Liu
  0 siblings, 0 replies; 23+ messages in thread
From: Song Liu @ 2020-11-24 23:53 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: open list, Kernel Team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung



> On Nov 24, 2020, at 11:51 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:
> 
> SNIP
> 
>> +	$(Q)$(BPFTOOL) gen skeleton $< > $@
>> +
>> +$(TMP_OUTPUT)/%.bpf.o: %.bpf.c $(TMP_OUTPUT)/vmlinux.h $(BPFOBJ) | $(TMP_OUTPUT)
>> +	$(call msg,BPF,$@)
>> +	$(Q)$(CLANG) -g -O2 -target bpf $(INCLUDES)			      \
>> +		 -c $(filter %.c,$^) -o $@ &&				      \
>> +	$(LLVM_STRIP) -g $@
>> +
>> +$(TMP_OUTPUT):
>> +	$(call msg,MKDIR,$@)
>> +	$(Q)mkdir -p $(TMP_OUTPUT)
>> +
>> +$(TMP_OUTPUT)/vmlinux.h: $(VMLINUX_BTF_PATH) | $(TMP_OUTPUT) $(BPFTOOL)
> 
> please add support to specify VMLINUX_H as it is in selftests
> or bpftool, we will need it in out building setup
> 
> thanks,
> jirka

Let me try that. 

Thanks,
Song

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-24 19:51   ` Jiri Olsa
@ 2020-11-24 23:59     ` Song Liu
  0 siblings, 0 replies; 23+ messages in thread
From: Song Liu @ 2020-11-24 23:59 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: open list, Kernel Team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung



> On Nov 24, 2020, at 11:51 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:
> 
> SNIP
> 
>> +
>> +$(TMP_OUTPUT):
>> +	$(call msg,MKDIR,$@)
>> +	$(Q)mkdir -p $(TMP_OUTPUT)
>> +
>> +$(TMP_OUTPUT)/vmlinux.h: $(VMLINUX_BTF_PATH) | $(TMP_OUTPUT) $(BPFTOOL)
>> +	$(call msg,GEN,$@)
>> +	$(Q)if [ ! -e "$(VMLINUX_BTF_PATH)" ] ; then \
>> +		echo "Couldn't find kernel BTF; set VMLINUX_BTF to"	       \
>> +			"specify its location." >&2;			       \
>> +		exit 1;\
>> +	fi
>> +	$(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF_PATH) format c > $@
>> +
>> +$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(TMP_OUTPUT)
>> +	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC)			       \
>> +		    OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
>> +
>> +$(DEFAULT_BPFTOOL): | $(TMP_OUTPUT)
>> +	$(Q)$(MAKE) $(submake_extras) -C ../../../bpf/bpftool		      \
>> +		    prefix= OUTPUT=$(TMP_OUTPUT)/ DESTDIR=$(TMP_OUTPUT) install
> 
> could we build just the bootstrap version of bpftool?
> should be enough for skeleton and vmlinux.h dump, no?
> 

Yeah, that should work. I thought about it, but didn't try that hard. 

Song


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 2/2] perf-stat: enable counting events for BPF programs
  2020-11-24 23:43     ` Song Liu
@ 2020-11-25  0:02       ` Song Liu
  2020-11-25 16:43         ` Jiri Olsa
  0 siblings, 1 reply; 23+ messages in thread
From: Song Liu @ 2020-11-25  0:02 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Kernel Team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung



> On Nov 24, 2020, at 3:43 PM, Song Liu <songliubraving@fb.com> wrote:
> 
> 
> 
>> On Nov 24, 2020, at 11:51 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>> 
>> On Wed, Nov 18, 2020 at 08:50:46PM -0800, Song Liu wrote:
>> 
>> SNIP
>> 
>>> +static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu,
>>> +					    int fd)
>>> +{
>>> +	struct bpf_prog_profiler_bpf *skel = evsel->bpf_counter.skel;
>>> +
>>> +	return bpf_map_update_elem(bpf_map__fd(skel->maps.events),
>>> +				   &cpu, &fd, BPF_ANY);
>>> +}
>>> +
>>> +struct bpf_counter_ops bpf_program_profiler_ops = {
>>> +	.load       = bpf_program_profiler__load,
>>> +	.enable	    = bpf_program_profiler__enable,
>>> +	.read       = bpf_program_profiler__read,
>>> +	.destroy    = bpf_program_profiler__destroy,
>>> +	.install_pe = bpf_program_profiler__install_pe,
>>> +};
>> 
>> hum, what's the point of this ops? you plan some other ops?
>> we could just define stat callbacks right?

Which callbacks do you mean here? I would like to try that as
well. 

Thanks,
Song

> 
> I do have other ideas using BPF program to aggregate perf event 
> counts. This ops provides common interface for different BPF 
> extensions to evsel. 
> 
>> 
>>> +SEC("fentry/XXX")
>>> +int BPF_PROG(fentry_XXX)
>>> +{
>>> +	u32 key = bpf_get_smp_processor_id();
>>> +	struct bpf_perf_event_value reading;
>>> +	struct bpf_perf_event_value *ptr;
>>> +	u32 zero = 0;
>>> +	long err;
>>> +
>>> +	/* look up before reading, to reduce error */
>>> +	ptr = bpf_map_lookup_elem(&fentry_readings, &zero);
>>> +	if (!ptr)
>>> +		return 0;
>>> +
>>> +	err = bpf_perf_event_read_value(&events, key, &reading,
>>> +					sizeof(reading));
>>> +	if (err)
>>> +		return 0;
>>> +
>>> +	*ptr = reading;
>>> +	return 0;
>>> +}
>> 
>> so currently it's one extra bpf program for each event,
>> but it seems bad for multiple events stat.. could we
>> just have one program that would read and process all events?
> 
> Multiple fentry programs should not be too expensive. Current design
> extends evsel, so it is a cleaner implementation. We can evaluate the 
> difference of these two designs by comparing this with 
> "bpftool prog profile".
> 
> Thanks,
> Song


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-24 23:47     ` Song Liu
@ 2020-11-25 16:38       ` Jiri Olsa
  0 siblings, 0 replies; 23+ messages in thread
From: Jiri Olsa @ 2020-11-25 16:38 UTC (permalink / raw)
  To: Song Liu
  Cc: open list, Kernel Team, Peter Ziljstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	namhyung

On Tue, Nov 24, 2020 at 11:47:05PM +0000, Song Liu wrote:
> 
> 
> > On Nov 22, 2020, at 3:27 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> > 
> > On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:
> > 
> > SNIP
> > 
> >> # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
> >> # of all feature tests
> >> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> >> index ce8516e4de34f..1c2c0f4badd85 100644
> >> --- a/tools/perf/Makefile.config
> >> +++ b/tools/perf/Makefile.config
> >> @@ -619,6 +619,13 @@ ifndef NO_LIBBPF
> >>     msg := $(warning BPF API too old. Please install recent kernel headers. BPF support in 'perf record' is disabled.)
> >>     NO_LIBBPF := 1
> >>   endif
> >> +
> >> +  ifndef NO_BPF_SKEL
> >> +    ifeq ($(feature-clang-bpf-co-re), 1)
> >> +      BUILD_BPF_SKEL := 1
> >> +      $(call detected,CONFIG_PERF_BPF_SKEL)
> >> +    endif
> >> +  endif
> >> endif
> >> 
> >> dwarf-post-unwind := 1
> >> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> >> index 7ce3f2e8b9c74..9a9fc71e2ffa4 100644
> >> --- a/tools/perf/Makefile.perf
> >> +++ b/tools/perf/Makefile.perf
> >> @@ -126,6 +126,8 @@ include ../scripts/utilities.mak
> >> #
> >> # Define NO_LIBDEBUGINFOD if you do not want support debuginfod
> >> #
> >> +# Define NO_BPF_SKEL if you do not want build BPF skeletons
> > 
> > we will need to make this the other way round and disable it
> > by default before we figure out al lthe dependencies,
> > my build's failing on:
> > 
> > 	[jolsa@krava perf]$ make JOBS=1
> > 	  BUILD:   Doing 'make -j1' parallel build
> > 	  CC       /home/jolsa/kernel/linux-perf/tools/perf/util/bpf_skel/.tmp/prog.o
> > 	  CLANG    /home/jolsa/kernel/linux-perf/tools/perf/util/bpf_skel/.tmp/pid_iter.bpf.o
> > 	In file included from skeleton/pid_iter.bpf.c:4:
> > 	In file included from /home/jolsa/kernel/linux-perf/tools/lib/bpf/bpf_helpers.h:11:
> > 	/home/jolsa/kernel/linux-perf/tools/lib/bpf/bpf_helper_defs.h:3560:57: warning: declaration of 'struct bpf_redir_neigh' will not be visible outside of this function [-Wvisibility]
> > 	static long (*bpf_redirect_neigh)(__u32 ifindex, struct bpf_redir_neigh *params, int plen, __u64 flags) = (void *) 152;
> > 								^
> > 	skeleton/pid_iter.bpf.c:35:48: error: no member named 'id' in 'struct bpf_link'
> > 			return BPF_CORE_READ((struct bpf_link *)ent, id);
> > 			       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
> > 	/home/jolsa/kernel/linux-perf/tools/lib/bpf/bpf_core_read.h:339:18: note: expanded from macro 'BPF_CORE_READ'
> > 			___type((src), a, ##__VA_ARGS__) __r;                       \
> > 	...
> 
> I guess this was caused by older vmlinux.h. Could you please try build 
> vmlinux first? 

yes, that helped, but I think that's the one of the reasons
we need to make this to be built in conditionaly

thanks,
jirka


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 1/2] perf: support build BPF skeletons with perf
  2020-11-24 23:52     ` Song Liu
@ 2020-11-25 16:40       ` Jiri Olsa
  0 siblings, 0 replies; 23+ messages in thread
From: Jiri Olsa @ 2020-11-25 16:40 UTC (permalink / raw)
  To: Song Liu
  Cc: open list, Kernel Team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

On Tue, Nov 24, 2020 at 11:52:43PM +0000, Song Liu wrote:
> 
> 
> > On Nov 22, 2020, at 3:35 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> > 
> > On Wed, Nov 18, 2020 at 08:50:45PM -0800, Song Liu wrote:
> >> BPF programs are useful in perf to profile BPF programs. BPF skeleton is
> >> by far the easiest way to write BPF tools. Enable building BPF skeletons
> >> in util/bpf_skel. A dummy bpf skeleton is added. More bpf skeletons will
> >> be added for different use cases.
> > 
> > I was just in a place adding bpf program to perf as well,
> > so this will save me some time ;-) thanks!
> 
> I'd love to learn about your plan. Maybe we have some similar ideas, 
> and could collaborate on them. 

the plan was to use skeleton as you did, because I agree it's the best
way to include bpf program in perf

I'm now using your patch and adding my bpf program on top of that ;-)

thanks,
jirka


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 2/2] perf-stat: enable counting events for BPF programs
  2020-11-24 23:31     ` Song Liu
@ 2020-11-25 16:42       ` Jiri Olsa
  0 siblings, 0 replies; 23+ messages in thread
From: Jiri Olsa @ 2020-11-25 16:42 UTC (permalink / raw)
  To: Song Liu
  Cc: open list, Kernel Team, Peter Ziljstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	namhyung

On Tue, Nov 24, 2020 at 11:31:49PM +0000, Song Liu wrote:
> 
> 
> > On Nov 23, 2020, at 3:47 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> [...]
> 
> > 
> > I still need to review this deeply, but so far I'm getting this error:
> > 
> > 	# ./perf stat -b 40
> > 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> > 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> > 	libbpf: XXX is not found in vmlinux BTF
> > 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> > 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> > 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> > 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> > 	libbpf: XXX is not found in vmlinux BTF
> > 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> > 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> > 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> > 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> > 	libbpf: XXX is not found in vmlinux BTF
> > 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> > 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> > 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> > 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> > 	libbpf: XXX is not found in vmlinux BTF
> > 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> > 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> > 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> > 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> > 	libbpf: XXX is not found in vmlinux BTF
> > 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> > 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> > 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> > 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> > 	libbpf: XXX is not found in vmlinux BTF
> > 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> > 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> > 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> > 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> > 	libbpf: XXX is not found in vmlinux BTF
> > 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> > 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> > 	libbpf: elf: skipping unrecognized data section(9) .eh_frame
> > 	libbpf: elf: skipping relo section(15) .rel.eh_frame for section(9) .eh_frame
> > 	libbpf: XXX is not found in vmlinux BTF
> > 	libbpf: failed to load object 'bpf_prog_profiler_bpf'
> > 	libbpf: failed to load BPF skeleton 'bpf_prog_profiler_bpf': -2
> > 	libbpf: Can't get the 0th fd from program fentry_XXX: only -1 instances
> > 	libbpf: prog 'fentry_XXX': can't attach before loaded
> > 	libbpf: failed to auto-attach program 'fentry_XXX': -22
> 
> I cannot reproduce this. Is 40 a valid BPF program ID? Could you please share 
> more information about it with "bpftool prog show id 40"?

it was bpftrace kfunc program, I'll try again and get back with
more details if there's still the problem

jirka


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC 2/2] perf-stat: enable counting events for BPF programs
  2020-11-25  0:02       ` Song Liu
@ 2020-11-25 16:43         ` Jiri Olsa
  0 siblings, 0 replies; 23+ messages in thread
From: Jiri Olsa @ 2020-11-25 16:43 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-kernel, Kernel Team, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

On Wed, Nov 25, 2020 at 12:02:13AM +0000, Song Liu wrote:
> 
> 
> > On Nov 24, 2020, at 3:43 PM, Song Liu <songliubraving@fb.com> wrote:
> > 
> > 
> > 
> >> On Nov 24, 2020, at 11:51 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> >> 
> >> On Wed, Nov 18, 2020 at 08:50:46PM -0800, Song Liu wrote:
> >> 
> >> SNIP
> >> 
> >>> +static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu,
> >>> +					    int fd)
> >>> +{
> >>> +	struct bpf_prog_profiler_bpf *skel = evsel->bpf_counter.skel;
> >>> +
> >>> +	return bpf_map_update_elem(bpf_map__fd(skel->maps.events),
> >>> +				   &cpu, &fd, BPF_ANY);
> >>> +}
> >>> +
> >>> +struct bpf_counter_ops bpf_program_profiler_ops = {
> >>> +	.load       = bpf_program_profiler__load,
> >>> +	.enable	    = bpf_program_profiler__enable,
> >>> +	.read       = bpf_program_profiler__read,
> >>> +	.destroy    = bpf_program_profiler__destroy,
> >>> +	.install_pe = bpf_program_profiler__install_pe,
> >>> +};
> >> 
> >> hum, what's the point of this ops? you plan some other ops?
> >> we could just define stat callbacks right?
> 
> Which callbacks do you mean here? I would like to try that as
> well. 

I meant just to drop that ops struct and have load/enable/read..
functions called from stat code

jirka


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2020-11-25 16:44 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-19  4:50 [RFC 0/2] Introduce perf-stat -b for BPF programs Song Liu
2020-11-19  4:50 ` [RFC 1/2] perf: support build BPF skeletons with perf Song Liu
2020-11-22 23:27   ` Jiri Olsa
2020-11-24 23:47     ` Song Liu
2020-11-25 16:38       ` Jiri Olsa
2020-11-22 23:32   ` Jiri Olsa
2020-11-24 23:51     ` Song Liu
2020-11-22 23:35   ` Jiri Olsa
2020-11-24 23:52     ` Song Liu
2020-11-25 16:40       ` Jiri Olsa
2020-11-24 19:51   ` Jiri Olsa
2020-11-24 23:59     ` Song Liu
2020-11-24 19:51   ` Jiri Olsa
2020-11-24 23:53     ` Song Liu
2020-11-19  4:50 ` [RFC 2/2] perf-stat: enable counting events for BPF programs Song Liu
2020-11-23 23:47   ` Jiri Olsa
2020-11-24 23:31     ` Song Liu
2020-11-25 16:42       ` Jiri Olsa
2020-11-24 19:51   ` Jiri Olsa
2020-11-24 23:43     ` Song Liu
2020-11-25  0:02       ` Song Liu
2020-11-25 16:43         ` Jiri Olsa
2020-11-19 14:52 ` [RFC 0/2] Introduce perf-stat -b " Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).