All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.