Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 27 Jun 2021 19:25:35 +0200
From:   Jiri Olsa <jolsa@redhat.com>
To:     Namhyung Kim <namhyung@kernel.org>
Cc:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Ian Rogers <irogers@google.com>,
        lkml <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Ingo Molnar <mingo@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Michael Petlan <mpetlan@redhat.com>,
        Riccardo Mancini <rickyman7@gmail.com>
Subject: Re: [RFC 00/10] perf: Add build id parsing fault detection/fix
Message-ID: <YNi0j8nhh8X6j/ZE@krava>
References: <20210622153918.688500-1-jolsa@kernel.org>
 <YNIgXkH1xaF7H3Tr@kernel.org>
 <CAP-5=fU=AAJ0_s1orsF=OCO0=bSmr9BhAmtN251bU_pf0ZFJ6Q@mail.gmail.com>
 <YNIobHgfVbiiNscn@kernel.org>
 <YNJT0wvghSZzPJ8Y@krava>
 <CAM9d7chs=4mdRG389G8eDzeKmvB7PDuWp7mxd_ScuFiy0hYt3g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAM9d7chs=4mdRG389G8eDzeKmvB7PDuWp7mxd_ScuFiy0hYt3g@mail.gmail.com>
Precedence: bulk

On Wed, Jun 23, 2021 at 12:48:30PM -0700, Namhyung Kim wrote:
> Hi Jiri,
> 
> Thanks for your work!
> 
> On Tue, Jun 22, 2021 at 2:19 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Tue, Jun 22, 2021 at 03:14:04PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Jun 22, 2021 at 10:47:54AM -0700, Ian Rogers escreveu:
> > > > On Tue, Jun 22, 2021 at 10:39 AM Arnaldo Carvalho de Melo
> > > > <acme@kernel.org> wrote:
> > > > >
> > > > > Em Tue, Jun 22, 2021 at 05:39:08PM +0200, Jiri Olsa escreveu:
> > > > > > hi,
> > > > > > this *RFC* patchset adds support to detect faults during
> > > > > > mmap2's build id parsing and a way to fix such maps in
> > > > > > generated perf.data.
> > > > > >
> > > > > > It adds support to record build id faults count for session
> > > > > > and store it in perf.data and perf inject support to find
> > > > > > these maps and reads build ids for them in user space.
> > > > >
> > > > > > It's probably best explained by the workflow:
> > > > > >
> > > > > >   Record data with --buildid-mmap option:
> > > > > >
> > > > > >     # perf record --buildid-mmap ...
> > > > > >     ...
> > > > > >     [ perf record: Woken up 1 times to write data ]
> > > > > >     [ perf record: Failed to parse 4 build ids]
> > > > > >     [ perf record: Captured and wrote 0.008 MB perf.data ]
> > > > > >
> > > > > >   Check if there's any build id fault reported:
> > > > > >
> > > > > >     # perf report --header-only
> > > > > >     ...
> > > > > >     # build id mmap stats: FAULTS 4, LOST 0, NOT FIXED
> > > > > >
> > > > > >   There is, check the stats:
> > > > > >
> > > > > >     # perf report --stat
> > > > > >
> > > > > >     Aggregated stats:
> > > > > >              TOTAL events:        104
> > > > > >                       ....
> > > > > >            BUILD_ID fails:          4  (14.3%)
> > > > > >
> > > > > >   Yep, let's fix it:
> > > > > >
> > > > > >     # perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data
> > > > >
> > > > > Can we make it possible to automate this with --fixup-buildids or a
> > > > > perfconfig 'record' knob?
> > > > >
> > > > > This would entail requesting that build-ids that _fail_ be sent to the
> > > > > side-band thread we have in 'perf record', this way we wouldn't have to
> > > > > traverse the whole perf.data file, be it with 'perf-record' at the end
> > > > > of a session with faulty build ids, or in a similar fashion using 'perf
> > > > > inject' as you suggest.
> > > > >
> > > > > I even think that we can have all these modes and let the user to decide
> > > > > how important is this for them and how convenient they want the whole
> > > > > process to be.
> >
> > right, that might be good to decide first.. because as I said,
> > I never hit faulted build id, so it probably needs the special
> > setup you guys are using.. could you try on your setup and check
> > how many faulted build ids you see?
> 
> Did you check data mmaps?  It might be easy to get faults
> from data files and we don't know if it's an ELF or not
> before reading the ELF header in the first page.

well, AFAICS the mmap event is sent right after the elf file
is loaded, so it does not have a chance to be swapped out

> 
> I'm not sure if we can limit it to exec mappings, there might
> be data-only DSOs and we may want to symbolize them too.

hum, I haven't checked the data-only DSO, which we'd load
manually, not the loader.. will check

thanks,
jirka