bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
@ 2020-03-04 18:07 Song Liu
  2020-03-04 18:07 ` [PATCH v4 bpf-next 1/4] bpftool: introduce "prog profile" command Song Liu
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Song Liu @ 2020-03-04 18:07 UTC (permalink / raw)
  To: netdev, bpf
  Cc: quentin, kernel-team, ast, daniel, arnaldo.melo, jolsa, Song Liu

This set introduces bpftool prog profile command, which uses hardware
counters to profile BPF programs.

This command attaches fentry/fexit programs to a target program. These two
programs read hardware counters before and after the target program and
calculate the difference.

Changes v3 => v4:
1. Simplify err handling in profile_open_perf_events() (Quentin);
2. Remove redundant p_err() (Quentin);
3. Replace tab with space in bash-completion; (Quentin);
4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).

Changes v2 => v3:
1. Change order of arguments (Quentin), as:
     bpftool prog profile PROG [duration DURATION] METRICs
2. Add bash-completion for bpftool prog profile (Quentin);
3. Fix build of selftests (Yonghong);
4. Better handling of bpf_map_lookup_elem() returns (Yonghong);
5. Improve clean up logic of do_profile() (Yonghong);
6. Other smaller fixes/cleanups.

Changes RFC => v2:
1. Use new bpf_program__set_attach_target() API;
2. Update output format to be perf-stat like (Alexei);
3. Incorporate skeleton generation into Makefile;
4. Make DURATION optional and Allow Ctrl-C (Alexei);
5. Add calcated values "insn per cycle" and "LLC misses per million isns".

Song Liu (4):
  bpftool: introduce "prog profile" command
  bpftool: Documentation for bpftool prog profile
  bpftool: bash completion for "bpftool prog profile"
  bpftool: fix typo in bash-completion

 .../bpftool/Documentation/bpftool-prog.rst    |  19 +
 tools/bpf/bpftool/Makefile                    |  18 +
 tools/bpf/bpftool/bash-completion/bpftool     |  47 +-
 tools/bpf/bpftool/prog.c                      | 425 +++++++++++++++++-
 tools/bpf/bpftool/skeleton/profiler.bpf.c     | 171 +++++++
 tools/bpf/bpftool/skeleton/profiler.h         |  47 ++
 tools/scripts/Makefile.include                |   1 +
 7 files changed, 725 insertions(+), 3 deletions(-)
 create mode 100644 tools/bpf/bpftool/skeleton/profiler.bpf.c
 create mode 100644 tools/bpf/bpftool/skeleton/profiler.h

--
2.17.1

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

* [PATCH v4 bpf-next 1/4] bpftool: introduce "prog profile" command
  2020-03-04 18:07 [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile Song Liu
@ 2020-03-04 18:07 ` Song Liu
  2020-03-04 19:21   ` Jiri Olsa
  2020-03-04 20:38   ` Jiri Olsa
  2020-03-04 18:07 ` [PATCH v4 bpf-next 2/4] bpftool: Documentation for bpftool prog profile Song Liu
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 20+ messages in thread
From: Song Liu @ 2020-03-04 18:07 UTC (permalink / raw)
  To: netdev, bpf
  Cc: quentin, kernel-team, ast, daniel, arnaldo.melo, jolsa, Song Liu

With fentry/fexit programs, it is possible to profile BPF program with
hardware counters. Introduce bpftool "prog profile", which measures key
metrics of a BPF program.

bpftool prog profile command creates per-cpu perf events. Then it attaches
fentry/fexit programs to the target BPF program. The fentry program saves
perf event value to a map. The fexit program reads the perf event again,
and calculates the difference, which is the instructions/cycles used by
the target program.

Example input and output:

  ./bpftool prog profile id 337 duration 3 cycles instructions llc_misses

        4228 run_cnt
     3403698 cycles                                              (84.08%)
     3525294 instructions   #  1.04 insn per cycle               (84.05%)
          13 llc_misses     #  3.69 LLC misses per million isns  (83.50%)

This command measures cycles and instructions for BPF program with id
337 for 3 seconds. The program has triggered 4228 times. The rest of the
output is similar to perf-stat. In this example, the counters were only
counting ~84% of the time because of time multiplexing of perf counters.

Note that, this approach measures cycles and instructions in very small
increments. So the fentry/fexit programs introduce noticeable errors to
the measurement results.

The fentry/fexit programs are generated with BPF skeletons. Therefore, we
build bpftool twice. The first time _bpftool is built without skeletons.
Then, _bpftool is used to generate the skeletons. The second time, bpftool
is built with skeletons.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/bpf/bpftool/Makefile                |  18 +
 tools/bpf/bpftool/prog.c                  | 425 +++++++++++++++++++++-
 tools/bpf/bpftool/skeleton/profiler.bpf.c | 171 +++++++++
 tools/bpf/bpftool/skeleton/profiler.h     |  47 +++
 tools/scripts/Makefile.include            |   1 +
 5 files changed, 661 insertions(+), 1 deletion(-)
 create mode 100644 tools/bpf/bpftool/skeleton/profiler.bpf.c
 create mode 100644 tools/bpf/bpftool/skeleton/profiler.h

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index c4e810335810..20a90d8450f8 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -59,6 +59,7 @@ LIBS = $(LIBBPF) -lelf -lz
 
 INSTALL ?= install
 RM ?= rm -f
+CLANG ?= clang
 
 FEATURE_USER = .bpftool
 FEATURE_TESTS = libbfd disassembler-four-args reallocarray zlib
@@ -110,6 +111,22 @@ SRCS += $(BFD_SRCS)
 endif
 
 OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
+_OBJS = $(filter-out $(OUTPUT)prog.o,$(OBJS)) $(OUTPUT)_prog.o
+
+$(OUTPUT)_prog.o: prog.c
+	$(QUIET_CC)$(COMPILE.c) -MMD -DBPFTOOL_WITHOUT_SKELETONS -o $@ $<
+
+$(OUTPUT)_bpftool: $(_OBJS) $(LIBBPF)
+	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(_OBJS) $(LIBS)
+
+skeleton/profiler.bpf.o: skeleton/profiler.bpf.c
+	$(QUIET_CLANG)$(CLANG) -I$(srctree)/tools/lib -g -O2 -target bpf -c $< -o $@
+
+profiler.skel.h: $(OUTPUT)_bpftool skeleton/profiler.bpf.o
+	$(QUIET_GEN)$(OUTPUT)./_bpftool gen skeleton skeleton/profiler.bpf.o > $@
+
+$(OUTPUT)prog.o: prog.c profiler.skel.h
+	$(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
 
 $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
 	$(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
@@ -125,6 +142,7 @@ $(OUTPUT)%.o: %.c
 clean: $(LIBBPF)-clean
 	$(call QUIET_CLEAN, bpftool)
 	$(Q)$(RM) -- $(OUTPUT)bpftool $(OUTPUT)*.o $(OUTPUT)*.d
+	$(Q)$(RM) -- $(OUTPUT)_bpftool profiler.skel.h skeleton/profiler.bpf.o
 	$(Q)$(RM) -r -- $(OUTPUT)libbpf/
 	$(call QUIET_CLEAN, core-gen)
 	$(Q)$(RM) -- $(OUTPUT)FEATURE-DUMP.bpftool
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 1996e67a2f00..8e6cded71660 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -4,6 +4,7 @@
 #define _GNU_SOURCE
 #include <errno.h>
 #include <fcntl.h>
+#include <signal.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -11,10 +12,13 @@
 #include <time.h>
 #include <unistd.h>
 #include <net/if.h>
+#include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/syscall.h>
 
 #include <linux/err.h>
+#include <linux/perf_event.h>
 #include <linux/sizes.h>
 
 #include <bpf/bpf.h>
@@ -1537,6 +1541,422 @@ static int do_loadall(int argc, char **argv)
 	return load_with_options(argc, argv, false);
 }
 
+#ifdef BPFTOOL_WITHOUT_SKELETONS
+
+static int do_profile(int argc, char **argv)
+{
+	return 0;
+}
+
+#else /* BPFTOOL_WITHOUT_SKELETONS */
+
+#include "profiler.skel.h"
+
+#define SAMPLE_PERIOD  0x7fffffffffffffffULL
+struct profile_metric {
+	const char *name;
+	struct bpf_perf_event_value val;
+	struct perf_event_attr attr;
+	bool selected;
+
+	/* calculate ratios like instructions per cycle */
+	const int ratio_metric; /* 0 for N/A, 1 for index 0 (cycles) */
+	const char *ratio_desc;
+	const float ratio_mul;
+} metrics[] = {
+	{
+		.name = "cycles",
+		.attr = {
+			.sample_period = SAMPLE_PERIOD,
+			.type = PERF_TYPE_HARDWARE,
+			.config = PERF_COUNT_HW_CPU_CYCLES,
+		},
+	},
+	{
+		.name = "instructions",
+		.attr = {
+			.sample_period = SAMPLE_PERIOD,
+			.type = PERF_TYPE_HARDWARE,
+			.config = PERF_COUNT_HW_INSTRUCTIONS,
+		},
+		.ratio_metric = 1,
+		.ratio_desc = "insns per cycle",
+		.ratio_mul = 1.0,
+	},
+	{
+		.name = "l1d_loads",
+		.attr = {
+			.sample_period = SAMPLE_PERIOD,
+			.type = PERF_TYPE_HW_CACHE,
+			.config =
+				PERF_COUNT_HW_CACHE_L1D |
+				(PERF_COUNT_HW_CACHE_OP_READ << 8) |
+				(PERF_COUNT_HW_CACHE_RESULT_ACCESS << 16),
+		},
+	},
+	{
+		.name = "llc_misses",
+		.attr = {
+			.sample_period = SAMPLE_PERIOD,
+			.type = PERF_TYPE_HW_CACHE,
+			.config =
+				PERF_COUNT_HW_CACHE_LL |
+				(PERF_COUNT_HW_CACHE_OP_READ << 8) |
+				(PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
+		},
+		.ratio_metric = 2,
+		.ratio_desc = "LLC misses per million insns",
+		.ratio_mul = 1e6,
+	},
+};
+
+static __u64 profile_total_count;
+
+#define MAX_NUM_PROFILE_METRICS 4
+
+static int profile_parse_metrics(int argc, char **argv)
+{
+	unsigned int metric_cnt;
+	int selected_cnt = 0;
+	unsigned int i;
+
+	metric_cnt = sizeof(metrics) / sizeof(struct profile_metric);
+
+	while (argc > 0) {
+		for (i = 0; i < metric_cnt; i++) {
+			if (is_prefix(argv[0], metrics[i].name)) {
+				if (!metrics[i].selected)
+					selected_cnt++;
+				metrics[i].selected = true;
+				break;
+			}
+		}
+		if (i == metric_cnt) {
+			p_err("unknown metric %s", argv[0]);
+			return -1;
+		}
+		NEXT_ARG();
+	}
+	if (selected_cnt > MAX_NUM_PROFILE_METRICS) {
+		p_err("too many (%d) metrics, please specify no more than %d metrics at at time",
+		      selected_cnt, MAX_NUM_PROFILE_METRICS);
+		return -1;
+	}
+	return selected_cnt;
+}
+
+static void profile_read_values(struct profiler_bpf *obj)
+{
+	__u32 m, cpu, num_cpu = obj->rodata->num_cpu;
+	int reading_map_fd, count_map_fd;
+	__u64 counts[num_cpu];
+	__u32 key = 0;
+	int err;
+
+	reading_map_fd = bpf_map__fd(obj->maps.accum_readings);
+	count_map_fd = bpf_map__fd(obj->maps.counts);
+	if (reading_map_fd < 0 || count_map_fd < 0) {
+		p_err("failed to get fd for map");
+		return;
+	}
+
+	err = bpf_map_lookup_elem(count_map_fd, &key, counts);
+	if (err) {
+		p_err("failed to read count_map: %s", strerror(errno));
+		return;
+	}
+
+	profile_total_count = 0;
+	for (cpu = 0; cpu < num_cpu; cpu++)
+		profile_total_count += counts[cpu];
+
+	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
+		struct bpf_perf_event_value values[num_cpu];
+
+		if (!metrics[m].selected)
+			continue;
+
+		err = bpf_map_lookup_elem(reading_map_fd, &key, values);
+		if (err) {
+			p_err("failed to read reading_map: %s",
+			      strerror(errno));
+			return;
+		}
+		for (cpu = 0; cpu < num_cpu; cpu++) {
+			metrics[m].val.counter += values[cpu].counter;
+			metrics[m].val.enabled += values[cpu].enabled;
+			metrics[m].val.running += values[cpu].running;
+		}
+		key++;
+	}
+}
+
+static void profile_print_readings_json(void)
+{
+	__u32 m;
+
+	jsonw_start_array(json_wtr);
+	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
+		if (!metrics[m].selected)
+			continue;
+		jsonw_start_object(json_wtr);
+		jsonw_string_field(json_wtr, "metric", metrics[m].name);
+		jsonw_lluint_field(json_wtr, "run_cnt", profile_total_count);
+		jsonw_lluint_field(json_wtr, "value", metrics[m].val.counter);
+		jsonw_lluint_field(json_wtr, "enabled", metrics[m].val.enabled);
+		jsonw_lluint_field(json_wtr, "running", metrics[m].val.running);
+
+		jsonw_end_object(json_wtr);
+	}
+	jsonw_end_array(json_wtr);
+}
+
+static void profile_print_readings_plain(void)
+{
+	__u32 m;
+
+	printf("\n%18llu %-20s\n", profile_total_count, "run_cnt");
+	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
+		struct bpf_perf_event_value *val = &metrics[m].val;
+		int r;
+
+		if (!metrics[m].selected)
+			continue;
+		printf("%18llu %-20s", val->counter, metrics[m].name);
+
+		r = metrics[m].ratio_metric - 1;
+		if (r >= 0 && metrics[r].selected &&
+		    metrics[r].val.counter > 0) {
+			printf("# %8.2f %-30s",
+			       val->counter * metrics[m].ratio_mul /
+			       metrics[r].val.counter,
+			       metrics[m].ratio_desc);
+		} else {
+			printf("%-41s", "");
+		}
+
+		if (val->enabled > val->running)
+			printf("(%4.2f%%)",
+			       val->running * 100.0 / val->enabled);
+		printf("\n");
+	}
+}
+
+static void profile_print_readings(void)
+{
+	if (json_output)
+		profile_print_readings_json();
+	else
+		profile_print_readings_plain();
+}
+
+static char *profile_target_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)) {
+		p_err("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)) {
+		p_err("prog FD %d doesn't have valid btf", tgt_fd);
+		goto out;
+	}
+
+	func_info = (struct bpf_func_info *)(info_linear->info.func_info);
+	t = btf__type_by_id(btf, func_info[0].type_id);
+	if (!t) {
+		p_err("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 struct profiler_bpf *profile_obj;
+static int profile_tgt_fd = -1;
+static char *profile_tgt_name;
+static int *profile_perf_events;
+static int profile_perf_event_cnt;
+
+static void profile_close_perf_events(struct profiler_bpf *obj)
+{
+	int i;
+
+	for (i = profile_perf_event_cnt - 1; i >= 0; i--)
+		close(profile_perf_events[i]);
+
+	free(profile_perf_events);
+	profile_perf_event_cnt = 0;
+}
+
+static int profile_open_perf_events(struct profiler_bpf *obj)
+{
+	unsigned int cpu, m;
+	int map_fd, pmu_fd;
+
+	profile_perf_events = calloc(
+		sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
+	if (!profile_perf_events) {
+		p_err("failed to allocate memory for perf_event array: %s",
+		      strerror(errno));
+		return -1;
+	}
+	map_fd = bpf_map__fd(obj->maps.events);
+	if (map_fd < 0) {
+		p_err("failed to get fd for events map");
+		return -1;
+	}
+
+	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
+		if (!metrics[m].selected)
+			continue;
+		for (cpu = 0; cpu < obj->rodata->num_cpu; cpu++) {
+			pmu_fd = syscall(__NR_perf_event_open, &metrics[m].attr,
+					 -1/*pid*/, cpu, -1/*group_fd*/, 0);
+			if (pmu_fd < 0 ||
+			    bpf_map_update_elem(map_fd, &profile_perf_event_cnt,
+						&pmu_fd, BPF_ANY) ||
+			    ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0)) {
+				p_err("failed to create event %s on cpu %d",
+				      metrics[m].name, cpu);
+				return -1;
+			}
+			profile_perf_events[profile_perf_event_cnt++] = pmu_fd;
+		}
+	}
+	return 0;
+}
+
+static void profile_print_and_cleanup(void)
+{
+	profile_close_perf_events(profile_obj);
+	profile_read_values(profile_obj);
+	profile_print_readings();
+	profiler_bpf__destroy(profile_obj);
+
+	close(profile_tgt_fd);
+	free(profile_tgt_name);
+}
+
+static void int_exit(int signo)
+{
+	profile_print_and_cleanup();
+	exit(0);
+}
+
+static int do_profile(int argc, char **argv)
+{
+	int num_metric, num_cpu, err = -1;
+	struct bpf_program *prog;
+	unsigned long duration;
+	char *endptr;
+
+	/* we at least need two args for the prog and one metric */
+	if (!REQ_ARGS(3))
+		return -EINVAL;
+
+	/* parse target fd */
+	profile_tgt_fd = prog_parse_fd(&argc, &argv);
+	if (profile_tgt_fd < 0) {
+		p_err("failed to parse fd");
+		return -1;
+	}
+
+	/* parse profiling optional duration */
+	if (argc > 2 && is_prefix(argv[0], "duration")) {
+		NEXT_ARG();
+		duration = strtoul(*argv, &endptr, 0);
+		if (*endptr)
+			usage();
+		NEXT_ARG();
+	} else {
+		duration = UINT_MAX;
+	}
+
+	num_metric = profile_parse_metrics(argc, argv);
+	if (num_metric <= 0)
+		goto out;
+
+	num_cpu = libbpf_num_possible_cpus();
+	if (num_cpu <= 0) {
+		p_err("failed to identify number of CPUs");
+		goto out;
+	}
+
+	profile_obj = profiler_bpf__open();
+	if (!profile_obj) {
+		p_err("failed to open and/or load BPF object");
+		goto out;
+	}
+
+	profile_obj->rodata->num_cpu = num_cpu;
+	profile_obj->rodata->num_metric = num_metric;
+
+	/* adjust map sizes */
+	bpf_map__resize(profile_obj->maps.events, num_metric * num_cpu);
+	bpf_map__resize(profile_obj->maps.fentry_readings, num_metric);
+	bpf_map__resize(profile_obj->maps.accum_readings, num_metric);
+	bpf_map__resize(profile_obj->maps.counts, 1);
+
+	/* change target name */
+	profile_tgt_name = profile_target_name(profile_tgt_fd);
+	if (!profile_tgt_name)
+		goto out;
+
+	bpf_object__for_each_program(prog, profile_obj->obj) {
+		err = bpf_program__set_attach_target(prog, profile_tgt_fd,
+						     profile_tgt_name);
+		if (err) {
+			p_err("failed to set attach target\n");
+			goto out;
+		}
+	}
+
+	set_max_rlimit();
+	err = profiler_bpf__load(profile_obj);
+	if (err) {
+		p_err("failed to load profile_obj");
+		goto out;
+	}
+
+	err = profile_open_perf_events(profile_obj);
+	if (err)
+		goto out;
+
+	err = profiler_bpf__attach(profile_obj);
+	if (err) {
+		p_err("failed to attach profile_obj");
+		goto out;
+	}
+	signal(SIGINT, int_exit);
+
+	sleep(duration);
+	profile_print_and_cleanup();
+	return 0;
+
+out:
+	profile_close_perf_events(profile_obj);
+	if (profile_obj)
+		profiler_bpf__destroy(profile_obj);
+	close(profile_tgt_fd);
+	free(profile_tgt_name);
+	return err;
+}
+
+#endif /* BPFTOOL_WITHOUT_SKELETONS */
+
 static int do_help(int argc, char **argv)
 {
 	if (json_output) {
@@ -1560,6 +1980,7 @@ static int do_help(int argc, char **argv)
 		"                         [data_out FILE [data_size_out L]] \\\n"
 		"                         [ctx_in FILE [ctx_out FILE [ctx_size_out M]]] \\\n"
 		"                         [repeat N]\n"
+		"       %s %s profile PROG [duration DURATION] METRICs\n"
 		"       %s %s tracelog\n"
 		"       %s %s help\n"
 		"\n"
@@ -1577,12 +1998,13 @@ static int do_help(int argc, char **argv)
 		"                 struct_ops | fentry | fexit | freplace }\n"
 		"       ATTACH_TYPE := { msg_verdict | stream_verdict | stream_parser |\n"
 		"                        flow_dissector }\n"
+		"       METRIC := { cycles | instructions | l1d_loads | llc_misses }\n"
 		"       " HELP_SPEC_OPTIONS "\n"
 		"",
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
-		bin_name, argv[-2]);
+		bin_name, argv[-2], bin_name, argv[-2]);
 
 	return 0;
 }
@@ -1599,6 +2021,7 @@ static const struct cmd cmds[] = {
 	{ "detach",	do_detach },
 	{ "tracelog",	do_tracelog },
 	{ "run",	do_run },
+	{ "profile",	do_profile },
 	{ 0 }
 };
 
diff --git a/tools/bpf/bpftool/skeleton/profiler.bpf.c b/tools/bpf/bpftool/skeleton/profiler.bpf.c
new file mode 100644
index 000000000000..20594ccb393d
--- /dev/null
+++ b/tools/bpf/bpftool/skeleton/profiler.bpf.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2020 Facebook
+#include "profiler.h"
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+#define ___bpf_concat(a, b) a ## b
+#define ___bpf_apply(fn, n) ___bpf_concat(fn, n)
+#define ___bpf_nth(_, _1, _2, _3, _4, _5, _6, _7, _8, _9, _a, _b, _c, N, ...) N
+#define ___bpf_narg(...) \
+	___bpf_nth(_, ##__VA_ARGS__, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
+#define ___bpf_empty(...) \
+	___bpf_nth(_, ##__VA_ARGS__, N, N, N, N, N, N, N, N, N, N, 0)
+
+#define ___bpf_ctx_cast0() ctx
+#define ___bpf_ctx_cast1(x) ___bpf_ctx_cast0(), (void *)ctx[0]
+#define ___bpf_ctx_cast2(x, args...) ___bpf_ctx_cast1(args), (void *)ctx[1]
+#define ___bpf_ctx_cast3(x, args...) ___bpf_ctx_cast2(args), (void *)ctx[2]
+#define ___bpf_ctx_cast4(x, args...) ___bpf_ctx_cast3(args), (void *)ctx[3]
+#define ___bpf_ctx_cast5(x, args...) ___bpf_ctx_cast4(args), (void *)ctx[4]
+#define ___bpf_ctx_cast6(x, args...) ___bpf_ctx_cast5(args), (void *)ctx[5]
+#define ___bpf_ctx_cast7(x, args...) ___bpf_ctx_cast6(args), (void *)ctx[6]
+#define ___bpf_ctx_cast8(x, args...) ___bpf_ctx_cast7(args), (void *)ctx[7]
+#define ___bpf_ctx_cast9(x, args...) ___bpf_ctx_cast8(args), (void *)ctx[8]
+#define ___bpf_ctx_cast10(x, args...) ___bpf_ctx_cast9(args), (void *)ctx[9]
+#define ___bpf_ctx_cast11(x, args...) ___bpf_ctx_cast10(args), (void *)ctx[10]
+#define ___bpf_ctx_cast12(x, args...) ___bpf_ctx_cast11(args), (void *)ctx[11]
+#define ___bpf_ctx_cast(args...) \
+	___bpf_apply(___bpf_ctx_cast, ___bpf_narg(args))(args)
+
+/*
+ * BPF_PROG is a convenience wrapper for generic tp_btf/fentry/fexit and
+ * similar kinds of BPF programs, that accept input arguments as a single
+ * pointer to untyped u64 array, where each u64 can actually be a typed
+ * pointer or integer of different size. Instead of requring user to write
+ * manual casts and work with array elements by index, BPF_PROG macro
+ * allows user to declare a list of named and typed input arguments in the
+ * same syntax as for normal C function. All the casting is hidden and
+ * performed transparently, while user code can just assume working with
+ * function arguments of specified type and name.
+ *
+ * Original raw context argument is preserved as well as 'ctx' argument.
+ * This is useful when using BPF helpers that expect original context
+ * as one of the parameters (e.g., for bpf_perf_event_output()).
+ */
+#define BPF_PROG(name, args...)						    \
+name(unsigned long long *ctx);						    \
+static __always_inline typeof(name(0))					    \
+____##name(unsigned long long *ctx, ##args);				    \
+typeof(name(0)) name(unsigned long long *ctx)				    \
+{									    \
+	_Pragma("GCC diagnostic push")					    \
+	_Pragma("GCC diagnostic ignored \"-Wint-conversion\"")		    \
+	return ____##name(___bpf_ctx_cast(args));			    \
+	_Pragma("GCC diagnostic pop")					    \
+}									    \
+static __always_inline typeof(name(0))					    \
+____##name(unsigned long long *ctx, ##args)
+
+/* 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));
+} 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));
+} accum_readings SEC(".maps");
+
+/* sample counts, one per cpu */
+struct {
+	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
+	__uint(key_size, sizeof(u32));
+	__uint(value_size, sizeof(u64));
+} counts SEC(".maps");
+
+const volatile __u32 num_cpu = 1;
+const volatile __u32 num_metric = 1;
+#define MAX_NUM_MATRICS 4
+
+SEC("fentry/XXX")
+int BPF_PROG(fentry_XXX)
+{
+	struct bpf_perf_event_value *ptrs[MAX_NUM_MATRICS];
+	u32 key = bpf_get_smp_processor_id();
+	u32 i;
+
+	/* look up before reading, to reduce error */
+	for (i = 0; i < num_metric && i < MAX_NUM_MATRICS; i++) {
+		u32 flag = i;
+
+		ptrs[i] = bpf_map_lookup_elem(&fentry_readings, &flag);
+		if (!ptrs[i])
+			return 0;
+	}
+
+	for (i = 0; i < num_metric && i < MAX_NUM_MATRICS; i++) {
+		struct bpf_perf_event_value reading;
+		int err;
+
+		err = bpf_perf_event_read_value(&events, key, &reading,
+						sizeof(reading));
+		if (err)
+			return 0;
+		*(ptrs[i]) = reading;
+		key += num_cpu;
+	}
+
+	return 0;
+}
+
+static inline void
+fexit_update_maps(u32 id, struct bpf_perf_event_value *after)
+{
+	struct bpf_perf_event_value *before, diff, *accum;
+
+	before = bpf_map_lookup_elem(&fentry_readings, &id);
+	/* 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, &id);
+		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 readings[MAX_NUM_MATRICS];
+	u32 cpu = bpf_get_smp_processor_id();
+	u32 i, one = 1, zero = 0;
+	int err;
+	u64 *count;
+
+	/* read all events before updating the maps, to reduce error */
+	for (i = 0; i < num_metric && i < MAX_NUM_MATRICS; i++) {
+		err = bpf_perf_event_read_value(&events, cpu + i * num_cpu,
+						readings + i, sizeof(*readings));
+		if (err)
+			return 0;
+	}
+	count = bpf_map_lookup_elem(&counts, &zero);
+	if (count) {
+		*count += 1;
+		for (i = 0; i < num_metric && i < MAX_NUM_MATRICS; i++)
+			fexit_update_maps(i, &readings[i]);
+	}
+	return 0;
+}
+
+char LICENSE[] SEC("license") = "GPL";
diff --git a/tools/bpf/bpftool/skeleton/profiler.h b/tools/bpf/bpftool/skeleton/profiler.h
new file mode 100644
index 000000000000..e03b53eae767
--- /dev/null
+++ b/tools/bpf/bpftool/skeleton/profiler.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
+#ifndef __PROFILER_H
+#define __PROFILER_H
+
+/* useful typedefs from vmlinux.h */
+
+typedef signed char __s8;
+typedef unsigned char __u8;
+typedef short int __s16;
+typedef short unsigned int __u16;
+typedef int __s32;
+typedef unsigned int __u32;
+typedef long long int __s64;
+typedef long long unsigned int __u64;
+
+typedef __s8 s8;
+typedef __u8 u8;
+typedef __s16 s16;
+typedef __u16 u16;
+typedef __s32 s32;
+typedef __u32 u32;
+typedef __s64 s64;
+typedef __u64 u64;
+
+enum {
+	false = 0,
+	true = 1,
+};
+
+#ifdef __CHECKER__
+#define __bitwise__ __attribute__((bitwise))
+#else
+#define __bitwise__
+#endif
+#define __bitwise __bitwise__
+
+typedef __u16 __bitwise __le16;
+typedef __u16 __bitwise __be16;
+typedef __u32 __bitwise __le32;
+typedef __u32 __bitwise __be32;
+typedef __u64 __bitwise __le64;
+typedef __u64 __bitwise __be64;
+
+typedef __u16 __bitwise __sum16;
+typedef __u32 __bitwise __wsum;
+
+#endif /* __PROFILER_H */
diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index ded7a950dc40..59f31f01cb93 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -106,6 +106,7 @@ ifneq ($(silent),1)
   ifneq ($(V),1)
 	QUIET_CC       = @echo '  CC       '$@;
 	QUIET_CC_FPIC  = @echo '  CC FPIC  '$@;
+	QUIET_CLANG    = @echo '  CLANG    '$@;
 	QUIET_AR       = @echo '  AR       '$@;
 	QUIET_LINK     = @echo '  LINK     '$@;
 	QUIET_MKDIR    = @echo '  MKDIR    '$@;
-- 
2.17.1


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

* [PATCH v4 bpf-next 2/4] bpftool: Documentation for bpftool prog profile
  2020-03-04 18:07 [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile Song Liu
  2020-03-04 18:07 ` [PATCH v4 bpf-next 1/4] bpftool: introduce "prog profile" command Song Liu
