Re: Building perf with BUILD_BPF_SKEL=1 by default

Song Liu
2021-12-06T22:34:36Z

> On Dec 6, 2021, at 12:00 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Mon, Dec 06, 2021 at 05:08:21PM +0000, Song Liu escreveu:
>>
>>
>>> On Dec 6, 2021, at 4:32 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>>>

[...]

>>
>> Please see the following commit log.
>
> This is for the other one, where I stamped this:
>
> commit ce71038e673ee8291c64631359e56c48c8616dc7 (HEAD -> perf/urgent)
> Author: Song Liu <songliubraving@fb.com>
> Date: Fri Dec 3 19:32:34 2021 +0000
>
> perf bpf: Fix building perf with BUILD_BPF_SKEL=1 by default in more distros
>
> Arnaldo reported that building all his containers with BUILD_BPF_SKEL=1
> to then make this the default he found problems in some distros where
> the system linux/bpf.h file was being used and lacked this:
>
> util/bpf_skel/bperf_leader.bpf.c:13:20: error: use of undeclared identifier 'BPF_F_PRESERVE_ELEMS'
> __uint(map_flags, BPF_F_PRESERVE_ELEMS);
>
> So use instead the vmlinux.h file generated by bpftool from BTF info.
>
> This fixed these as well, getting the build back working on debian:11,
> debian:experimental and ubuntu:21.10:
>
> In file included from In file included from util/bpf_skel/bperf_leader.bpf.cutil/bpf_skel/bpf_prog_profiler.bpf.c::33:
> :
> In file included from In file included from /usr/include/linux/bpf.h/usr/include/linux/bpf.h::1111:
> :
> /usr/include/linux/types.h/usr/include/linux/types.h::55::1010:: In file included from util/bpf_skel/bperf_follower.bpf.c:3fatal errorfatal error:
> : : In file included from /usr/include/linux/bpf.h:'asm/types.h' file not found11'asm/types.h' file not found:
>
> /usr/include/linux/types.h:5:10: fatal error: 'asm/types.h' file not found
> #include <asm/types.h>#include <asm/types.h>
>
> ^~~~~~~~~~~~~ ^~~~~~~~~~~~~
>
> #include <asm/types.h>
> ^~~~~~~~~~~~~
> 1 error generated.
>
> Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Signed-off-by: Song Liu <song@kernel.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Link: http://lore.kernel.org/lkml/CF175681-8101-43D1-ABDB-449E644BE986@fb.com
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> For this patch:
>
> diff --git a/tools/perf/util/bpf_skel/bperf_follower.bpf.c b/tools/perf/util/bpf_skel/bperf_follower.bpf.c
> index b8fa3cb2da230803..4a6acfde14937704 100644
> --- a/tools/perf/util/bpf_skel/bperf_follower.bpf.c
> +++ b/tools/perf/util/bpf_skel/bperf_follower.bpf.c
> @@ -1,7 +1,6 @@
> // SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> // Copyright (c) 2021 Facebook
> -#include <linux/bpf.h>
> -#include <linux/perf_event.h>
> +#include "vmlinux.h"
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
> #include "bperf.h"
> diff --git a/tools/perf/util/bpf_skel/bperf_leader.bpf.c b/tools/perf/util/bpf_skel/bperf_leader.bpf.c
> index 4f70d1459e86cb99..40d962b05863421e 100644
> --- a/tools/perf/util/bpf_skel/bperf_leader.bpf.c
> +++ b/tools/perf/util/bpf_skel/bperf_leader.bpf.c
> @@ -1,7 +1,6 @@
> // SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> // Copyright (c) 2021 Facebook
> -#include <linux/bpf.h>
> -#include <linux/perf_event.h>
> +#include "vmlinux.h"
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
> #include "bperf.h"
> diff --git a/tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c b/tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c
> index ab12b4c4ece21a9a..97037d3b3d9fa4cd 100644
> --- a/tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c
> +++ b/tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> // Copyright (c) 2020 Facebook
> -#include <linux/bpf.h>
> +#include "vmlinux.h"
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
>
>
> But what I asked now was a commit log message for this other one:

Sorry for the confusion. Please use this one, which also removes
bperf.h (which holds the typedef).

Thanks,
Song

====================================== 8< ========================================

From e033fd36657288ece383ba528aaff7b56eebfee2 Mon Sep 17 00:00:00 2001
From: Song Liu <songliubraving@fb.com>
Date: Fri, 3 Dec 2021 15:14:41 -0800
Subject: [PATCH] perf/bpf_skel: do not use typedef to avoid error on old clang

When building bpf_skel with clang-10, typedef causes confusions like:

libbpf: map 'prev_readings': unexpected def kind var.

Fix this by removing the typedef.

Fixes: 7fac83aaf2ee ("perf stat: Introduce 'bperf' to share hardware PMCs with BPF")
Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/perf/util/bpf_skel/bperf.h | 14 --------------
tools/perf/util/bpf_skel/bperf_follower.bpf.c | 16 +++++++++++++---
tools/perf/util/bpf_skel/bperf_leader.bpf.c | 16 +++++++++++++---
3 files changed, 26 insertions(+), 20 deletions(-)
delete mode 100644 tools/perf/util/bpf_skel/bperf.h

diff --git a/tools/perf/util/bpf_skel/bperf.h b/tools/perf/util/bpf_skel/bperf.h
deleted file mode 100644
index 186a5551ddb9d..0000000000000
--- a/tools/perf/util/bpf_skel/bperf.h
+++ /dev/null
@@ -1,14 +0,0 @@
-// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
-// Copyright (c) 2021 Facebook
-
-#ifndef __BPERF_STAT_H
-#define __BPERF_STAT_H
-
-typedef 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);
-} reading_map;
-
-#endif /* __BPERF_STAT_H */
diff --git a/tools/perf/util/bpf_skel/bperf_follower.bpf.c b/tools/perf/util/bpf_skel/bperf_follower.bpf.c
index 4a6acfde14937..f193998530d43 100644
--- a/tools/perf/util/bpf_skel/bperf_follower.bpf.c
+++ b/tools/perf/util/bpf_skel/bperf_follower.bpf.c
@@ -3,11 +3,21 @@
#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
-#include "bperf.h"
#include "bperf_u.h"

-reading_map diff_readings SEC(".maps");
-reading_map accum_readings SEC(".maps");
+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);
+} diff_readings SEC(".maps");
+
+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");

struct {
__uint(type, BPF_MAP_TYPE_HASH);
diff --git a/tools/perf/util/bpf_skel/bperf_leader.bpf.c b/tools/perf/util/bpf_skel/bperf_leader.bpf.c
index 40d962b058634..e2a2d4cd7779c 100644
--- a/tools/perf/util/bpf_skel/bperf_leader.bpf.c
+++ b/tools/perf/util/bpf_skel/bperf_leader.bpf.c
@@ -3,7 +3,6 @@
#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
-#include "bperf.h"

struct {
__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
@@ -12,8 +11,19 @@ struct {
__uint(map_flags, BPF_F_PRESERVE_ELEMS);
} events SEC(".maps");

-reading_map prev_readings SEC(".maps");
-reading_map diff_readings SEC(".maps");
+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);
+} prev_readings SEC(".maps");
+
+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);
+} diff_readings SEC(".maps");

SEC("raw_tp/sched_switch")
int BPF_PROG(on_switch)
--
2.30.2