linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Blake Jones <blakejones@google.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Ian Rogers <irogers@google.com>, Jiri Olsa <jolsa@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Kan Liang <kan.liang@linux.intel.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Tomas Glozar <tglozar@redhat.com>,
	James Clark <james.clark@linaro.org>, Leo Yan <leo.yan@arm.com>,
	Guilherme Amadio <amadio@gentoo.org>,
	Yang Jihong <yangjihong@bytedance.com>,
	Charlie Jenkins <charlie@rivosinc.com>,
	Chun-Tse Shao <ctshao@google.com>,
	Aditya Gupta <adityag@linux.ibm.com>,
	Athira Rajeev <atrajeev@linux.vnet.ibm.com>,
	Zhongqiu Han <quic_zhonhan@quicinc.com>,
	Andi Kleen <ak@linux.intel.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Yujie Liu <yujie.liu@intel.com>,
	Graham Woodward <graham.woodward@arm.com>,
	Yicong Yang <yangyicong@hisilicon.com>,
	Ben Gainey <ben.gainey@arm.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	bpf@vger.kernel.org
Subject: Re: [PATCH v3 0/5] perf: generate events for BPF metadata
Date: Wed, 11 Jun 2025 11:29:26 -0700	[thread overview]
Message-ID: <aEnLBgCTuuZjeakP@google.com> (raw)
In-Reply-To: <20250606215246.2419387-1-blakejones@google.com>

Hi Blake,

On Fri, Jun 06, 2025 at 02:52:41PM -0700, Blake Jones wrote:
> Commit ffa915f46193 ("Merge branch 'bpf_metadata'"), from September 2020,
> added support to the kernel, libbpf, and bpftool to treat read-only BPF
> variables that have names starting with 'bpf_metadata_' specially. This
> patch series updates perf to handle these variables similarly, allowing a
> perf.data file to capture relevant information about BPF programs on the
> system being profiled.
> 
> When it encounters a BPF program, it reads the program's maps to find an
> '.rodata' map with 'bpf_metadata_' variables. If it finds one, it extracts
> their values as strings, and creates a new PERF_RECORD_BPF_METADATA
> synthetic event using that data. It does this both for BPF programs that
> were loaded when a 'perf record' starts, as well as for programs that are
> loaded while the profile is running. For the latter case, it stores the
> metadata for the duration of the profile, and then dumps it at the end of
> the profile, where it's in a better context to do so.
> 
> The PERF_RECORD_BPF_METADATA event holds an array of key-value pairs, where
> the key is the variable name (minus the "bpf_metadata_" prefix) and the
> value is the variable's value, formatted as a string. There is one such
> event generated for each BPF subprogram. Generating it per subprogram
> rather than per program allows it to be correlated with PERF_RECORD_KSYMBOL
> events; the metadata event's "prog_name" is designed to be identical to the
> "name" field of a perf_record_ksymbol. This allows specific BPF metadata to
> be associated with each BPF address range in the collection.
> 
> Changes:
> 
> * v2 -> v3:
>   - Split out event collection from event display.
>   - Resync with tmp.perf-tools-next.
>   - Link to v2:
>     https://lore.kernel.org/linux-perf-users/20250605233934.1881839-1-blakejones@google.com/T/#t
> 
> * v1 -> v2:
>   - Split out libbpf change and send it to the bpf tree.
>   - Add feature detection to perf to detect the libbpf change.
>   - Allow the feature to be skipped if the libbpf support is not found.
>   - Add an example of a PERF_RECORD_BPF_METADATA record.
>   - Change calloc() calls to zalloc().
>   - Don't check for NULL before calling free().
>   - Update the perf_event header when it is created, rather than
>     storing the event size and updating it later.
>   - Add a BPF metadata variable (with the perf version) to all
>     perf BPF programs.
>   - Update the selftest to look for the new perf_version variable.
>   - Split out the selftest into its own patch.
>   - Link to v1:
>     https://lore.kernel.org/linux-perf-users/20250521222725.3895192-1-blakejones@google.com/T/#t
> 
> Blake Jones (5):
>   perf: detect support for libbpf's emit_strings option
>   perf: collect BPF metadata from existing BPF programs
>   perf: collect BPF metadata from new programs
>   perf: display the new PERF_RECORD_BPF_METADATA event
>   perf: add test for PERF_RECORD_BPF_METADATA collection

I tried to process your patches but it failed to build like below:

  util/bpf-event.h: In function 'bpf_metadata_free':
  util/bpf-event.h:68:59: error: unused parameter 'metadata' [-Werror=unused-parameter]
     68 | static inline void bpf_metadata_free(struct bpf_metadata *metadata)
        |                                      ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~