@ 2020-03-04 18:07 ` Song Liu
  2020-03-04 18:07 ` [PATCH v4 bpf-next 3/4] bpftool: bash completion for "bpftool prog profile" Song Liu
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2020-03-04 18:07 UTC (permalink / raw)
  To: netdev, bpf
  Cc: quentin, kernel-team, ast, daniel, arnaldo.melo, jolsa, Song Liu

Add documentation for the new bpftool prog profile command.

Reviewed-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
 .../bpftool/Documentation/bpftool-prog.rst    | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 46862e85fed2..9f19404f470e 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -30,6 +30,7 @@ PROG COMMANDS
 |	**bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
 |	**bpftool** **prog tracelog**
 |	**bpftool** **prog run** *PROG* **data_in** *FILE* [**data_out** *FILE* [**data_size_out** *L*]] [**ctx_in** *FILE* [**ctx_out** *FILE* [**ctx_size_out** *M*]]] [**repeat** *N*]
+|	**bpftool** **prog profile** *PROG* [**duration** *DURATION*] *METRICs*
 |	**bpftool** **prog help**
 |
 |	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
@@ -48,6 +49,9 @@ PROG COMMANDS
 |       *ATTACH_TYPE* := {
 |		**msg_verdict** | **stream_verdict** | **stream_parser** | **flow_dissector**
 |	}
