By following the same protocol, other tools can share hardware PMCs with
perf. Move perf_event_attr_map_entry and BPERF_DEFAULT_ATTR_MAP_PATH to
bperf.h for other tools to use.
Also add bperf_attr_map_compatible() to check whether existing attr_map
is compatible with current perf binary.
Signed-off-by: Song Liu <[email protected]>
---
tools/lib/perf/include/perf/bperf.h | 29 +++++++++++++++++++
tools/perf/util/bpf_counter.c | 44 ++++++++++++++---------------
2 files changed, 50 insertions(+), 23 deletions(-)
create mode 100644 tools/lib/perf/include/perf/bperf.h
diff --git a/tools/lib/perf/include/perf/bperf.h b/tools/lib/perf/include/perf/bperf.h
new file mode 100644
index 0000000000000..02b2fd5e50c75
--- /dev/null
+++ b/tools/lib/perf/include/perf/bperf.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
+#ifndef __LIBPERF_BPERF_H
+#define __LIBPERF_BPERF_H
+
+/*
+ * bperf uses a hashmap, the attr_map, to track all the leader programs.
+ * The hashmap is pinned in bpffs. flock() on this file is used to ensure
+ * no concurrent access to the attr_map. The key of attr_map is struct
+ * perf_event_attr, and the value is struct perf_event_attr_map_entry.
+ *
+ * struct perf_event_attr_map_entry contains two __u32 IDs, bpf_link of the
+ * leader prog, and the diff_map. Each perf-stat session holds a reference
+ * to the bpf_link to make sure the leader prog is attached to sched_switch
+ * tracepoint.
+ *
+ * Since the hashmap only contains IDs of the bpf_link and diff_map, it
+ * does not hold any references to the leader program. Once all perf-stat
+ * sessions of these events exit, the leader prog, its maps, and the
+ * perf_events will be freed.
+ */
+struct perf_event_attr_map_entry {
+ __u32 link_id;
+ __u32 diff_map_id;
+};
+
+/* pin the map at sysfs__mountpoint()/BPERF_DEFAULT_ATTR_MAP_PATH */
+#define BPERF_DEFAULT_ATTR_MAP_PATH "fs/bpf/perf_attr_map"
+
+#endif /* __LIBPERF_BPERF_H */
diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
index 81d1df3c4ec0e..df70c8dcf7850 100644
--- a/tools/perf/util/bpf_counter.c
+++ b/tools/perf/util/bpf_counter.c
@@ -14,6 +14,7 @@
#include <bpf/btf.h>
#include <bpf/libbpf.h>
#include <api/fs/fs.h>
+#include <perf/bperf.h>
#include "bpf_counter.h"
#include "counts.h"
@@ -29,28 +30,6 @@
#include "bpf_skel/bperf_leader.skel.h"
#include "bpf_skel/bperf_follower.skel.h"
-/*
- * bperf uses a hashmap, the attr_map, to track all the leader programs.
- * The hashmap is pinned in bpffs. flock() on this file is used to ensure
- * no concurrent access to the attr_map. The key of attr_map is struct
- * perf_event_attr, and the value is struct perf_event_attr_map_entry.
- *
- * struct perf_event_attr_map_entry contains two __u32 IDs, bpf_link of the
- * leader prog, and the diff_map. Each perf-stat session holds a reference
- * to the bpf_link to make sure the leader prog is attached to sched_switch
- * tracepoint.
- *
- * Since the hashmap only contains IDs of the bpf_link and diff_map, it
- * does not hold any references to the leader program. Once all perf-stat
- * sessions of these events exit, the leader prog, its maps, and the
- * perf_events will be freed.
- */
-struct perf_event_attr_map_entry {
- __u32 link_id;
- __u32 diff_map_id;
-};
-
-#define DEFAULT_ATTR_MAP_PATH "fs/bpf/perf_attr_map"
#define ATTR_MAP_SIZE 16
static inline void *u64_to_ptr(__u64 ptr)
@@ -333,6 +312,20 @@ static __u32 bpf_map_get_id(int fd)
return map_info.id;
}
+static bool bperf_attr_map_compatible(int attr_map_fd)
+{
+ struct bpf_map_info map_info = {0};
+ __u32 map_info_len = sizeof(map_info);
+ int err;
+
+ err = bpf_obj_get_info_by_fd(attr_map_fd, &map_info, &map_info_len);
+
+ if (err)
+ return false;
+ return (map_info.key_size == sizeof(struct perf_event_attr)) &&
+ (map_info.value_size == sizeof(struct perf_event_attr_map_entry));
+}
+
static int bperf_lock_attr_map(struct target *target)
{
char path[PATH_MAX];
@@ -342,7 +335,7 @@ static int bperf_lock_attr_map(struct target *target)
scnprintf(path, PATH_MAX, "%s", target->attr_map);
} else {
scnprintf(path, PATH_MAX, "%s/%s", sysfs__mountpoint(),
- DEFAULT_ATTR_MAP_PATH);
+ BPERF_DEFAULT_ATTR_MAP_PATH);
}
if (access(path, F_OK)) {
@@ -367,6 +360,11 @@ static int bperf_lock_attr_map(struct target *target)
return -1;
}
+ if (!bperf_attr_map_compatible(map_fd)) {
+ close(map_fd);
+ return -1;
+
+ }
err = flock(map_fd, LOCK_EX);
if (err) {
close(map_fd);
--
2.30.2
On Fri, Apr 02, 2021 at 05:29:37PM -0700, Song Liu wrote:
> By following the same protocol, other tools can share hardware PMCs with
> perf. Move perf_event_attr_map_entry and BPERF_DEFAULT_ATTR_MAP_PATH to
> bperf.h for other tools to use.
hi,
so is this necessary for some other tool now?
>
> Also add bperf_attr_map_compatible() to check whether existing attr_map
> is compatible with current perf binary.
please separate this change
>
> Signed-off-by: Song Liu <[email protected]>
> ---
> tools/lib/perf/include/perf/bperf.h | 29 +++++++++++++++++++
> tools/perf/util/bpf_counter.c | 44 ++++++++++++++---------------
> 2 files changed, 50 insertions(+), 23 deletions(-)
> create mode 100644 tools/lib/perf/include/perf/bperf.h
>
> diff --git a/tools/lib/perf/include/perf/bperf.h b/tools/lib/perf/include/perf/bperf.h
> new file mode 100644
> index 0000000000000..02b2fd5e50c75
> --- /dev/null
> +++ b/tools/lib/perf/include/perf/bperf.h
I wonder we want to call it bpf_perf.h to be more generic?
or best just bpf.h ... but that might give us some conflict
headache in future ;-)
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
> +#ifndef __LIBPERF_BPERF_H
> +#define __LIBPERF_BPERF_H
> +
> +/*
> + * bperf uses a hashmap, the attr_map, to track all the leader programs.
> + * The hashmap is pinned in bpffs. flock() on this file is used to ensure
> + * no concurrent access to the attr_map. The key of attr_map is struct
> + * perf_event_attr, and the value is struct perf_event_attr_map_entry.
> + *
> + * struct perf_event_attr_map_entry contains two __u32 IDs, bpf_link of the
> + * leader prog, and the diff_map. Each perf-stat session holds a reference
> + * to the bpf_link to make sure the leader prog is attached to sched_switch
> + * tracepoint.
> + *
> + * Since the hashmap only contains IDs of the bpf_link and diff_map, it
> + * does not hold any references to the leader program. Once all perf-stat
> + * sessions of these events exit, the leader prog, its maps, and the
> + * perf_events will be freed.
> + */
> +struct perf_event_attr_map_entry {
> + __u32 link_id;
> + __u32 diff_map_id;
> +};
this header file should be self contained,
so you need __u32 definitions
> +
> +/* pin the map at sysfs__mountpoint()/BPERF_DEFAULT_ATTR_MAP_PATH */
> +#define BPERF_DEFAULT_ATTR_MAP_PATH "fs/bpf/perf_attr_map"
if we are going to expose this, I think we should expose just
"perf_attr_map" ... without the 'fs/bpf' part, because that
could be mounted anywhere
thanks,
jirka
> +
> +#endif /* __LIBPERF_BPERF_H */
> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
> index 81d1df3c4ec0e..df70c8dcf7850 100644
> --- a/tools/perf/util/bpf_counter.c
> +++ b/tools/perf/util/bpf_counter.c
> @@ -14,6 +14,7 @@
> #include <bpf/btf.h>
> #include <bpf/libbpf.h>
> #include <api/fs/fs.h>
> +#include <perf/bperf.h>
>
> #include "bpf_counter.h"
> #include "counts.h"
> @@ -29,28 +30,6 @@
> #include "bpf_skel/bperf_leader.skel.h"
> #include "bpf_skel/bperf_follower.skel.h"
>
> -/*
> - * bperf uses a hashmap, the attr_map, to track all the leader programs.
> - * The hashmap is pinned in bpffs. flock() on this file is used to ensure
> - * no concurrent access to the attr_map. The key of attr_map is struct
> - * perf_event_attr, and the value is struct perf_event_attr_map_entry.
> - *
> - * struct perf_event_attr_map_entry contains two __u32 IDs, bpf_link of the
> - * leader prog, and the diff_map. Each perf-stat session holds a reference
> - * to the bpf_link to make sure the leader prog is attached to sched_switch
> - * tracepoint.
> - *
> - * Since the hashmap only contains IDs of the bpf_link and diff_map, it
> - * does not hold any references to the leader program. Once all perf-stat
> - * sessions of these events exit, the leader prog, its maps, and the
> - * perf_events will be freed.
> - */
> -struct perf_event_attr_map_entry {
> - __u32 link_id;
> - __u32 diff_map_id;
> -};
> -
> -#define DEFAULT_ATTR_MAP_PATH "fs/bpf/perf_attr_map"
> #define ATTR_MAP_SIZE 16
>
> static inline void *u64_to_ptr(__u64 ptr)
> @@ -333,6 +312,20 @@ static __u32 bpf_map_get_id(int fd)
> return map_info.id;
> }
>
> +static bool bperf_attr_map_compatible(int attr_map_fd)
> +{
> + struct bpf_map_info map_info = {0};
> + __u32 map_info_len = sizeof(map_info);
> + int err;
> +
> + err = bpf_obj_get_info_by_fd(attr_map_fd, &map_info, &map_info_len);
> +
> + if (err)
> + return false;
> + return (map_info.key_size == sizeof(struct perf_event_attr)) &&
> + (map_info.value_size == sizeof(struct perf_event_attr_map_entry));
> +}
> +
> static int bperf_lock_attr_map(struct target *target)
> {
> char path[PATH_MAX];
> @@ -342,7 +335,7 @@ static int bperf_lock_attr_map(struct target *target)
> scnprintf(path, PATH_MAX, "%s", target->attr_map);
> } else {
> scnprintf(path, PATH_MAX, "%s/%s", sysfs__mountpoint(),
> - DEFAULT_ATTR_MAP_PATH);
> + BPERF_DEFAULT_ATTR_MAP_PATH);
> }
>
> if (access(path, F_OK)) {
> @@ -367,6 +360,11 @@ static int bperf_lock_attr_map(struct target *target)
> return -1;
> }
>
> + if (!bperf_attr_map_compatible(map_fd)) {
> + close(map_fd);
> + return -1;
> +
> + }
> err = flock(map_fd, LOCK_EX);
> if (err) {
> close(map_fd);
> --
> 2.30.2
>
> On Apr 6, 2021, at 7:21 AM, Jiri Olsa <[email protected]> wrote:
>
> On Fri, Apr 02, 2021 at 05:29:37PM -0700, Song Liu wrote:
>> By following the same protocol, other tools can share hardware PMCs with
>> perf. Move perf_event_attr_map_entry and BPERF_DEFAULT_ATTR_MAP_PATH to
>> bperf.h for other tools to use.
>
> hi,
> so is this necessary for some other tool now?
We have monitoring tools do perf_event_open(). I would like to migrate these
to bperf.
>
>>
>> Also add bperf_attr_map_compatible() to check whether existing attr_map
>> is compatible with current perf binary.
>
> please separate this change
Will do.
>
>>
>> Signed-off-by: Song Liu <[email protected]>
>> ---
>> tools/lib/perf/include/perf/bperf.h | 29 +++++++++++++++++++
>> tools/perf/util/bpf_counter.c | 44 ++++++++++++++---------------
>> 2 files changed, 50 insertions(+), 23 deletions(-)
>> create mode 100644 tools/lib/perf/include/perf/bperf.h
>>
>> diff --git a/tools/lib/perf/include/perf/bperf.h b/tools/lib/perf/include/perf/bperf.h
>> new file mode 100644
>> index 0000000000000..02b2fd5e50c75
>> --- /dev/null
>> +++ b/tools/lib/perf/include/perf/bperf.h
>
> I wonder we want to call it bpf_perf.h to be more generic?
> or best just bpf.h ... but that might give us some conflict
> headache in future ;-)
I would rather avoid bpf.h... I am ok with bpf_perf.h or bperf.h
>
>> @@ -0,0 +1,29 @@
>> +/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
>> +#ifndef __LIBPERF_BPERF_H
>> +#define __LIBPERF_BPERF_H
>> +
>> +/*
>> + * bperf uses a hashmap, the attr_map, to track all the leader programs.
>> + * The hashmap is pinned in bpffs. flock() on this file is used to ensure
>> + * no concurrent access to the attr_map. The key of attr_map is struct
>> + * perf_event_attr, and the value is struct perf_event_attr_map_entry.
>> + *
>> + * struct perf_event_attr_map_entry contains two __u32 IDs, bpf_link of the
>> + * leader prog, and the diff_map. Each perf-stat session holds a reference
>> + * to the bpf_link to make sure the leader prog is attached to sched_switch
>> + * tracepoint.
>> + *
>> + * Since the hashmap only contains IDs of the bpf_link and diff_map, it
>> + * does not hold any references to the leader program. Once all perf-stat
>> + * sessions of these events exit, the leader prog, its maps, and the
>> + * perf_events will be freed.
>> + */
>> +struct perf_event_attr_map_entry {
>> + __u32 link_id;
>> + __u32 diff_map_id;
>> +};
>
> this header file should be self contained,
> so you need __u32 definitions
Will add.
>
>
>> +
>> +/* pin the map at sysfs__mountpoint()/BPERF_DEFAULT_ATTR_MAP_PATH */
>> +#define BPERF_DEFAULT_ATTR_MAP_PATH "fs/bpf/perf_attr_map"
>
> if we are going to expose this, I think we should expose just
> "perf_attr_map" ... without the 'fs/bpf' part, because that
> could be mounted anywhere
Will fix this.
Thanks,
Song