It was a simple to fix by adding __maybe_unused but after that I got
another build error so I stopped.

  /usr/bin/ld: /tmp/tmp.N9VJQ2A3pl/perf-in.o: in function `cmd_record':
  (.text+0x191ae): undefined reference to `perf_event__synthesize_final_bpf_metadata'
  collect2: error: ld returned 1 exit status
  make[4]: *** [Makefile.perf:804: /tmp/tmp.N9VJQ2A3pl/perf] Error 1
  make[4]: *** Waiting for unfinished jobs....
  make[3]: *** [Makefile.perf:290: sub-make] Error 2
  make[2]: *** [Makefile:76: all] Error 2
  make[1]: *** [tests/make:341: make_no_libbpf_O] Error 1
  make: *** [Makefile:109: build-test] Error 2

Please run 'make build-test' and send v4.

Thanks,
Namhyung

> 
>  tools/build/Makefile.feature                |   1 +
>  tools/build/feature/Makefile                |   4 +
>  tools/build/feature/test-libbpf-strings.c   |  10 +
>  tools/lib/perf/include/perf/event.h         |  18 +
>  tools/perf/Documentation/perf-check.txt     |   1 +
>  tools/perf/Makefile.config                  |  12 +
>  tools/perf/Makefile.perf                    |   3 +-
>  tools/perf/builtin-check.c                  |   1 +
>  tools/perf/builtin-inject.c                 |   1 +
>  tools/perf/builtin-record.c                 |   8 +
>  tools/perf/builtin-script.c                 |  15 +-
>  tools/perf/tests/shell/test_bpf_metadata.sh |  76 ++++
>  tools/perf/util/bpf-event.c                 | 378 ++++++++++++++++++++
>  tools/perf/util/bpf-event.h                 |  13 +
>  tools/perf/util/bpf_skel/perf_version.h     |  17 +
>  tools/perf/util/env.c                       |  19 +-
>  tools/perf/util/env.h                       |   4 +
>  tools/perf/util/event.c                     |  21 ++
>  tools/perf/util/event.h                     |   1 +
>  tools/perf/util/header.c                    |   1 +
>  tools/perf/util/session.c                   |   4 +
>  tools/perf/util/synthetic-events.h          |   2 +
>  tools/perf/util/tool.c                      |  14 +
>  tools/perf/util/tool.h                      |   3 +-
>  24 files changed, 622 insertions(+), 5 deletions(-)
>  create mode 100644 tools/build/feature/test-libbpf-strings.c
>  create mode 100755 tools/perf/tests/shell/test_bpf_metadata.sh
>  create mode 100644 tools/perf/util/bpf_skel/perf_version.h
> 
> -- 
> 2.50.0.rc0.604.gd4ff7b7c86-goog
> 

  parent reply	other threads:[~2025-06-11 18:29 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-06 21:52 [PATCH v3 0/5] perf: generate events for BPF metadata Blake Jones
2025-06-06 21:52 ` [PATCH v3 1/5] perf: detect support for libbpf's emit_strings option Blake Jones
2025-06-06 21:52 ` [PATCH v3 2/5] perf: collect BPF metadata from existing BPF programs Blake Jones
2025-06-06 21:52 ` [PATCH v3 3/5] perf: collect BPF metadata from new programs Blake Jones
2025-06-06 21:52 ` [PATCH v3 4/5] perf: display the new PERF_RECORD_BPF_METADATA event Blake Jones
2025-06-06 21:52 ` [PATCH v3 5/5] perf: add test for PERF_RECORD_BPF_METADATA collection Blake Jones
2025-06-11 18:29 ` Namhyung Kim [this message]
2025-06-12  0:39   ` [PATCH v3 0/5] perf: generate events for BPF metadata Blake Jones
2025-06-12  1:02     ` Blake Jones
2025-06-20 21:41       ` Namhyung Kim
2025-06-12  5:19     ` Ian Rogers
2025-06-12 19:45       ` Blake Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aEnLBgCTuuZjeakP@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adityag@linux.ibm.com \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=amadio@gentoo.org \
    --cc=atrajeev@linux.vnet.ibm.com \
    --cc=ben.gainey@arm.com \
    --cc=blakejones@google.com \
    --cc=bpf@vger.kernel.org \
    --cc=charlie@rivosinc.com \
    --cc=ctshao@google.com \
    --cc=dvyukov@google.com \
    --cc=graham.woodward@arm.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=leo.yan@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=quic_zhonhan@quicinc.com \
    --cc=rostedt@goodmis.org \
    --cc=tglozar@redhat.com \
    --cc=yangjihong@bytedance.com \
    --cc=yangyicong@hisilicon.com \
    --cc=yujie.liu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).