+|	*METRIC* := {
+|		**cycles** | **instructions** | **l1d_loads** | **llc_misses**
+|	}
 
 
 DESCRIPTION
@@ -189,6 +193,12 @@ DESCRIPTION
 		  not all of them can take the **ctx_in**/**ctx_out**
 		  arguments. bpftool does not perform checks on program types.
 
+	**bpftool prog profile** *PROG* [**duration** *DURATION*] *METRICs*
+		  Profile *METRICs* for bpf program *PROG* for *DURATION*
+		  seconds or until user hits Ctrl-C. *DURATION* is optional.
+		  If *DURATION* is not specified, the profiling will run up to
+		  UINT_MAX seconds.
+
 	**bpftool prog help**
 		  Print short help message.
 
@@ -311,6 +321,15 @@ EXAMPLES
 
 **# rm /sys/fs/bpf/xdp1**
 
+|
+| **# bpftool prog profile id 337 duration 10 cycles instructions llc_misses**
+
+::
+         51397 run_cnt
+      40176203 cycles                                                 (83.05%)
+      42518139 instructions    #   1.06 insns per cycle               (83.39%)
+           123 llc_misses      #   2.89 LLC misses per million insns  (83.15%)
+
 SEE ALSO
 ========
 	**bpf**\ (2),
-- 
2.17.1


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

* [PATCH v4 bpf-next 3/4] bpftool: bash completion for "bpftool prog profile"
  2020-03-04 18:07 [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile Song Liu
  2020-03-04 18:07 ` [PATCH v4 bpf-next 1/4] bpftool: introduce "prog profile" command Song Liu
  2020-03-04 18:07 ` [PATCH v4 bpf-next 2/4] bpftool: Documentation for bpftool prog profile Song Liu
@ 2020-03-04 18:07 ` Song Liu
  2020-03-04 18:07 ` [PATCH v4 bpf-next 4/4] bpftool: fix typo in bash-completion Song Liu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2020-03-04 18:07 UTC (permalink / raw)
  To: netdev, bpf
  Cc: quentin, kernel-team, ast, daniel, arnaldo.melo, jolsa, Song Liu

Add bash completion for "bpftool prog profile" command.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/bpf/bpftool/bash-completion/bpftool | 45 ++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index f2838a658339..49f4ab2f67e3 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -337,6 +337,7 @@ _bpftool()
 
             local PROG_TYPE='id pinned tag name'
             local MAP_TYPE='id pinned name'
+            local METRIC_TYPE='cycles instructions l1d_loads llc_misses'
             case $command in
                 show|list)
                     [[ $prev != "$command" ]] && return 0
@@ -498,6 +499,48 @@ _bpftool()
                 tracelog)
                     return 0
                     ;;
+                profile)
+                    case $cword in
+                        3)
+                            COMPREPLY=( $( compgen -W "$PROG_TYPE" -- "$cur" ) )
+                            return 0
+                            ;;
+                        4)
+                            case $prev in
+                                id)
+                                    _bpftool_get_prog_ids
+                                    ;;
+                                name)
+                                    _bpftool_get_prog_names
+                                    ;;
+                                pinned)
+                                    _filedir
+                                    ;;
+                            esac
+                            return 0
+                            ;;
+                        5)
+                            COMPREPLY=( $( compgen -W "$METRIC_TYPE duration" -- "$cur" ) )
+                            return 0
+                            ;;
+                        6)
+                            case $prev in
+                                duration)
+                                    return 0
+                                    ;;
+                                *)
+                                    COMPREPLY=( $( compgen -W "$METRIC_TYPE" -- "$cur" ) )
+                                    return 0
+                                    ;;
+                            esac
+                            return 0
+                            ;;
+                        *)
+                            COMPREPLY=( $( compgen -W "$METRIC_TYPE" -- "$cur" ) )
+                            return 0
+                            ;;
+                    esac
+                    ;;
                 run)
                     if [[ ${#words[@]} -lt 5 ]]; then
                         _filedir
@@ -525,7 +568,7 @@ _bpftool()
                 *)
                     [[ $prev == $object ]] && \
                         COMPREPLY=( $( compgen -W 'dump help pin attach detach \
-                            load loadall show list tracelog run' -- "$cur" ) )
+                            load loadall show list tracelog run profile' -- "$cur" ) )
                     ;;
             esac
             ;;
-- 
2.17.1


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

* [PATCH v4 bpf-next 4/4] bpftool: fix typo in bash-completion
  2020-03-04 18:07 [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile Song Liu
                   ` (2 preceding siblings ...)
  2020-03-04 18:07 ` [PATCH v4 bpf-next 3/4] bpftool: bash completion for "bpftool prog profile" Song Liu
@ 2020-03-04 18:07 ` Song Liu
  2020-03-04 19:08 ` [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile Jiri Olsa
  2020-03-04 19:13 ` Quentin Monnet
  5 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2020-03-04 18:07 UTC (permalink / raw)
  To: netdev, bpf
  Cc: quentin, kernel-team, ast, daniel, arnaldo.melo, jolsa, Song Liu,
	Paul Chaignon

_bpftool_get_map_names => _bpftool_get_prog_names for prog-attach|detach.

Fixes: 99f9863a0c45 ("bpftool: Match maps by name")
Cc: Paul Chaignon <paul.chaignon@orange.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/bpf/bpftool/bash-completion/bpftool | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 49f4ab2f67e3..a9cce9d3745a 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -389,7 +389,7 @@ _bpftool()
                                     _bpftool_get_prog_ids
                                     ;;
                                 name)
-                                    _bpftool_get_map_names
+                                    _bpftool_get_prog_names
                                     ;;
                                 pinned)
                                     _filedir
-- 
2.17.1


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

* Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
  2020-03-04 18:07 [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile Song Liu
                   ` (3 preceding siblings ...)
  2020-03-04 18:07 ` [PATCH v4 bpf-next 4/4] bpftool: fix typo in bash-completion Song Liu
@ 2020-03-04 19:08 ` Jiri Olsa
  2020-03-04 20:41   ` Jiri Olsa
  2020-03-04 19:13 ` Quentin Monnet
  5 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2020-03-04 19:08 UTC (permalink / raw)
  To: Song Liu
  Cc: netdev, bpf, quentin, kernel-team, ast, daniel, arnaldo.melo, jolsa

On Wed, Mar 04, 2020 at 10:07:06AM -0800, Song Liu wrote:
> This set introduces bpftool prog profile command, which uses hardware
> counters to profile BPF programs.
> 
> This command attaches fentry/fexit programs to a target program. These two
> programs read hardware counters before and after the target program and
> calculate the difference.
> 
> Changes v3 => v4:
> 1. Simplify err handling in profile_open_perf_events() (Quentin);
> 2. Remove redundant p_err() (Quentin);
> 3. Replace tab with space in bash-completion; (Quentin);
> 4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).

hum, I'm getting:

	[jolsa@dell-r440-01 bpftool]$ pwd
	/home/jolsa/linux-perf/tools/bpf/bpftool
	[jolsa@dell-r440-01 bpftool]$ make
	...
	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
	  LINK     _bpftool
	make: *** No rule to make target 'skeleton/profiler.bpf.c', needed by 'skeleton/profiler.bpf.o'.  Stop.

jirka

> 
> Changes v2 => v3:
> 1. Change order of arguments (Quentin), as:
>      bpftool prog profile PROG [duration DURATION] METRICs
> 2. Add bash-completion for bpftool prog profile (Quentin);
> 3. Fix build of selftests (Yonghong);
> 4. Better handling of bpf_map_lookup_elem() returns (Yonghong);
> 5. Improve clean up logic of do_profile() (Yonghong);
> 6. Other smaller fixes/cleanups.
> 
> Changes RFC => v2:
> 1. Use new bpf_program__set_attach_target() API;
> 2. Update output format to be perf-stat like (Alexei);
> 3. Incorporate skeleton generation into Makefile;
> 4. Make DURATION optional and Allow Ctrl-C (Alexei);
> 5. Add calcated values "insn per cycle" and "LLC misses per million isns".
> 
> Song Liu (4):
>   bpftool: introduce "prog profile" command
>   bpftool: Documentation for bpftool prog profile
>   bpftool: bash completion for "bpftool prog profile"
>   bpftool: fix typo in bash-completion
> 
>  .../bpftool/Documentation/bpftool-prog.rst    |  19 +
>  tools/bpf/bpftool/Makefile                    |  18 +
>  tools/bpf/bpftool/bash-completion/bpftool     |  47 +-
>  tools/bpf/bpftool/prog.c                      | 425 +++++++++++++++++-
>  tools/bpf/bpftool/skeleton/profiler.bpf.c     | 171 +++++++
>  tools/bpf/bpftool/skeleton/profiler.h         |  47 ++
>  tools/scripts/Makefile.include                |   1 +
>  7 files changed, 725 insertions(+), 3 deletions(-)
>  create mode 100644 tools/bpf/bpftool/skeleton/profiler.bpf.c
>  create mode 100644 tools/bpf/bpftool/skeleton/profiler.h
> 
> --
> 2.17.1
> 


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

* Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
  2020-03-04 18:07 [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile Song Liu
                   ` (4 preceding siblings ...)
  2020-03-04 19:08 ` [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile Jiri Olsa
@ 2020-03-04 19:13 ` Quentin Monnet
  5 siblings, 0 replies; 20+ messages in thread
From: Quentin Monnet @ 2020-03-04 19:13 UTC (permalink / raw)
  To: Song Liu, netdev, bpf; +Cc: kernel-team, ast, daniel, arnaldo.melo, jolsa

2020-03-04 10:07 UTC-0800 ~ Song Liu <songliubraving@fb.com>
> This set introduces bpftool prog profile command, which uses hardware
> counters to profile BPF programs.
> 
> This command attaches fentry/fexit programs to a target program. These two
> programs read hardware counters before and after the target program and
> calculate the difference.
> 
> Changes v3 => v4:
> 1. Simplify err handling in profile_open_perf_events() (Quentin);
> 2. Remove redundant p_err() (Quentin);
> 3. Replace tab with space in bash-completion; (Quentin);
> 4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).
> 
> Changes v2 => v3:
> 1. Change order of arguments (Quentin), as:
>      bpftool prog profile PROG [duration DURATION] METRICs
> 2. Add bash-completion for bpftool prog profile (Quentin);
> 3. Fix build of selftests (Yonghong);
> 4. Better handling of bpf_map_lookup_elem() returns (Yonghong);
> 5. Improve clean up logic of do_profile() (Yonghong);
> 6. Other smaller fixes/cleanups.
> 
> Changes RFC => v2:
> 1. Use new bpf_program__set_attach_target() API;
> 2. Update output format to be perf-stat like (Alexei);
> 3. Incorporate skeleton generation into Makefile;
> 4. Make DURATION optional and Allow Ctrl-C (Alexei);
> 5. Add calcated values "insn per cycle" and "LLC misses per million isns".
> 
> Song Liu (4):
>   bpftool: introduce "prog profile" command
>   bpftool: Documentation for bpftool prog profile
>   bpftool: bash completion for "bpftool prog profile"
>   bpftool: fix typo in bash-completion
> 
>  .../bpftool/Documentation/bpftool-prog.rst    |  19 +
>  tools/bpf/bpftool/Makefile                    |  18 +
>  tools/bpf/bpftool/bash-completion/bpftool     |  47 +-
>  tools/bpf/bpftool/prog.c                      | 425 +++++++++++++++++-
>  tools/bpf/bpftool/skeleton/profiler.bpf.c     | 171 +++++++
>  tools/bpf/bpftool/skeleton/profiler.h         |  47 ++
>  tools/scripts/Makefile.include                |   1 +
>  7 files changed, 725 insertions(+), 3 deletions(-)
>  create mode 100644 tools/bpf/bpftool/skeleton/profiler.bpf.c
>  create mode 100644 tools/bpf/bpftool/skeleton/profiler.h
> 
> --

Thanks again! This version looks good to me, although I've not tested
the patchset, so there's still the error met by Jiri to figure out. For
the rest of the series:

Reviewed-by: Quentin Monnet <quentin@isovalent.com>

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

* Re: [PATCH v4 bpf-next 1/4] bpftool: introduce "prog profile" command
  2020-03-04 18:07 ` [PATCH v4 bpf-next 1/4] bpftool: introduce "prog profile" command Song Liu
@ 2020-03-04 19:21   ` Jiri Olsa
  2020-03-04 21:17     ` Song Liu
  2020-03-04 20:38   ` Jiri Olsa
  1 sibling, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2020-03-04 19:21 UTC (permalink / raw)
  To: Song Liu
  Cc: netdev, bpf, quentin, kernel-team, ast, daniel, arnaldo.melo, jolsa

On Wed, Mar 04, 2020 at 10:07:07AM -0800, Song Liu wrote:
> With fentry/fexit programs, it is possible to profile BPF program with
> hardware counters. Introduce bpftool "prog profile", which measures key
> metrics of a BPF program.
> 
> bpftool prog profile command creates per-cpu perf events. Then it attaches
> fentry/fexit programs to the target BPF program. The fentry program saves
> perf event value to a map. The fexit program reads the perf event again,
> and calculates the difference, which is the instructions/cycles used by
> the target program.
> 
> Example input and output:
> 
>   ./bpftool prog profile id 337 duration 3 cycles instructions llc_misses
> 
>         4228 run_cnt
>      3403698 cycles                                              (84.08%)
>      3525294 instructions   #  1.04 insn per cycle               (84.05%)
>           13 llc_misses     #  3.69 LLC misses per million isns  (83.50%)

FYI I'm in the middle of moving perf's 'events parsing' interface to libperf,
which takes event name/s on input and returns list of perf_event_attr objects

  parse_events("cycles") -> ready to use 'struct perf_event_attr'

You can use any event that's listed in 'perf list' command, which includes
also all vendor (Intel/Arm/ppc..) events. It might be useful extension for
this command.

jirka


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

* Re: [PATCH v4 bpf-next 1/4] bpftool: introduce "prog profile" command
  2020-03-04 18:07 ` [PATCH v4 bpf-next 1/4] bpftool: introduce "prog profile" command Song Liu
  2020-03-04 19:21   ` Jiri Olsa
@ 2020-03-04 20:38   ` Jiri Olsa
  2020-03-05 20:03     ` Song Liu
  1 sibling, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2020-03-04 20:38 UTC (permalink / raw)
  To: Song Liu
  Cc: netdev, bpf, quentin, kernel-team, ast, daniel, arnaldo.melo, jolsa

On Wed, Mar 04, 2020 at 10:07:07AM -0800, Song Liu wrote:

SNIP

> +
> +#include "profiler.skel.h"
> +
> +#define SAMPLE_PERIOD  0x7fffffffffffffffULL
> +struct profile_metric {
> +	const char *name;
> +	struct bpf_perf_event_value val;
> +	struct perf_event_attr attr;
> +	bool selected;
> +
> +	/* calculate ratios like instructions per cycle */
> +	const int ratio_metric; /* 0 for N/A, 1 for index 0 (cycles) */
> +	const char *ratio_desc;
> +	const float ratio_mul;
> +} metrics[] = {
> +	{
> +		.name = "cycles",
> +		.attr = {
> +			.sample_period = SAMPLE_PERIOD,

I don't think you need to set sample_period for counting.. why?

> +			.type = PERF_TYPE_HARDWARE,
> +			.config = PERF_COUNT_HW_CPU_CYCLES,

you could also add .exclude_user = 1

jirka


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

* Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
  2020-03-04 19:08 ` [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile Jiri Olsa
@ 2020-03-04 20:41   ` Jiri Olsa
  2020-03-04 21:16     ` Song Liu
  0 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2020-03-04 20:41 UTC (permalink / raw)
  To: Song Liu
  Cc: netdev, bpf, quentin, kernel-team, ast, daniel, arnaldo.melo, jolsa

On Wed, Mar 04, 2020 at 08:08:07PM +0100, Jiri Olsa wrote:
> On Wed, Mar 04, 2020 at 10:07:06AM -0800, Song Liu wrote:
> > This set introduces bpftool prog profile command, which uses hardware
> > counters to profile BPF programs.
> > 
> > This command attaches fentry/fexit programs to a target program. These two
> > programs read hardware counters before and after the target program and
> > calculate the difference.
> > 
> > Changes v3 => v4:
> > 1. Simplify err handling in profile_open_perf_events() (Quentin);
> > 2. Remove redundant p_err() (Quentin);
> > 3. Replace tab with space in bash-completion; (Quentin);
> > 4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).
> 
> hum, I'm getting:
> 
> 	[jolsa@dell-r440-01 bpftool]$ pwd
> 	/home/jolsa/linux-perf/tools/bpf/bpftool
> 	[jolsa@dell-r440-01 bpftool]$ make
> 	...
> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
> 	  LINK     _bpftool
> 	make: *** No rule to make target 'skeleton/profiler.bpf.c', needed by 'skeleton/profiler.bpf.o'.  Stop.

ok, I had to apply your patches by hand, because 'git am' refused to
due to fuzz.. so some of you new files did not make it to my tree ;-)

anyway I hit another error now:

	  CC       prog.o
	In file included from prog.c:1553:
	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
	      |                                   ^~
	prog.c: In function ‘profile_read_values’:
	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;

I'll try to figure it out.. might be error on my end

do you have git repo with these changes?

thanks,
jirka


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

* Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
  2020-03-04 20:41   ` Jiri Olsa
@ 2020-03-04 21:16     ` Song Liu
  2020-03-04 21:29       ` Jiri Olsa
  0 siblings, 1 reply; 20+ messages in thread
From: Song Liu @ 2020-03-04 21:16 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Networking, bpf, quentin, Kernel Team, ast, daniel, arnaldo.melo, jolsa



> On Mar 4, 2020, at 12:41 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Mar 04, 2020 at 08:08:07PM +0100, Jiri Olsa wrote:
>> On Wed, Mar 04, 2020 at 10:07:06AM -0800, Song Liu wrote:
>>> This set introduces bpftool prog profile command, which uses hardware
>>> counters to profile BPF programs.
>>> 
>>> This command attaches fentry/fexit programs to a target program. These two
>>> programs read hardware counters before and after the target program and
>>> calculate the difference.
>>> 
>>> Changes v3 => v4:
>>> 1. Simplify err handling in profile_open_perf_events() (Quentin);
>>> 2. Remove redundant p_err() (Quentin);
>>> 3. Replace tab with space in bash-completion; (Quentin);
>>> 4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).
>> 
>> hum, I'm getting:
>> 
>> 	[jolsa@dell-r440-01 bpftool]$ pwd
>> 	/home/jolsa/linux-perf/tools/bpf/bpftool
>> 	[jolsa@dell-r440-01 bpftool]$ make
>> 	...
>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
>> 	  LINK     _bpftool
>> 	make: *** No rule to make target 'skeleton/profiler.bpf.c', needed by 'skeleton/profiler.bpf.o'.  Stop.
> 
> ok, I had to apply your patches by hand, because 'git am' refused to
> due to fuzz.. so some of you new files did not make it to my tree ;-)
> 
> anyway I hit another error now:
> 
> 	  CC       prog.o
> 	In file included from prog.c:1553:
> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
> 	      |                                   ^~
> 	prog.c: In function ‘profile_read_values’:
> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
> 
> I'll try to figure it out.. might be error on my end
> 
> do you have git repo with these changes?

I pushed it to 

https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git/tree/?h=bpf-per-prog-stats

Thanks,
Song


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

* Re: [PATCH v4 bpf-next 1/4] bpftool: introduce "prog profile" command
  2020-03-04 19:21   ` Jiri Olsa
@ 2020-03-04 21:17     ` Song Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2020-03-04 21:17 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Networking, bpf, quentin, Kernel Team, ast, daniel, arnaldo.melo, jolsa



> On Mar 4, 2020, at 11:21 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Mar 04, 2020 at 10:07:07AM -0800, Song Liu wrote:
>> With fentry/fexit programs, it is possible to profile BPF program with
>> hardware counters. Introduce bpftool "prog profile", which measures key
>> metrics of a BPF program.
>> 
>> bpftool prog profile command creates per-cpu perf events. Then it attaches
>> fentry/fexit programs to the target BPF program. The fentry program saves
>> perf event value to a map. The fexit program reads the perf event again,
>> and calculates the difference, which is the instructions/cycles used by
>> the target program.
>> 
>> Example input and output:
>> 
>>  ./bpftool prog profile id 337 duration 3 cycles instructions llc_misses
>> 
>>        4228 run_cnt
>>     3403698 cycles                                              (84.08%)
>>     3525294 instructions   #  1.04 insn per cycle               (84.05%)
>>          13 llc_misses     #  3.69 LLC misses per million isns  (83.50%)
> 
> FYI I'm in the middle of moving perf's 'events parsing' interface to libperf,
> which takes event name/s on input and returns list of perf_event_attr objects
> 
>  parse_events("cycles") -> ready to use 'struct perf_event_attr'
> 
> You can use any event that's listed in 'perf list' command, which includes
> also all vendor (Intel/Arm/ppc..) events. It might be useful extension for
> this command.

That's great news! Thanks for the information!

Song


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

* Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
  2020-03-04 21:16     ` Song Liu
@ 2020-03-04 21:29       ` Jiri Olsa
  2020-03-04 21:39         ` Song Liu
  0 siblings, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2020-03-04 21:29 UTC (permalink / raw)
  To: Song Liu
  Cc: Networking, bpf, quentin, Kernel Team, ast, daniel, arnaldo.melo, jolsa

On Wed, Mar 04, 2020 at 09:16:29PM +0000, Song Liu wrote:
> 
> 
> > On Mar 4, 2020, at 12:41 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> > 
> > On Wed, Mar 04, 2020 at 08:08:07PM +0100, Jiri Olsa wrote:
> >> On Wed, Mar 04, 2020 at 10:07:06AM -0800, Song Liu wrote:
> >>> This set introduces bpftool prog profile command, which uses hardware
> >>> counters to profile BPF programs.
> >>> 
> >>> This command attaches fentry/fexit programs to a target program. These two
> >>> programs read hardware counters before and after the target program and
> >>> calculate the difference.
> >>> 
> >>> Changes v3 => v4:
> >>> 1. Simplify err handling in profile_open_perf_events() (Quentin);
> >>> 2. Remove redundant p_err() (Quentin);
> >>> 3. Replace tab with space in bash-completion; (Quentin);
> >>> 4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).
> >> 
> >> hum, I'm getting:
> >> 
> >> 	[jolsa@dell-r440-01 bpftool]$ pwd
> >> 	/home/jolsa/linux-perf/tools/bpf/bpftool
> >> 	[jolsa@dell-r440-01 bpftool]$ make
> >> 	...
> >> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
> >> 	  LINK     _bpftool
> >> 	make: *** No rule to make target 'skeleton/profiler.bpf.c', needed by 'skeleton/profiler.bpf.o'.  Stop.
> > 
> > ok, I had to apply your patches by hand, because 'git am' refused to
> > due to fuzz.. so some of you new files did not make it to my tree ;-)
> > 
> > anyway I hit another error now:
> > 
> > 	  CC       prog.o
> > 	In file included from prog.c:1553:
> > 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
> > 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> > 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
> > 	      |                                   ^~
> > 	prog.c: In function ‘profile_read_values’:
> > 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> > 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
> > 
> > I'll try to figure it out.. might be error on my end
> > 
> > do you have git repo with these changes?
> 
> I pushed it to 
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git/tree/?h=bpf-per-prog-stats

still the same:

	[jolsa@dell-r440-01 bpftool]$ git show --oneline HEAD | head -1
	7bbda5cca00a bpftool: fix typo in bash-completion
	[jolsa@dell-r440-01 bpftool]$ make 
	make[1]: Entering directory '/home/jolsa/linux-perf/tools/lib/bpf'
	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
	  CC       prog.o
	In file included from prog.c:1553:
	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
	      |                                   ^~
	prog.c: In function ‘profile_read_values’:
	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
	      |                             ^~
	prog.c: In function ‘profile_open_perf_events’:
	prog.c:1810:19: error: ‘struct profiler_bpf’ has no member named ‘rodata’
	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
	      |                   ^~
	prog.c:1810:42: error: ‘struct profiler_bpf’ has no member named ‘rodata’
	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
	      |                                          ^~
	prog.c:1825:26: error: ‘struct profiler_bpf’ has no member named ‘rodata’
	 1825 |   for (cpu = 0; cpu < obj->rodata->num_cpu; cpu++) {
	      |                          ^~
	prog.c: In function ‘do_profile’:
	prog.c:1904:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
	 1904 |  profile_obj->rodata->num_cpu = num_cpu;
	      |             ^~
	prog.c:1905:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
	 1905 |  profile_obj->rodata->num_metric = num_metric;
	      |             ^~
	make: *** [Makefile:129: prog.o] Error 1


jirka


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

* Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
  2020-03-04 21:29       ` Jiri Olsa
@ 2020-03-04 21:39         ` Song Liu
  2020-03-05 20:03           ` Song Liu
  2020-03-09 18:04           ` Quentin Monnet
  0 siblings, 2 replies; 20+ messages in thread
From: Song Liu @ 2020-03-04 21:39 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Networking, bpf, quentin, Kernel Team, ast, daniel, arnaldo.melo, jolsa



> On Mar 4, 2020, at 1:29 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Mar 04, 2020 at 09:16:29PM +0000, Song Liu wrote:
>> 
>> 
>>> On Mar 4, 2020, at 12:41 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>>> 
>>> On Wed, Mar 04, 2020 at 08:08:07PM +0100, Jiri Olsa wrote:
>>>> On Wed, Mar 04, 2020 at 10:07:06AM -0800, Song Liu wrote:
>>>>> This set introduces bpftool prog profile command, which uses hardware
>>>>> counters to profile BPF programs.
>>>>> 
>>>>> This command attaches fentry/fexit programs to a target program. These two
>>>>> programs read hardware counters before and after the target program and
>>>>> calculate the difference.
>>>>> 
>>>>> Changes v3 => v4:
>>>>> 1. Simplify err handling in profile_open_perf_events() (Quentin);
>>>>> 2. Remove redundant p_err() (Quentin);
>>>>> 3. Replace tab with space in bash-completion; (Quentin);
>>>>> 4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).
>>>> 
>>>> hum, I'm getting:
>>>> 
>>>> 	[jolsa@dell-r440-01 bpftool]$ pwd
>>>> 	/home/jolsa/linux-perf/tools/bpf/bpftool
>>>> 	[jolsa@dell-r440-01 bpftool]$ make
>>>> 	...
>>>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
>>>> 	  LINK     _bpftool
>>>> 	make: *** No rule to make target 'skeleton/profiler.bpf.c', needed by 'skeleton/profiler.bpf.o'.  Stop.
>>> 
>>> ok, I had to apply your patches by hand, because 'git am' refused to
>>> due to fuzz.. so some of you new files did not make it to my tree ;-)
>>> 
>>> anyway I hit another error now:
>>> 
>>> 	  CC       prog.o
>>> 	In file included from prog.c:1553:
>>> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
>>> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
>>> 	      |                                   ^~
>>> 	prog.c: In function ‘profile_read_values’:
>>> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>>> 
>>> I'll try to figure it out.. might be error on my end
>>> 
>>> do you have git repo with these changes?
>> 
>> I pushed it to 
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git/tree/?h=bpf-per-prog-stats
> 
> still the same:
> 
> 	[jolsa@dell-r440-01 bpftool]$ git show --oneline HEAD | head -1
> 	7bbda5cca00a bpftool: fix typo in bash-completion
> 	[jolsa@dell-r440-01 bpftool]$ make 
> 	make[1]: Entering directory '/home/jolsa/linux-perf/tools/lib/bpf'
> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
> 	  CC       prog.o
> 	In file included from prog.c:1553:
> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
> 	      |                                   ^~
> 	prog.c: In function ‘profile_read_values’:
> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
> 	      |                             ^~
> 	prog.c: In function ‘profile_open_perf_events’:
> 	prog.c:1810:19: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
> 	      |                   ^~
> 	prog.c:1810:42: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
> 	      |                                          ^~
> 	prog.c:1825:26: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> 	 1825 |   for (cpu = 0; cpu < obj->rodata->num_cpu; cpu++) {
> 	      |                          ^~
> 	prog.c: In function ‘do_profile’:
> 	prog.c:1904:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> 	 1904 |  profile_obj->rodata->num_cpu = num_cpu;
> 	      |             ^~
> 	prog.c:1905:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> 	 1905 |  profile_obj->rodata->num_metric = num_metric;
> 	      |             ^~
> 	make: *** [Makefile:129: prog.o] Error 1

I guess you need a newer version of clang that supports global data in BPF programs. 

Thanks,
Song

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

* Re: [PATCH v4 bpf-next 1/4] bpftool: introduce "prog profile" command
  2020-03-04 20:38   ` Jiri Olsa
@ 2020-03-05 20:03     ` Song Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2020-03-05 20:03 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Networking, bpf, Quentin Monnet, Kernel Team, ast, daniel,
	arnaldo.melo, jolsa



> On Mar 4, 2020, at 12:38 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Wed, Mar 04, 2020 at 10:07:07AM -0800, Song Liu wrote:
> 
> SNIP
> 
>> +
>> +#include "profiler.skel.h"
>> +
>> +#define SAMPLE_PERIOD  0x7fffffffffffffffULL
>> +struct profile_metric {
>> +	const char *name;
>> +	struct bpf_perf_event_value val;
>> +	struct perf_event_attr attr;
>> +	bool selected;
>> +
>> +	/* calculate ratios like instructions per cycle */
>> +	const int ratio_metric; /* 0 for N/A, 1 for index 0 (cycles) */
>> +	const char *ratio_desc;
>> +	const float ratio_mul;
>> +} metrics[] = {
>> +	{
>> +		.name = "cycles",
>> +		.attr = {
>> +			.sample_period = SAMPLE_PERIOD,
> 
> I don't think you need to set sample_period for counting.. why?
> 
>> +			.type = PERF_TYPE_HARDWARE,
>> +			.config = PERF_COUNT_HW_CPU_CYCLES,
> 
> you could also add .exclude_user = 1

Fixed this in v5. Thanks!
Song


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

* Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
  2020-03-04 21:39         ` Song Liu
@ 2020-03-05 20:03           ` Song Liu
  2020-03-06  8:51             ` Jiri Olsa
  2020-03-09 18:04           ` Quentin Monnet
  1 sibling, 1 reply; 20+ messages in thread
From: Song Liu @ 2020-03-05 20:03 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Networking, bpf, quentin, Kernel Team, ast, daniel, arnaldo.melo, jolsa



> On Mar 4, 2020, at 1:39 PM, Song Liu <songliubraving@fb.com> wrote:
> 
> 
> 
>> On Mar 4, 2020, at 1:29 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>> 
>> On Wed, Mar 04, 2020 at 09:16:29PM +0000, Song Liu wrote:
>>> 
>>> 
>>>> On Mar 4, 2020, at 12:41 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>>>> 
>>>> On Wed, Mar 04, 2020 at 08:08:07PM +0100, Jiri Olsa wrote:
>>>>> On Wed, Mar 04, 2020 at 10:07:06AM -0800, Song Liu wrote:
>>>>>> This set introduces bpftool prog profile command, which uses hardware
>>>>>> counters to profile BPF programs.
>>>>>> 
>>>>>> This command attaches fentry/fexit programs to a target program. These two
>>>>>> programs read hardware counters before and after the target program and
>>>>>> calculate the difference.
>>>>>> 
>>>>>> Changes v3 => v4:
>>>>>> 1. Simplify err handling in profile_open_perf_events() (Quentin);
>>>>>> 2. Remove redundant p_err() (Quentin);
>>>>>> 3. Replace tab with space in bash-completion; (Quentin);
>>>>>> 4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).
>>>>> 
>>>>> hum, I'm getting:
>>>>> 
>>>>> 	[jolsa@dell-r440-01 bpftool]$ pwd
>>>>> 	/home/jolsa/linux-perf/tools/bpf/bpftool
>>>>> 	[jolsa@dell-r440-01 bpftool]$ make
>>>>> 	...
>>>>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
>>>>> 	  LINK     _bpftool
>>>>> 	make: *** No rule to make target 'skeleton/profiler.bpf.c', needed by 'skeleton/profiler.bpf.o'. Stop.
>>>> 
>>>> ok, I had to apply your patches by hand, because 'git am' refused to
>>>> due to fuzz.. so some of you new files did not make it to my tree ;-)
>>>> 
>>>> anyway I hit another error now:
>>>> 
>>>> 	  CC       prog.o
>>>> 	In file included from prog.c:1553:
>>>> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
>>>> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>>> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
>>>> 	      |                                   ^~
>>>> 	prog.c: In function ‘profile_read_values’:
>>>> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>>> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>>>> 
>>>> I'll try to figure it out.. might be error on my end
>>>> 
>>>> do you have git repo with these changes?
>>> 
>>> I pushed it to 
>>> 
>>> https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git/tree/?h=bpf-per-prog-stats
>> 
>> still the same:
>> 
>> 	[jolsa@dell-r440-01 bpftool]$ git show --oneline HEAD | head -1
>> 	7bbda5cca00a bpftool: fix typo in bash-completion
>> 	[jolsa@dell-r440-01 bpftool]$ make 
>> 	make[1]: Entering directory '/home/jolsa/linux-perf/tools/lib/bpf'
>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
>> 	  CC       prog.o
>> 	In file included from prog.c:1553:
>> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
>> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
>> 	      |                                   ^~
>> 	prog.c: In function ‘profile_read_values’:
>> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>> 	      |                             ^~
>> 	prog.c: In function ‘profile_open_perf_events’:
>> 	prog.c:1810:19: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
>> 	      |                   ^~
>> 	prog.c:1810:42: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
>> 	      |                                          ^~
>> 	prog.c:1825:26: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1825 |   for (cpu = 0; cpu < obj->rodata->num_cpu; cpu++) {
>> 	      |                          ^~
>> 	prog.c: In function ‘do_profile’:
>> 	prog.c:1904:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1904 |  profile_obj->rodata->num_cpu = num_cpu;
>> 	      |             ^~
>> 	prog.c:1905:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1905 |  profile_obj->rodata->num_metric = num_metric;
>> 	      |             ^~
>> 	make: *** [Makefile:129: prog.o] Error 1
> 
> I guess you need a newer version of clang that supports global data in BPF programs. 

Hi Jiri,

Have you got chance to test this with latest clang? 

Thanks,
Song


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

* Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
  2020-03-05 20:03           ` Song Liu
@ 2020-03-06  8:51             ` Jiri Olsa
  0 siblings, 0 replies; 20+ messages in thread
From: Jiri Olsa @ 2020-03-06  8:51 UTC (permalink / raw)
  To: Song Liu
  Cc: Networking, bpf, quentin, Kernel Team, ast, daniel, arnaldo.melo, jolsa

On Thu, Mar 05, 2020 at 08:03:53PM +0000, Song Liu wrote:

SNIP

> >> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
> >> 	      |                             ^~
> >> 	prog.c: In function ‘profile_open_perf_events’:
> >> 	prog.c:1810:19: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
> >> 	      |                   ^~
> >> 	prog.c:1810:42: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
> >> 	      |                                          ^~
> >> 	prog.c:1825:26: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >> 	 1825 |   for (cpu = 0; cpu < obj->rodata->num_cpu; cpu++) {
> >> 	      |                          ^~
> >> 	prog.c: In function ‘do_profile’:
> >> 	prog.c:1904:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >> 	 1904 |  profile_obj->rodata->num_cpu = num_cpu;
> >> 	      |             ^~
> >> 	prog.c:1905:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >> 	 1905 |  profile_obj->rodata->num_metric = num_metric;
> >> 	      |             ^~
> >> 	make: *** [Makefile:129: prog.o] Error 1
> > 
> > I guess you need a newer version of clang that supports global data in BPF programs. 
> 
> Hi Jiri,
> 
> Have you got chance to test this with latest clang? 

yep, got it compiled with new clang

I was testing in on bpftrace programs and couldn't made it work,
because it relies on BTF info.. so I got stuck ;-)

  # bpftool prog profile id 241 duration 3 cycles instructions llc_misses
  Error: prog FD 3 doesn't have valid btf

nit.. ^^^ you could display ID instead of FD in here

I need to check if we can provide BTF info for bpftrace programs

jirka


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

* Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
  2020-03-04 21:39         ` Song Liu
  2020-03-05 20:03           ` Song Liu
@ 2020-03-09 18:04           ` Quentin Monnet
  2020-03-09 18:24             ` Song Liu
  1 sibling, 1 reply; 20+ messages in thread
From: Quentin Monnet @ 2020-03-09 18:04 UTC (permalink / raw)
  To: Song Liu, Jiri Olsa
  Cc: Networking, bpf, Kernel Team, ast, daniel, arnaldo.melo, jolsa

2020-03-04 21:39 UTC+0000 ~ Song Liu <songliubraving@fb.com>
> 
> 
>> On Mar 4, 2020, at 1:29 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>>
>> On Wed, Mar 04, 2020 at 09:16:29PM +0000, Song Liu wrote:
>>>
>>>
>>>> On Mar 4, 2020, at 12:41 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>>>>
>>>> On Wed, Mar 04, 2020 at 08:08:07PM +0100, Jiri Olsa wrote:
>>>>> On Wed, Mar 04, 2020 at 10:07:06AM -0800, Song Liu wrote:
>>>>>> This set introduces bpftool prog profile command, which uses hardware
>>>>>> counters to profile BPF programs.
>>>>>>
>>>>>> This command attaches fentry/fexit programs to a target program. These two
>>>>>> programs read hardware counters before and after the target program and
>>>>>> calculate the difference.
>>>>>>
>>>>>> Changes v3 => v4:
>>>>>> 1. Simplify err handling in profile_open_perf_events() (Quentin);
>>>>>> 2. Remove redundant p_err() (Quentin);
>>>>>> 3. Replace tab with space in bash-completion; (Quentin);
>>>>>> 4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).
>>>>>
>>>>> hum, I'm getting:
>>>>>
>>>>> 	[jolsa@dell-r440-01 bpftool]$ pwd
>>>>> 	/home/jolsa/linux-perf/tools/bpf/bpftool
>>>>> 	[jolsa@dell-r440-01 bpftool]$ make
>>>>> 	...
>>>>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
>>>>> 	  LINK     _bpftool
>>>>> 	make: *** No rule to make target 'skeleton/profiler.bpf.c', needed by 'skeleton/profiler.bpf.o'.  Stop.
>>>>
>>>> ok, I had to apply your patches by hand, because 'git am' refused to
>>>> due to fuzz.. so some of you new files did not make it to my tree ;-)
>>>>
>>>> anyway I hit another error now:
>>>>
>>>> 	  CC       prog.o
>>>> 	In file included from prog.c:1553:
>>>> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
>>>> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>>> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
>>>> 	      |                                   ^~
>>>> 	prog.c: In function ‘profile_read_values’:
>>>> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>>> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>>>>
>>>> I'll try to figure it out.. might be error on my end
>>>>
>>>> do you have git repo with these changes?
>>>
>>> I pushed it to 
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git/tree/?h=bpf-per-prog-stats
>>
>> still the same:
>>
>> 	[jolsa@dell-r440-01 bpftool]$ git show --oneline HEAD | head -1
>> 	7bbda5cca00a bpftool: fix typo in bash-completion
>> 	[jolsa@dell-r440-01 bpftool]$ make 
>> 	make[1]: Entering directory '/home/jolsa/linux-perf/tools/lib/bpf'
>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
>> 	  CC       prog.o
>> 	In file included from prog.c:1553:
>> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
>> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
>> 	      |                                   ^~
>> 	prog.c: In function ‘profile_read_values’:
>> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>> 	      |                             ^~
>> 	prog.c: In function ‘profile_open_perf_events’:
>> 	prog.c:1810:19: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
>> 	      |                   ^~
>> 	prog.c:1810:42: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
>> 	      |                                          ^~
>> 	prog.c:1825:26: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1825 |   for (cpu = 0; cpu < obj->rodata->num_cpu; cpu++) {
>> 	      |                          ^~
>> 	prog.c: In function ‘do_profile’:
>> 	prog.c:1904:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1904 |  profile_obj->rodata->num_cpu = num_cpu;
>> 	      |             ^~
>> 	prog.c:1905:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>> 	 1905 |  profile_obj->rodata->num_metric = num_metric;
>> 	      |             ^~
>> 	make: *** [Makefile:129: prog.o] Error 1
> 
> I guess you need a newer version of clang that supports global data in BPF programs. 
> 
> Thanks,
> Song
> 

Thinking about this requirement again... Do you think it would be worth
adding (as a follow-up) a feature check on the availability of clang
with global data support to bpftool's Makefile? So that we could compile
out program profiling if clang is not present or does not support it.
Just like libbfd support is optional already.

I'm asking mostly because a number of distributions now package bpftool,
and e.g. Ubuntu builds it from kernel source when creating its
linux-images and linux-tools-* packages. And I am pretty sure the build
environment does not have latest clang/LLVM, but it would be great to
remain able to build bpftool.

Best regards,
Quentin

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

* Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
  2020-03-09 18:04           ` Quentin Monnet
@ 2020-03-09 18:24             ` Song Liu
  2020-03-09 19:30               ` Jiri Olsa
  0 siblings, 1 reply; 20+ messages in thread
From: Song Liu @ 2020-03-09 18:24 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Jiri Olsa, Networking, bpf, Kernel Team, ast, daniel,
	arnaldo.melo, jolsa



> On Mar 9, 2020, at 11:04 AM, Quentin Monnet <quentin@isovalent.com> wrote:
> 
> 2020-03-04 21:39 UTC+0000 ~ Song Liu <songliubraving@fb.com>
>> 
>> 
>>> On Mar 4, 2020, at 1:29 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>>> 
>>> On Wed, Mar 04, 2020 at 09:16:29PM +0000, Song Liu wrote:
>>>> 
>>>> 
>>>>> On Mar 4, 2020, at 12:41 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>>>>> 
>>>>> On Wed, Mar 04, 2020 at 08:08:07PM +0100, Jiri Olsa wrote:
>>>>>> On Wed, Mar 04, 2020 at 10:07:06AM -0800, Song Liu wrote:
>>>>>>> This set introduces bpftool prog profile command, which uses hardware
>>>>>>> counters to profile BPF programs.
>>>>>>> 
>>>>>>> This command attaches fentry/fexit programs to a target program. These two
>>>>>>> programs read hardware counters before and after the target program and
>>>>>>> calculate the difference.
>>>>>>> 
>>>>>>> Changes v3 => v4:
>>>>>>> 1. Simplify err handling in profile_open_perf_events() (Quentin);
>>>>>>> 2. Remove redundant p_err() (Quentin);
>>>>>>> 3. Replace tab with space in bash-completion; (Quentin);
>>>>>>> 4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).
>>>>>> 
>>>>>> hum, I'm getting:
>>>>>> 
>>>>>> 	[jolsa@dell-r440-01 bpftool]$ pwd
>>>>>> 	/home/jolsa/linux-perf/tools/bpf/bpftool
>>>>>> 	[jolsa@dell-r440-01 bpftool]$ make
>>>>>> 	...
>>>>>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
>>>>>> 	  LINK     _bpftool
>>>>>> 	make: *** No rule to make target 'skeleton/profiler.bpf.c', needed by 'skeleton/profiler.bpf.o'.  Stop.
>>>>> 
>>>>> ok, I had to apply your patches by hand, because 'git am' refused to
>>>>> due to fuzz.. so some of you new files did not make it to my tree ;-)
>>>>> 
>>>>> anyway I hit another error now:
>>>>> 
>>>>> 	  CC       prog.o
>>>>> 	In file included from prog.c:1553:
>>>>> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
>>>>> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>>>> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
>>>>> 	      |                                   ^~
>>>>> 	prog.c: In function ‘profile_read_values’:
>>>>> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>>>> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>>>>> 
>>>>> I'll try to figure it out.. might be error on my end
>>>>> 
>>>>> do you have git repo with these changes?
>>>> 
>>>> I pushed it to 
>>>> 
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git/tree/?h=bpf-per-prog-stats
>>> 
>>> still the same:
>>> 
>>> 	[jolsa@dell-r440-01 bpftool]$ git show --oneline HEAD | head -1
>>> 	7bbda5cca00a bpftool: fix typo in bash-completion
>>> 	[jolsa@dell-r440-01 bpftool]$ make 
>>> 	make[1]: Entering directory '/home/jolsa/linux-perf/tools/lib/bpf'
>>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
>>> 	  CC       prog.o
>>> 	In file included from prog.c:1553:
>>> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
>>> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
>>> 	      |                                   ^~
>>> 	prog.c: In function ‘profile_read_values’:
>>> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>>> 	      |                             ^~
>>> 	prog.c: In function ‘profile_open_perf_events’:
>>> 	prog.c:1810:19: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
>>> 	      |                   ^~
>>> 	prog.c:1810:42: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
>>> 	      |                                          ^~
>>> 	prog.c:1825:26: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>> 	 1825 |   for (cpu = 0; cpu < obj->rodata->num_cpu; cpu++) {
>>> 	      |                          ^~
>>> 	prog.c: In function ‘do_profile’:
>>> 	prog.c:1904:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>> 	 1904 |  profile_obj->rodata->num_cpu = num_cpu;
>>> 	      |             ^~
>>> 	prog.c:1905:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
>>> 	 1905 |  profile_obj->rodata->num_metric = num_metric;
>>> 	      |             ^~
>>> 	make: *** [Makefile:129: prog.o] Error 1
>> 
>> I guess you need a newer version of clang that supports global data in BPF programs. 
>> 
>> Thanks,
>> Song
>> 
> 
> Thinking about this requirement again... Do you think it would be worth
> adding (as a follow-up) a feature check on the availability of clang
> with global data support to bpftool's Makefile? So that we could compile
> out program profiling if clang is not present or does not support it.
> Just like libbfd support is optional already.
> 
> I'm asking mostly because a number of distributions now package bpftool,
> and e.g. Ubuntu builds it from kernel source when creating its
> linux-images and linux-tools-* packages. And I am pretty sure the build
> environment does not have latest clang/LLVM, but it would be great to
> remain able to build bpftool.

Yeah, I think it is a good idea. Some more Makefile fun. ;)

