* [PATCH 1/3] perf inject: Add missing callbacks in perf_tool
@ 2020-09-14 14:18 Namhyung Kim
2020-09-14 14:18 ` [PATCH 2/3] perf inject: Enter namespace when reading build-id Namhyung Kim
2020-09-14 14:18 ` [PATCH 3/3] perf inject: Do not load map/dso when injecting build-id Namhyung Kim
0 siblings, 2 replies; 9+ messages in thread
From: Namhyung Kim @ 2020-09-14 14:18 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
Stephane Eranian, LKML, Ian Rogers
I found some events (like PERF_RECORD_CGROUP) are not copied by perf
inject due to the missing callbacks. Let's add them.
While at it, I've changed the order of the callbacks to match with
struct perf_tool so that we can compare them easily.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-inject.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 6d2f410d773a..59576877c67f 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -97,6 +97,13 @@ static int perf_event__repipe_op2_synth(struct perf_session *session,
return perf_event__repipe_synth(session->tool, event);
}
+static int perf_event__repipe_op4_synth(struct perf_session *session,
+ union perf_event *event,
+ u64 data __maybe_unused)
+{
+ return perf_event__repipe_synth(session->tool, event);
+}
+
static int perf_event__repipe_attr(struct perf_tool *tool,
union perf_event *event,
struct evlist **pevlist)
@@ -115,6 +122,13 @@ static int perf_event__repipe_attr(struct perf_tool *tool,
return perf_event__repipe_synth(tool, event);
}
+static int perf_event__repipe_event_update(struct perf_tool *tool,
+ union perf_event *event,
+ struct evlist **pevlist __maybe_unused)
+{
+ return perf_event__repipe_synth(tool, event);
+}
+
#ifdef HAVE_AUXTRACE_SUPPORT
static int copy_bytes(struct perf_inject *inject, int fd, off_t size)
@@ -708,9 +722,12 @@ int cmd_inject(int argc, const char **argv)
struct perf_inject inject = {
.tool = {
.sample = perf_event__repipe_sample,
+ .read = perf_event__repipe_sample,
.mmap = perf_event__repipe,
.mmap2 = perf_event__repipe,
.comm = perf_event__repipe,
+ .namespaces = perf_event__repipe,
+ .cgroup = perf_event__repipe,
.fork = perf_event__repipe,
.exit = perf_event__repipe,
.lost = perf_event__repipe,
@@ -718,19 +735,28 @@ int cmd_inject(int argc, const char **argv)
.aux = perf_event__repipe,
.itrace_start = perf_event__repipe,
.context_switch = perf_event__repipe,
- .read = perf_event__repipe_sample,
.throttle = perf_event__repipe,
.unthrottle = perf_event__repipe,
+ .ksymbol = perf_event__repipe,
+ .bpf = perf_event__repipe,
+ .text_poke = perf_event__repipe,
.attr = perf_event__repipe_attr,
+ .event_update = perf_event__repipe_event_update,
.tracing_data = perf_event__repipe_op2_synth,
- .auxtrace_info = perf_event__repipe_op2_synth,
- .auxtrace = perf_event__repipe_auxtrace,
- .auxtrace_error = perf_event__repipe_op2_synth,
- .time_conv = perf_event__repipe_op2_synth,
.finished_round = perf_event__repipe_oe_synth,
.build_id = perf_event__repipe_op2_synth,
.id_index = perf_event__repipe_op2_synth,
+ .auxtrace_info = perf_event__repipe_op2_synth,
+ .auxtrace_error = perf_event__repipe_op2_synth,
+ .time_conv = perf_event__repipe_op2_synth,
+ .thread_map = perf_event__repipe_op2_synth,
+ .cpu_map = perf_event__repipe_op2_synth,
+ .stat_config = perf_event__repipe_op2_synth,
+ .stat = perf_event__repipe_op2_synth,
+ .stat_round = perf_event__repipe_op2_synth,
.feature = perf_event__repipe_op2_synth,
+ .compressed = perf_event__repipe_op4_synth,
+ .auxtrace = perf_event__repipe_auxtrace,
},
.input_name = "-",
.samples = LIST_HEAD_INIT(inject.samples),
--
2.28.0.618.gf4bc123cb7-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] perf inject: Enter namespace when reading build-id
2020-09-14 14:18 [PATCH 1/3] perf inject: Add missing callbacks in perf_tool Namhyung Kim
@ 2020-09-14 14:18 ` Namhyung Kim
2020-09-14 14:18 ` [PATCH 3/3] perf inject: Do not load map/dso when injecting build-id Namhyung Kim
1 sibling, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2020-09-14 14:18 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
Stephane Eranian, LKML, Ian Rogers
It should be in a proper mnt namespace when accessing the file.
I think this had no problem since the build-id was actually read from
map__load() -> dso__load() already. But I'd like to change it in the
following commit.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-inject.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 59576877c67f..a2804d906d2a 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -23,6 +23,7 @@
#include "util/symbol.h"
#include "util/synthetic-events.h"
#include "util/thread.h"
+#include "util/namespaces.h"
#include <linux/err.h>
#include <subcmd/parse-options.h>
@@ -419,16 +420,19 @@ static int perf_event__repipe_tracing_data(struct perf_session *session,
static int dso__read_build_id(struct dso *dso)
{
+ struct nscookie nsc;
+
if (dso->has_build_id)
return 0;
+ nsinfo__mountns_enter(dso->nsinfo, &nsc);
if (filename__read_build_id(dso->long_name, dso->build_id,
sizeof(dso->build_id)) > 0) {
dso->has_build_id = true;
- return 0;
}
+ nsinfo__mountns_exit(&nsc);
- return -1;
+ return dso->has_build_id ? 0 : -1;
}
static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
--
2.28.0.618.gf4bc123cb7-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] perf inject: Do not load map/dso when injecting build-id
2020-09-14 14:18 [PATCH 1/3] perf inject: Add missing callbacks in perf_tool Namhyung Kim
2020-09-14 14:18 ` [PATCH 2/3] perf inject: Enter namespace when reading build-id Namhyung Kim
@ 2020-09-14 14:18 ` Namhyung Kim
2020-09-15 10:05 ` Jiri Olsa
1 sibling, 1 reply; 9+ messages in thread
From: Namhyung Kim @ 2020-09-14 14:18 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
Stephane Eranian, LKML, Ian Rogers
No need to load symbols in a DSO when injecting build-id. I guess the
reason was to check the DSO is a special file like anon files. Use
some helper functions in map.c to check them before reading build-id.
Also pass sample event's cpumode to a new build-id event.
Original-patch-by: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-inject.c | 30 ++++++++++--------------------
tools/perf/util/map.c | 17 +----------------
tools/perf/util/map.h | 14 ++++++++++++++
3 files changed, 25 insertions(+), 36 deletions(-)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index a2804d906d2a..6d4e6833efed 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -436,21 +436,22 @@ static int dso__read_build_id(struct dso *dso)
}
static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
- struct machine *machine)
+ struct machine *machine, u8 cpumode)
{
- u16 misc = PERF_RECORD_MISC_USER;
int err;
+ if (is_anon_memory(dso->long_name))
+ return 0;
+ if (is_no_dso_memory(dso->long_name))
+ return 0;
+
if (dso__read_build_id(dso) < 0) {
pr_debug("no build_id found for %s\n", dso->long_name);
return -1;
}
- if (dso->kernel)
- misc = PERF_RECORD_MISC_KERNEL;
-
- err = perf_event__synthesize_build_id(tool, dso, misc, perf_event__repipe,
- machine);
+ err = perf_event__synthesize_build_id(tool, dso, cpumode,
+ perf_event__repipe, machine);
if (err) {
pr_err("Can't synthesize build_id event for %s\n", dso->long_name);
return -1;
@@ -478,19 +479,8 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
if (thread__find_map(thread, sample->cpumode, sample->ip, &al)) {
if (!al.map->dso->hit) {
al.map->dso->hit = 1;
- if (map__load(al.map) >= 0) {
- dso__inject_build_id(al.map->dso, tool, machine);
- /*
- * If this fails, too bad, let the other side
- * account this as unresolved.
- */
- } else {
-#ifdef HAVE_LIBELF_SUPPORT
- pr_warning("no symbols found in %s, maybe "
- "install a debug package?\n",
- al.map->dso->long_name);
-#endif
- }
+ dso__inject_build_id(al.map->dso, tool, machine,
+ sample->cpumode);
}
}
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index cc0faf8f1321..8b305e624124 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -27,21 +27,6 @@
static void __maps__insert(struct maps *maps, struct map *map);
-static inline int is_anon_memory(const char *filename, u32 flags)
-{
- return flags & MAP_HUGETLB ||
- !strcmp(filename, "//anon") ||
- !strncmp(filename, "/dev/zero", sizeof("/dev/zero") - 1) ||
- !strncmp(filename, "/anon_hugepage", sizeof("/anon_hugepage") - 1);
-}
-
-static inline int is_no_dso_memory(const char *filename)
-{
- return !strncmp(filename, "[stack", 6) ||
- !strncmp(filename, "/SYSV",5) ||
- !strcmp(filename, "[heap]");
-}
-
static inline int is_android_lib(const char *filename)
{
return strstarts(filename, "/data/app-lib/") ||
@@ -158,7 +143,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
int anon, no_dso, vdso, android;
android = is_android_lib(filename);
- anon = is_anon_memory(filename, flags);
+ anon = is_anon_memory(filename) || flags & MAP_HUGETLB;
vdso = is_vdso_map(filename);
no_dso = is_no_dso_memory(filename);
map->prot = prot;
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index c2f5d28fe73a..b1c0686db1b7 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -171,4 +171,18 @@ static inline bool is_bpf_image(const char *name)
return strncmp(name, "bpf_trampoline_", sizeof("bpf_trampoline_") - 1) == 0 ||
strncmp(name, "bpf_dispatcher_", sizeof("bpf_dispatcher_") - 1) == 0;
}
+
+static inline int is_anon_memory(const char *filename)
+{
+ return !strcmp(filename, "//anon") ||
+ !strncmp(filename, "/dev/zero", sizeof("/dev/zero") - 1) ||
+ !strncmp(filename, "/anon_hugepage", sizeof("/anon_hugepage") - 1);
+}
+
+static inline int is_no_dso_memory(const char *filename)
+{
+ return !strncmp(filename, "[stack", 6) ||
+ !strncmp(filename, "/SYSV", 5) ||
+ !strcmp(filename, "[heap]");
+}
#endif /* __PERF_MAP_H */
--
2.28.0.618.gf4bc123cb7-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] perf inject: Do not load map/dso when injecting build-id
2020-09-14 14:18 ` [PATCH 3/3] perf inject: Do not load map/dso when injecting build-id Namhyung Kim
@ 2020-09-15 10:05 ` Jiri Olsa
2020-09-15 14:55 ` Namhyung Kim
2020-09-17 1:31 ` Namhyung Kim
0 siblings, 2 replies; 9+ messages in thread
From: Jiri Olsa @ 2020-09-15 10:05 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, Stephane Eranian, LKML,
Ian Rogers
On Mon, Sep 14, 2020 at 11:18:59PM +0900, Namhyung Kim wrote:
> No need to load symbols in a DSO when injecting build-id. I guess the
> reason was to check the DSO is a special file like anon files. Use
> some helper functions in map.c to check them before reading build-id.
> Also pass sample event's cpumode to a new build-id event.
>
> Original-patch-by: Stephane Eranian <eranian@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-inject.c | 30 ++++++++++--------------------
> tools/perf/util/map.c | 17 +----------------
> tools/perf/util/map.h | 14 ++++++++++++++
> 3 files changed, 25 insertions(+), 36 deletions(-)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index a2804d906d2a..6d4e6833efed 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -436,21 +436,22 @@ static int dso__read_build_id(struct dso *dso)
> }
>
> static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
> - struct machine *machine)
> + struct machine *machine, u8 cpumode)
> {
> - u16 misc = PERF_RECORD_MISC_USER;
> int err;
>
> + if (is_anon_memory(dso->long_name))
> + return 0;
> + if (is_no_dso_memory(dso->long_name))
> + return 0;
should we check for vdso as well? I don't think it has build id
> +
> if (dso__read_build_id(dso) < 0) {
> pr_debug("no build_id found for %s\n", dso->long_name);
> return -1;
> }
>
> - if (dso->kernel)
> - misc = PERF_RECORD_MISC_KERNEL;
> -
> - err = perf_event__synthesize_build_id(tool, dso, misc, perf_event__repipe,
> - machine);
> + err = perf_event__synthesize_build_id(tool, dso, cpumode,
> + perf_event__repipe, machine);
> if (err) {
> pr_err("Can't synthesize build_id event for %s\n", dso->long_name);
> return -1;
> @@ -478,19 +479,8 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
> if (thread__find_map(thread, sample->cpumode, sample->ip, &al)) {
> if (!al.map->dso->hit) {
> al.map->dso->hit = 1;
> - if (map__load(al.map) >= 0) {
nice, that might do some nice speedup, did you see any?
jirka
> - dso__inject_build_id(al.map->dso, tool, machine);
> - /*
> - * If this fails, too bad, let the other side
> - * account this as unresolved.
> - */
> - } else {
> -#ifdef HAVE_LIBELF_SUPPORT
> - pr_warning("no symbols found in %s, maybe "
> - "install a debug package?\n",
> - al.map->dso->long_name);
> -#endif
> - }
> + dso__inject_build_id(al.map->dso, tool, machine,
> + sample->cpumode);
> }
SNIP
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] perf inject: Do not load map/dso when injecting build-id
2020-09-15 10:05 ` Jiri Olsa
@ 2020-09-15 14:55 ` Namhyung Kim
2020-09-16 8:24 ` Jiri Olsa
2020-09-17 1:31 ` Namhyung Kim
1 sibling, 1 reply; 9+ messages in thread
From: Namhyung Kim @ 2020-09-15 14:55 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, Stephane Eranian, LKML,
Ian Rogers
On Tue, Sep 15, 2020 at 7:05 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Sep 14, 2020 at 11:18:59PM +0900, Namhyung Kim wrote:
> > No need to load symbols in a DSO when injecting build-id. I guess the
> > reason was to check the DSO is a special file like anon files. Use
> > some helper functions in map.c to check them before reading build-id.
> > Also pass sample event's cpumode to a new build-id event.
> >
> > Original-patch-by: Stephane Eranian <eranian@google.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/builtin-inject.c | 30 ++++++++++--------------------
> > tools/perf/util/map.c | 17 +----------------
> > tools/perf/util/map.h | 14 ++++++++++++++
> > 3 files changed, 25 insertions(+), 36 deletions(-)
> >
> > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > index a2804d906d2a..6d4e6833efed 100644
> > --- a/tools/perf/builtin-inject.c
> > +++ b/tools/perf/builtin-inject.c
> > @@ -436,21 +436,22 @@ static int dso__read_build_id(struct dso *dso)
> > }
> >
> > static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
> > - struct machine *machine)
> > + struct machine *machine, u8 cpumode)
> > {
> > - u16 misc = PERF_RECORD_MISC_USER;
> > int err;
> >
> > + if (is_anon_memory(dso->long_name))
> > + return 0;
> > + if (is_no_dso_memory(dso->long_name))
> > + return 0;
>
> should we check for vdso as well? I don't think it has build id
I don't know.. But I guess there's no reason it shouldn't?
>
> > +
> > if (dso__read_build_id(dso) < 0) {
> > pr_debug("no build_id found for %s\n", dso->long_name);
> > return -1;
> > }
> >
> > - if (dso->kernel)
> > - misc = PERF_RECORD_MISC_KERNEL;
> > -
> > - err = perf_event__synthesize_build_id(tool, dso, misc, perf_event__repipe,
> > - machine);
> > + err = perf_event__synthesize_build_id(tool, dso, cpumode,
> > + perf_event__repipe, machine);
> > if (err) {
> > pr_err("Can't synthesize build_id event for %s\n", dso->long_name);
> > return -1;
> > @@ -478,19 +479,8 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
> > if (thread__find_map(thread, sample->cpumode, sample->ip, &al)) {
> > if (!al.map->dso->hit) {
> > al.map->dso->hit = 1;
> > - if (map__load(al.map) >= 0) {
>
> nice, that might do some nice speedup, did you see any?
Yes, I believe so. But my quick test on a laptop was too short.
I will test it again and share the speedup.
Thanks
Namhyung
>
> jirka
>
> > - dso__inject_build_id(al.map->dso, tool, machine);
> > - /*
> > - * If this fails, too bad, let the other side
> > - * account this as unresolved.
> > - */
> > - } else {
> > -#ifdef HAVE_LIBELF_SUPPORT
> > - pr_warning("no symbols found in %s, maybe "
> > - "install a debug package?\n",
> > - al.map->dso->long_name);
> > -#endif
> > - }
> > + dso__inject_build_id(al.map->dso, tool, machine,
> > + sample->cpumode);
> > }
>
> SNIP
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] perf inject: Do not load map/dso when injecting build-id
2020-09-15 14:55 ` Namhyung Kim
@ 2020-09-16 8:24 ` Jiri Olsa
2020-09-17 1:27 ` Namhyung Kim
0 siblings, 1 reply; 9+ messages in thread
From: Jiri Olsa @ 2020-09-16 8:24 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, Stephane Eranian, LKML,
Ian Rogers
On Tue, Sep 15, 2020 at 11:55:34PM +0900, Namhyung Kim wrote:
> On Tue, Sep 15, 2020 at 7:05 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Mon, Sep 14, 2020 at 11:18:59PM +0900, Namhyung Kim wrote:
> > > No need to load symbols in a DSO when injecting build-id. I guess the
> > > reason was to check the DSO is a special file like anon files. Use
> > > some helper functions in map.c to check them before reading build-id.
> > > Also pass sample event's cpumode to a new build-id event.
> > >
> > > Original-patch-by: Stephane Eranian <eranian@google.com>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > > tools/perf/builtin-inject.c | 30 ++++++++++--------------------
> > > tools/perf/util/map.c | 17 +----------------
> > > tools/perf/util/map.h | 14 ++++++++++++++
> > > 3 files changed, 25 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > > index a2804d906d2a..6d4e6833efed 100644
> > > --- a/tools/perf/builtin-inject.c
> > > +++ b/tools/perf/builtin-inject.c
> > > @@ -436,21 +436,22 @@ static int dso__read_build_id(struct dso *dso)
> > > }
> > >
> > > static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
> > > - struct machine *machine)
> > > + struct machine *machine, u8 cpumode)
> > > {
> > > - u16 misc = PERF_RECORD_MISC_USER;
> > > int err;
> > >
> > > + if (is_anon_memory(dso->long_name))
> > > + return 0;
> > > + if (is_no_dso_memory(dso->long_name))
> > > + return 0;
> >
> > should we check for vdso as well? I don't think it has build id
>
> I don't know.. But I guess there's no reason it shouldn't?
I haven't checked, it's just I always saw only zeros for vdso build ids
jirka
>
>
> >
> > > +
> > > if (dso__read_build_id(dso) < 0) {
> > > pr_debug("no build_id found for %s\n", dso->long_name);
> > > return -1;
> > > }
> > >
> > > - if (dso->kernel)
> > > - misc = PERF_RECORD_MISC_KERNEL;
> > > -
> > > - err = perf_event__synthesize_build_id(tool, dso, misc, perf_event__repipe,
> > > - machine);
> > > + err = perf_event__synthesize_build_id(tool, dso, cpumode,
> > > + perf_event__repipe, machine);
> > > if (err) {
> > > pr_err("Can't synthesize build_id event for %s\n", dso->long_name);
> > > return -1;
> > > @@ -478,19 +479,8 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
> > > if (thread__find_map(thread, sample->cpumode, sample->ip, &al)) {
> > > if (!al.map->dso->hit) {
> > > al.map->dso->hit = 1;
> > > - if (map__load(al.map) >= 0) {
> >
> > nice, that might do some nice speedup, did you see any?
>
> Yes, I believe so. But my quick test on a laptop was too short.
> I will test it again and share the speedup.
>
> Thanks
> Namhyung
>
>
> >
> > jirka
> >
> > > - dso__inject_build_id(al.map->dso, tool, machine);
> > > - /*
> > > - * If this fails, too bad, let the other side
> > > - * account this as unresolved.
> > > - */
> > > - } else {
> > > -#ifdef HAVE_LIBELF_SUPPORT
> > > - pr_warning("no symbols found in %s, maybe "
> > > - "install a debug package?\n",
> > > - al.map->dso->long_name);
> > > -#endif
> > > - }
> > > + dso__inject_build_id(al.map->dso, tool, machine,
> > > + sample->cpumode);
> > > }
> >
> > SNIP
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] perf inject: Do not load map/dso when injecting build-id
2020-09-16 8:24 ` Jiri Olsa
@ 2020-09-17 1:27 ` Namhyung Kim
2020-09-17 8:01 ` Jiri Olsa
0 siblings, 1 reply; 9+ messages in thread
From: Namhyung Kim @ 2020-09-17 1:27 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, Stephane Eranian, LKML,
Ian Rogers
On Wed, Sep 16, 2020 at 5:24 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Tue, Sep 15, 2020 at 11:55:34PM +0900, Namhyung Kim wrote:
> > On Tue, Sep 15, 2020 at 7:05 PM Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > On Mon, Sep 14, 2020 at 11:18:59PM +0900, Namhyung Kim wrote:
> > > > No need to load symbols in a DSO when injecting build-id. I guess the
> > > > reason was to check the DSO is a special file like anon files. Use
> > > > some helper functions in map.c to check them before reading build-id.
> > > > Also pass sample event's cpumode to a new build-id event.
> > > >
> > > > Original-patch-by: Stephane Eranian <eranian@google.com>
> > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > ---
> > > > tools/perf/builtin-inject.c | 30 ++++++++++--------------------
> > > > tools/perf/util/map.c | 17 +----------------
> > > > tools/perf/util/map.h | 14 ++++++++++++++
> > > > 3 files changed, 25 insertions(+), 36 deletions(-)
> > > >
> > > > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > > > index a2804d906d2a..6d4e6833efed 100644
> > > > --- a/tools/perf/builtin-inject.c
> > > > +++ b/tools/perf/builtin-inject.c
> > > > @@ -436,21 +436,22 @@ static int dso__read_build_id(struct dso *dso)
> > > > }
> > > >
> > > > static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
> > > > - struct machine *machine)
> > > > + struct machine *machine, u8 cpumode)
> > > > {
> > > > - u16 misc = PERF_RECORD_MISC_USER;
> > > > int err;
> > > >
> > > > + if (is_anon_memory(dso->long_name))
> > > > + return 0;
> > > > + if (is_no_dso_memory(dso->long_name))
> > > > + return 0;
> > >
> > > should we check for vdso as well? I don't think it has build id
> >
> > I don't know.. But I guess there's no reason it shouldn't?
>
> I haven't checked, it's just I always saw only zeros for vdso build ids
I found this in arch/x86/entry/vdso/Makefile. It seems to have one..
quiet_cmd_vdso = VDSO $@
cmd_vdso = $(LD) -nostdlib -o $@ \
$(VDSO_LDFLAGS) $(VDSO_LDFLAGS_$(filter %.lds,$(^F))) \
-T $(filter %.lds,$^) $(filter %.o,$^) && \
sh $(srctree)/$(src)/checkundef.sh '$(NM)' '$@'
VDSO_LDFLAGS = -shared --hash-style=both --build-id \
$(call ld-option, --eh-frame-hdr) -Bsymbolic
Thanks
Namhyung
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] perf inject: Do not load map/dso when injecting build-id
2020-09-15 10:05 ` Jiri Olsa
2020-09-15 14:55 ` Namhyung Kim
@ 2020-09-17 1:31 ` Namhyung Kim
1 sibling, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2020-09-17 1:31 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, Stephane Eranian, LKML,
Ian Rogers
On Tue, Sep 15, 2020 at 7:05 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Sep 14, 2020 at 11:18:59PM +0900, Namhyung Kim wrote:
> > @@ -478,19 +479,8 @@ static int perf_event__inject_buildid(struct perf_tool *tool,
> > if (thread__find_map(thread, sample->cpumode, sample->ip, &al)) {
> > if (!al.map->dso->hit) {
> > al.map->dso->hit = 1;
> > - if (map__load(al.map) >= 0) {
>
> nice, that might do some nice speedup, did you see any?
I've checked it but the speed up was tiny on my machine.
I guess it's because the machine was idle and had everything
in memory. But I also found some more points to improve.
Will share the result in the next spin.
Thanks
Namhyung
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] perf inject: Do not load map/dso when injecting build-id
2020-09-17 1:27 ` Namhyung Kim
@ 2020-09-17 8:01 ` Jiri Olsa
0 siblings, 0 replies; 9+ messages in thread
From: Jiri Olsa @ 2020-09-17 8:01 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, Stephane Eranian, LKML,
Ian Rogers
On Thu, Sep 17, 2020 at 10:27:41AM +0900, Namhyung Kim wrote:
> On Wed, Sep 16, 2020 at 5:24 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Tue, Sep 15, 2020 at 11:55:34PM +0900, Namhyung Kim wrote:
> > > On Tue, Sep 15, 2020 at 7:05 PM Jiri Olsa <jolsa@redhat.com> wrote:
> > > >
> > > > On Mon, Sep 14, 2020 at 11:18:59PM +0900, Namhyung Kim wrote:
> > > > > No need to load symbols in a DSO when injecting build-id. I guess the
> > > > > reason was to check the DSO is a special file like anon files. Use
> > > > > some helper functions in map.c to check them before reading build-id.
> > > > > Also pass sample event's cpumode to a new build-id event.
> > > > >
> > > > > Original-patch-by: Stephane Eranian <eranian@google.com>
> > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > > ---
> > > > > tools/perf/builtin-inject.c | 30 ++++++++++--------------------
> > > > > tools/perf/util/map.c | 17 +----------------
> > > > > tools/perf/util/map.h | 14 ++++++++++++++
> > > > > 3 files changed, 25 insertions(+), 36 deletions(-)
> > > > >
> > > > > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > > > > index a2804d906d2a..6d4e6833efed 100644
> > > > > --- a/tools/perf/builtin-inject.c
> > > > > +++ b/tools/perf/builtin-inject.c
> > > > > @@ -436,21 +436,22 @@ static int dso__read_build_id(struct dso *dso)
> > > > > }
> > > > >
> > > > > static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
> > > > > - struct machine *machine)
> > > > > + struct machine *machine, u8 cpumode)
> > > > > {
> > > > > - u16 misc = PERF_RECORD_MISC_USER;
> > > > > int err;
> > > > >
> > > > > + if (is_anon_memory(dso->long_name))
> > > > > + return 0;
> > > > > + if (is_no_dso_memory(dso->long_name))
> > > > > + return 0;
> > > >
> > > > should we check for vdso as well? I don't think it has build id
> > >
> > > I don't know.. But I guess there's no reason it shouldn't?
> >
> > I haven't checked, it's just I always saw only zeros for vdso build ids
>
> I found this in arch/x86/entry/vdso/Makefile. It seems to have one..
>
> quiet_cmd_vdso = VDSO $@
> cmd_vdso = $(LD) -nostdlib -o $@ \
> $(VDSO_LDFLAGS) $(VDSO_LDFLAGS_$(filter %.lds,$(^F))) \
> -T $(filter %.lds,$^) $(filter %.o,$^) && \
> sh $(srctree)/$(src)/checkundef.sh '$(NM)' '$@'
>
> VDSO_LDFLAGS = -shared --hash-style=both --build-id \
> $(call ld-option, --eh-frame-hdr) -Bsymbolic
I see, I'll check why I'm getting zeros
thanks,
jirka
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-09-17 8:10 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 14:18 [PATCH 1/3] perf inject: Add missing callbacks in perf_tool Namhyung Kim
2020-09-14 14:18 ` [PATCH 2/3] perf inject: Enter namespace when reading build-id Namhyung Kim
2020-09-14 14:18 ` [PATCH 3/3] perf inject: Do not load map/dso when injecting build-id Namhyung Kim
2020-09-15 10:05 ` Jiri Olsa
2020-09-15 14:55 ` Namhyung Kim
2020-09-16 8:24 ` Jiri Olsa
2020-09-17 1:27 ` Namhyung Kim
2020-09-17 8:01 ` Jiri Olsa
2020-09-17 1:31 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).