lkml.org 
[lkml]   [2022]   [Sep]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 3/4] perf record: Save DSO build-ID for synthesizing
From
On 20/09/22 21:30, Namhyung Kim wrote:
> On Tue, Sep 20, 2022 at 7:00 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 16/09/22 20:59, Namhyung Kim wrote:
>>> When synthesizing MMAP2 with build-id, it'd read the same file repeatedly as
>>> it has no idea if it's done already. Maintain a dsos to check that and skip
>>> the file access if possible.
>>>
>>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>>> ---
>>> tools/perf/util/synthetic-events.c | 49 +++++++++++++++++++++++++-----
>>> 1 file changed, 42 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
>>> index 9d4f5dacd154..e6978b2dee8f 100644
>>> --- a/tools/perf/util/synthetic-events.c
>>> +++ b/tools/perf/util/synthetic-events.c
>>> @@ -4,6 +4,7 @@
>>> #include "util/data.h"
>>> #include "util/debug.h"
>>> #include "util/dso.h"
>>> +#include "util/dsos.h"
>>> #include "util/event.h"
>>> #include "util/evlist.h"
>>> #include "util/machine.h"
>>> @@ -47,12 +48,25 @@
>>>
>>> unsigned int proc_map_timeout = DEFAULT_PROC_MAP_PARSE_TIMEOUT;
>>>
>>> +static bool synth_init_done;
>>> +static struct dsos synth_dsos;
>>> +
>>> void perf_event__synthesize_start(void)
>>> {
>>> + if (synth_init_done)
>>> + return;
>>> +
>>> + dsos__init(&synth_dsos);
>>> +
>>> + synth_init_done = true;
>>> }
>>>
>>> void perf_event__synthesize_stop(void)
>>> {
>>> + if (!synth_init_done)
>>> + return;
>>> +
>>> + dsos__exit(&synth_dsos);
>>> }
>>>
>>> int perf_tool__process_synth_event(struct perf_tool *tool,
>>> @@ -374,26 +388,47 @@ static bool read_proc_maps_line(struct io *io, __u64 *start, __u64 *end,
>>> static void perf_record_mmap2__read_build_id(struct perf_record_mmap2 *event,
>>> bool is_kernel)
>>> {
>>> - struct build_id bid;
>>> + struct build_id _bid, *bid = &_bid;
>>> + struct dso *dso = NULL;
>>> + struct dso_id id;
>>> int rc;
>>>
>>> - if (is_kernel)
>>> - rc = sysfs__read_build_id("/sys/kernel/notes", &bid);
>>> - else
>>> - rc = filename__read_build_id(event->filename, &bid) > 0 ? 0 : -1;
>>> + if (is_kernel) {
>>> + rc = sysfs__read_build_id("/sys/kernel/notes", bid);
>>> + goto out;
>>> + }
>>> +
>>> + id.maj = event->maj;
>>> + id.min = event->min;
>>> + id.ino = event->ino;
>>> + id.ino_generation = event->ino_generation;
>>>
>>
>> There might be some paths missing perf_event__synthesize_start()
>> e.g. perf trace
>> So it would probably be safer to use lazy initialization for
>> synth_dsos.
>
> I thought about that too, but it'd need a cleanup routine at the end
> anyway. So I ended up having a pair of start/stop routines.
>
>>
>> Also, callers of perf_record_mmap2__read_build_id() have a struct
>> machine so I wonder about putting synth_dsos in struct machine ?
>> Or even just using machine->dsos ?
>
> My intention was to use minimal info from struct dso - name, id and
> build-id. But as it already uses dsos/dso routines, it's not much
> different from using the existing machine->dsos.
>
> So yeah, I think it's good to reuse the existing one as it'd benefit
> the build-id processing at the end. Will change.
>
> Thanks,
> Namhyung
>
>
>>
>>> + dso = dsos__findnew_id(&synth_dsos, event->filename, &id);
>>> + if (dso && dso->has_build_id) {
>>> + bid = &dso->bid;
>>> + rc = 0;
>>> + goto out;
>>> + }

Also I guess the dsos optimization could be a separate patch ?

>>> +
>>> + rc = filename__read_build_id(event->filename, bid) > 0 ? 0 : -1;
>>> +
>>> +out:
>>> if (rc == 0) {
>>> - memcpy(event->build_id, bid.data, sizeof(bid.data));
>>> - event->build_id_size = (u8) bid.size;
>>> + memcpy(event->build_id, bid->data, sizeof(bid->data));
>>> + event->build_id_size = (u8) bid->size;
>>> event->header.misc |= PERF_RECORD_MISC_MMAP_BUILD_ID;
>>> event->__reserved_1 = 0;
>>> event->__reserved_2 = 0;
>>> +
>>> + if (dso && !dso->has_build_id)
>>> + dso__set_build_id(dso, bid);
>>> } else {
>>> if (event->filename[0] == '/') {
>>> pr_debug2("Failed to read build ID for %s\n",
>>> event->filename);
>>> }
>>> }
>>> + dso__put(dso);
>>> }
>>>
>>> int perf_event__synthesize_mmap_events(struct perf_tool *tool,
>>

\
 
 \ /
  Last update: 2022-09-21 09:27    [W:0.041 / U:1.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site