Thanks,
Song


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

* Re: [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile
  2020-03-09 18:24             ` Song Liu
@ 2020-03-09 19:30               ` Jiri Olsa
  0 siblings, 0 replies; 20+ messages in thread
From: Jiri Olsa @ 2020-03-09 19:30 UTC (permalink / raw)
  To: Song Liu
  Cc: Quentin Monnet, Networking, bpf, Kernel Team, ast, daniel,
	arnaldo.melo, jolsa

On Mon, Mar 09, 2020 at 06:24:22PM +0000, Song Liu wrote:
> 
> 
> > On Mar 9, 2020, at 11:04 AM, Quentin Monnet <quentin@isovalent.com> wrote:
> > 
> > 2020-03-04 21:39 UTC+0000 ~ Song Liu <songliubraving@fb.com>
> >> 
> >> 
> >>> On Mar 4, 2020, at 1:29 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> >>> 
> >>> On Wed, Mar 04, 2020 at 09:16:29PM +0000, Song Liu wrote:
> >>>> 
> >>>> 
> >>>>> On Mar 4, 2020, at 12:41 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> >>>>> 
> >>>>> On Wed, Mar 04, 2020 at 08:08:07PM +0100, Jiri Olsa wrote:
> >>>>>> On Wed, Mar 04, 2020 at 10:07:06AM -0800, Song Liu wrote:
> >>>>>>> This set introduces bpftool prog profile command, which uses hardware
> >>>>>>> counters to profile BPF programs.
> >>>>>>> 
> >>>>>>> This command attaches fentry/fexit programs to a target program. These two
> >>>>>>> programs read hardware counters before and after the target program and
> >>>>>>> calculate the difference.
> >>>>>>> 
> >>>>>>> Changes v3 => v4:
> >>>>>>> 1. Simplify err handling in profile_open_perf_events() (Quentin);
> >>>>>>> 2. Remove redundant p_err() (Quentin);
> >>>>>>> 3. Replace tab with space in bash-completion; (Quentin);
> >>>>>>> 4. Fix typo _bpftool_get_map_names => _bpftool_get_prog_names (Quentin).
> >>>>>> 
> >>>>>> hum, I'm getting:
> >>>>>> 
> >>>>>> 	[jolsa@dell-r440-01 bpftool]$ pwd
> >>>>>> 	/home/jolsa/linux-perf/tools/bpf/bpftool
> >>>>>> 	[jolsa@dell-r440-01 bpftool]$ make
> >>>>>> 	...
> >>>>>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
> >>>>>> 	  LINK     _bpftool
> >>>>>> 	make: *** No rule to make target 'skeleton/profiler.bpf.c', needed by 'skeleton/profiler.bpf.o'.  Stop.
> >>>>> 
> >>>>> ok, I had to apply your patches by hand, because 'git am' refused to
> >>>>> due to fuzz.. so some of you new files did not make it to my tree ;-)
> >>>>> 
> >>>>> anyway I hit another error now:
> >>>>> 
> >>>>> 	  CC       prog.o
> >>>>> 	In file included from prog.c:1553:
> >>>>> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
> >>>>> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >>>>> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
> >>>>> 	      |                                   ^~
> >>>>> 	prog.c: In function ‘profile_read_values’:
> >>>>> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >>>>> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
> >>>>> 
> >>>>> I'll try to figure it out.. might be error on my end
> >>>>> 
> >>>>> do you have git repo with these changes?
> >>>> 
> >>>> I pushed it to 
> >>>> 
> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git/tree/?h=bpf-per-prog-stats
> >>> 
> >>> still the same:
> >>> 
> >>> 	[jolsa@dell-r440-01 bpftool]$ git show --oneline HEAD | head -1
> >>> 	7bbda5cca00a bpftool: fix typo in bash-completion
> >>> 	[jolsa@dell-r440-01 bpftool]$ make 
> >>> 	make[1]: Entering directory '/home/jolsa/linux-perf/tools/lib/bpf'
> >>> 	make[1]: Leaving directory '/home/jolsa/linux-perf/tools/lib/bpf'
> >>> 	  CC       prog.o
> >>> 	In file included from prog.c:1553:
> >>> 	profiler.skel.h: In function ‘profiler_bpf__create_skeleton’:
> >>> 	profiler.skel.h:136:35: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >>> 	  136 |  s->maps[4].mmaped = (void **)&obj->rodata;
> >>> 	      |                                   ^~
> >>> 	prog.c: In function ‘profile_read_values’:
> >>> 	prog.c:1650:29: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >>> 	 1650 |  __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
> >>> 	      |                             ^~
> >>> 	prog.c: In function ‘profile_open_perf_events’:
> >>> 	prog.c:1810:19: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >>> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
> >>> 	      |                   ^~
> >>> 	prog.c:1810:42: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >>> 	 1810 |   sizeof(int), obj->rodata->num_cpu * obj->rodata->num_metric);
> >>> 	      |                                          ^~
> >>> 	prog.c:1825:26: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >>> 	 1825 |   for (cpu = 0; cpu < obj->rodata->num_cpu; cpu++) {
> >>> 	      |                          ^~
> >>> 	prog.c: In function ‘do_profile’:
> >>> 	prog.c:1904:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >>> 	 1904 |  profile_obj->rodata->num_cpu = num_cpu;
> >>> 	      |             ^~
> >>> 	prog.c:1905:13: error: ‘struct profiler_bpf’ has no member named ‘rodata’
> >>> 	 1905 |  profile_obj->rodata->num_metric = num_metric;
> >>> 	      |             ^~
> >>> 	make: *** [Makefile:129: prog.o] Error 1
> >> 
> >> I guess you need a newer version of clang that supports global data in BPF programs. 
> >> 
> >> Thanks,
> >> Song
> >> 
> > 
> > Thinking about this requirement again... Do you think it would be worth
> > adding (as a follow-up) a feature check on the availability of clang
> > with global data support to bpftool's Makefile? So that we could compile
> > out program profiling if clang is not present or does not support it.
> > Just like libbfd support is optional already.
> > 
> > I'm asking mostly because a number of distributions now package bpftool,
> > and e.g. Ubuntu builds it from kernel source when creating its
> > linux-images and linux-tools-* packages. And I am pretty sure the build
> > environment does not have latest clang/LLVM, but it would be great to
> > remain able to build bpftool.
> 
> Yeah, I think it is a good idea. Some more Makefile fun. ;)

