MIME-Version: 1.0
References: <20230316170149.4106586-1-jolsa@kernel.org> <ZBNTMZjEoETU9d8N@casper.infradead.org>
 <CAP-5=fVYriALLwF2FU1ZUtLuHndnvPw=3SctVqY6Uwex8JfscA@mail.gmail.com>
 <CAEf4BzYgyGTVv=cDwaW+DBke1uk_aLCg3CB_9W6+9tkS8Nyn_Q@mail.gmail.com> <ZBPjs1b8crUv4ur6@casper.infradead.org>
In-Reply-To: <ZBPjs1b8crUv4ur6@casper.infradead.org>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Fri, 17 Mar 2023 09:33:17 -0700
Message-ID: <CAEf4BzbPa-5b9uU0+GN=iaMGc6otje3iNQd+MOg_byTSYU8fEQ@mail.gmail.com>
Subject: Re: [PATCHv3 bpf-next 0/9] mm/bpf/perf: Store build id in file object
To:     Matthew Wilcox <willy@infradead.org>
Cc:     Ian Rogers <irogers@google.com>, Jiri Olsa <jolsa@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Andrii Nakryiko <andrii@kernel.org>,
        Hao Luo <haoluo@google.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        bpf@vger.kernel.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-perf-users@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@chromium.org>,
        Stanislav Fomichev <sdf@google.com>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Namhyung Kim <namhyung@gmail.com>,
        Dave Chinner <david@fromorbit.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Mar 16, 2023 at 8:51=E2=80=AFPM Matthew Wilcox <willy@infradead.org=
> wrote:
>
> On Thu, Mar 16, 2023 at 02:51:52PM -0700, Andrii Nakryiko wrote:
> > Yep, Meta is also capturing stack traces with build ID as well, if
> > possible. Build IDs help with profiling short-lived processes which
> > exit before the profiling session is done and user-space tooling is
> > able to collect /proc/<pid>/maps contents (which is what Ian is
> > referring to here). But also build ID allows to offload more of the
> > expensive stack symbolization process (converting raw memory addresses
> > into human readable function+offset+file path+line numbers
> > information) to dedicated remote servers, by allowing to cache and
> > reuse preprocessed DWARF/ELF information based on build ID.
> >
> > I believe perf tool is also using build ID, so any tool relying on
> > perf capturing full and complete profiling data for system-wide
> > performance analysis would benefit as well.
> >
> > Generally speaking, there is a whole ecosystem built on top of
> > assumption that binaries have build ID and profiling tooling is able
> > to provide more value if those build IDs are more reliably collected.
> > Which ultimately benefits the entire open-source ecosystem by allowing
> > people to spot issues (not necessarily just performance, it could be
> > correctness issues as well) more reliably, fix them, and benefit every
> > user.
>
> But build IDs are _generally_ available.  The only problem (AIUI)
> is when you're trying to examine the contents of one container from
> another container.  And to solve that problem, you're imposing a cost
> on everybody else with (so far) pretty vague justifications.  I really
> don't like to see you growing struct file for this (nor struct inode,
> nor struct vm_area_struct).  It's all quite unsatisfactory and I don't
> have a good suggestion.

There is a lot of profiling, observability and debugging tooling built
using BPF. And when capturing stack traces from BPF programs, if the
build ID note is not physically present in memory, fetching it from
the BPF program might fail in NMI (and other non-faultable contexts).
This patch set is about making sure we always can fetch build ID, even
from most restrictive environments. It's guarded by Kconfig to avoid
adding 8 bytes of overhead to struct file for environment where this
might be unacceptable, giving users and distros a choice.