Message-ID: <20250120185033.GA261349@e132581.arm.com>
Date: Mon, 20 Jan 2025 18:50:33 +0000
From: Leo Yan <leo.yan@....com>
To: Daniel Borkmann <daniel@...earbox.net>
Cc: Alexei Starovoitov <ast@...nel.org>,
	Andrii Nakryiko <andrii@...nel.org>,
	Martin KaFai Lau <martin.lau@...ux.dev>,
	Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
	Yonghong Song <yonghong.song@...ux.dev>,
	John Fastabend <john.fastabend@...il.com>,
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
	Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>,
	Jesper Dangaard Brouer <hawk@...nel.org>,
	James Clark <james.clark@...aro.org>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>, Ian Rogers <irogers@...gle.com>,
	linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
	Quentin Monnet <qmo@...nel.org>
Subject: Re: [PATCH v1] samples/bpf: Add a trace tool with perf PMU counters

Hi Daniel,

On Mon, Jan 20, 2025 at 05:18:23PM +0100, Daniel Borkmann wrote:
> 
> Hi Leo,
> 
> On 1/19/25 4:33 PM, Leo Yan wrote:
> > Developers might need to profile a program with fine-grained
> > granularity.  E.g., a user case is to account the CPU cycles for a small
> > program or for a specific function within the program.
> > 
> > This commit introduces a small tool with using eBPF program to read the
> > perf PMU counters for performance metrics.  As the first step, the four
> > counters are supported with the '-e' option: cycles, instructions,
> > branches, branch-misses.
> > 
> > The '-r' option is provided for support raw event number.  This option
> > is mutually exclusive to the '-e' option, users either pass a raw event
> > number or a counter name.
> > 
> > The tool enables the counters for the entire trace session in free-run
> > mode.  It reads the beginning values for counters when the profiled
> > program is scheduled in, and calculate the interval when the task is
> > scheduled out.  The advantage of this approach is to dismiss the
> > statistics noise (e.g. caused by the tool itself) as possible.

[...]

> Thanks for this work! Few suggestions.. the contents of samples/bpf/ are in process of being
> migrated into BPF selftests given they have been bit-rotting for quite some time, so we'd like
> to migrate missing coverage into BPF CI (see test_progs in tools/testing/selftests/bpf/). That
> could be one option, or an alternative is to extend bpftool for profiling BPF programs (see
> 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")).

Thanks for suggestions!

The naming or info in the commit log might cause confuse.  The purpose
of this patch is to measure PMU counters for normal programs (not BPF
program specific).  To achieve profiling accuracy, it opens perf event
in thread mode, and support function based trace and can be userspace
mode only.

My understanding for bpftool is for eBPF program specific.  I looked
into a bit the commit 47c09d6a9f67, it is nature for integrating the
tracing feature for eBPF program specific.  My patch is for tracing
normal userspace programs, I am not sure if this is really wanted by
bpftool.  I would like to hear opinions from bpftool maintainer before
proceeding.

My program mainly uses eBPF attaching to uprobe.  selftest/bpf has
contained the related functionality testing, actually I refered the
test for writing my code :).  So maybe it is not quite useful for
merging the code as a test?

If both options are not ideal, I would spend time to land the
feature in perf tool - the perf tool has supported eBPF backend for
reading PMU counters, but it is absent function based profiling.

Thanks,
Leo

Powered by blists - more mailing lists