I think it's good idea, also bpftool is already using feature
detection from tools/build/features.. you can check commits like:

  fb982666e380 tools/bpftool: fix bpftool build with bintutils >= 2.9

for adding new feature detection

jirka


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

end of thread, other threads:[~2020-03-09 19:30 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-04 18:07 [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile Song Liu
2020-03-04 18:07 ` [PATCH v4 bpf-next 1/4] bpftool: introduce "prog profile" command Song Liu
2020-03-04 19:21   ` Jiri Olsa
2020-03-04 21:17     ` Song Liu
2020-03-04 20:38   ` Jiri Olsa
2020-03-05 20:03     ` Song Liu
2020-03-04 18:07 ` [PATCH v4 bpf-next 2/4] bpftool: Documentation for bpftool prog profile Song Liu
2020-03-04 18:07 ` [PATCH v4 bpf-next 3/4] bpftool: bash completion for "bpftool prog profile" Song Liu
2020-03-04 18:07 ` [PATCH v4 bpf-next 4/4] bpftool: fix typo in bash-completion Song Liu
2020-03-04 19:08 ` [PATCH v4 bpf-next 0/4] bpftool: introduce prog profile Jiri Olsa
2020-03-04 20:41   ` Jiri Olsa
2020-03-04 21:16     ` Song Liu
2020-03-04 21:29       ` Jiri Olsa
2020-03-04 21:39         ` Song Liu
2020-03-05 20:03           ` Song Liu
2020-03-06  8:51             ` Jiri Olsa
2020-03-09 18:04           ` Quentin Monnet
2020-03-09 18:24             ` Song Liu
2020-03-09 19:30               ` Jiri Olsa
2020-03-04 19:13 ` Quentin Monnet

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).