[PATCH v1] perf inject: Inject build ids for entire call chain

[PATCH v1] perf inject: Inject build ids for entire call chain

[Ian Rogers]

The DSO build id is injected when the dso is first encountered but the
checking for first encountered only looks at the sample->ip not the
entire callchain. Use the callchain logic to ensure all build ids are
inserted.

Add a for_each callback style API to callchain with
sample__for_each_callchain_node. Possibly in the future such an API
can avoid the overhead of constructing the call chain list.

Fixes: 454c407ec17a ("perf: add perf-inject builtin")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-inject.c | 31 +++++++++++++++++++++++++++++++
 tools/perf/util/callchain.c | 35 +++++++++++++++++++++++++++++++++++
 tools/perf/util/callchain.h |  6 ++++++
 3 files changed, 72 insertions(+)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 7b4a5d56d279..ea5f1d57e7e1 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -743,6 +743,29 @@ static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
 	return 0;
 }
 
+struct mark_dso_hit_args {
+	struct perf_tool *tool;
+	struct machine *machine;
+	u8 cpumode;
+};
+
+static int mark_dso_hit_callback(struct callchain_cursor_node *node, void *data)
+{
+	struct mark_dso_hit_args *args = data;
+	struct map *map = node->ms.map;
+
+	if (map) {
+		struct dso *dso = map__dso(map);
+
+		if (dso && !dso__hit(dso)) {
+			dso__set_hit(dso);
+			dso__inject_build_id(dso, args->tool, args->machine,
+					     args->cpumode, map__flags(map));
+		}
+	}
+	return 0;
+}
+
 int perf_event__inject_buildid(struct perf_tool *tool, union perf_event *event,
 			       struct perf_sample *sample,
 			       struct evsel *evsel __maybe_unused,
@@ -750,6 +773,11 @@ int perf_event__inject_buildid(struct perf_tool *tool, union perf_event *event,
 {
 	struct addr_location al;
 	struct thread *thread;
+	struct mark_dso_hit_args args = {
+		.tool = tool,
+		.machine = machine,
+		.cpumode = sample->cpumode,
+	};
 
 	addr_location__init(&al);
 	thread = machine__findnew_thread(machine, sample->pid, sample->tid);
@@ -769,6 +797,9 @@ int perf_event__inject_buildid(struct perf_tool *tool, union perf_event *event,
 		}
 	}
 
+	sample__for_each_callchain_node(thread, evsel, sample, PERF_MAX_STACK_DEPTH,
+					mark_dso_hit_callback, &args);
+
 	thread__put(thread);
 repipe:
 	perf_event__repipe(tool, event, sample, machine);
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 6d075648d2cc..0d608e875fe9 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -1797,3 +1797,38 @@ s64 callchain_avg_cycles(struct callchain_node *cnode)
 
 	return cycles;
 }
+
+int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
+				    struct perf_sample *sample, int max_stack,
+				    callchain_iter_fn cb, void *data)
+{
+	struct callchain_cursor *cursor = get_tls_callchain_cursor();
+	int ret;
+
+	if (!cursor)
+		return -ENOMEM;
+
+	/* Fill in the callchain. */
+	ret = thread__resolve_callchain(thread, cursor, evsel, sample,
+					/*parent=*/NULL, /*root_al=*/NULL,
+					max_stack);
+	if (ret)
+		return ret;
+
+	/* Switch from writing the callchain to reading it. */
+	callchain_cursor_commit(cursor);
+
+	while (1) {
+		struct callchain_cursor_node *node = callchain_cursor_current(cursor);
+
+		if (!node)
+			break;
+
+		ret = cb(node, data);
+		if (ret)
+			return ret;
+
+		callchain_cursor_advance(cursor);
+	}
+	return 0;
+}
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index d5c66345ae31..76891f8e2373 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -311,4 +311,10 @@ u64 callchain_total_hits(struct hists *hists);
 
 s64 callchain_avg_cycles(struct callchain_node *cnode);
 
+typedef int (*callchain_iter_fn)(struct callchain_cursor_node *node, void *data);
+
+int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
+				    struct perf_sample *sample, int max_stack,
+				    callchain_iter_fn cb, void *data);
+
 #endif	/* __PERF_CALLCHAIN_H */
-- 
2.46.0.rc2.264.g509ed76dc8-goog

Re: [PATCH v1] perf inject: Inject build ids for entire call chain

[Namhyung Kim]

Hi Ian,

On Wed, Jul 31, 2024 at 08:12:49PM -0700, Ian Rogers wrote:
> The DSO build id is injected when the dso is first encountered but the
> checking for first encountered only looks at the sample->ip not the
> entire callchain. Use the callchain logic to ensure all build ids are
> inserted.
> 
> Add a for_each callback style API to callchain with
> sample__for_each_callchain_node. Possibly in the future such an API
> can avoid the overhead of constructing the call chain list.
> 
> Fixes: 454c407ec17a ("perf: add perf-inject builtin")
> Signed-off-by: Ian Rogers <irogers@google.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  tools/perf/builtin-inject.c | 31 +++++++++++++++++++++++++++++++
>  tools/perf/util/callchain.c | 35 +++++++++++++++++++++++++++++++++++
>  tools/perf/util/callchain.h |  6 ++++++
>  3 files changed, 72 insertions(+)
> 
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 7b4a5d56d279..ea5f1d57e7e1 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -743,6 +743,29 @@ static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
>  	return 0;
>  }
>  
> +struct mark_dso_hit_args {
> +	struct perf_tool *tool;
> +	struct machine *machine;
> +	u8 cpumode;
> +};
> +
> +static int mark_dso_hit_callback(struct callchain_cursor_node *node, void *data)
> +{
> +	struct mark_dso_hit_args *args = data;
> +	struct map *map = node->ms.map;
> +
> +	if (map) {
> +		struct dso *dso = map__dso(map);
> +
> +		if (dso && !dso__hit(dso)) {
> +			dso__set_hit(dso);
> +			dso__inject_build_id(dso, args->tool, args->machine,
> +					     args->cpumode, map__flags(map));
> +		}
> +	}
> +	return 0;
> +}
> +
>  int perf_event__inject_buildid(struct perf_tool *tool, union perf_event *event,
>  			       struct perf_sample *sample,
>  			       struct evsel *evsel __maybe_unused,
> @@ -750,6 +773,11 @@ int perf_event__inject_buildid(struct perf_tool *tool, union perf_event *event,
>  {
>  	struct addr_location al;
>  	struct thread *thread;
> +	struct mark_dso_hit_args args = {
> +		.tool = tool,
> +		.machine = machine,
> +		.cpumode = sample->cpumode,
> +	};
>  
>  	addr_location__init(&al);
>  	thread = machine__findnew_thread(machine, sample->pid, sample->tid);
> @@ -769,6 +797,9 @@ int perf_event__inject_buildid(struct perf_tool *tool, union perf_event *event,
>  		}
>  	}
>  
> +	sample__for_each_callchain_node(thread, evsel, sample, PERF_MAX_STACK_DEPTH,
> +					mark_dso_hit_callback, &args);
> +
>  	thread__put(thread);
>  repipe:
>  	perf_event__repipe(tool, event, sample, machine);
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index 6d075648d2cc..0d608e875fe9 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -1797,3 +1797,38 @@ s64 callchain_avg_cycles(struct callchain_node *cnode)
>  
>  	return cycles;
>  }
> +
> +int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
> +				    struct perf_sample *sample, int max_stack,
> +				    callchain_iter_fn cb, void *data)
> +{
> +	struct callchain_cursor *cursor = get_tls_callchain_cursor();
> +	int ret;
> +
> +	if (!cursor)
> +		return -ENOMEM;
> +
> +	/* Fill in the callchain. */
> +	ret = thread__resolve_callchain(thread, cursor, evsel, sample,
> +					/*parent=*/NULL, /*root_al=*/NULL,
> +					max_stack);
> +	if (ret)
> +		return ret;
> +
> +	/* Switch from writing the callchain to reading it. */
> +	callchain_cursor_commit(cursor);
> +
> +	while (1) {
> +		struct callchain_cursor_node *node = callchain_cursor_current(cursor);
> +
> +		if (!node)
> +			break;
> +
> +		ret = cb(node, data);
> +		if (ret)
> +			return ret;
> +
> +		callchain_cursor_advance(cursor);
> +	}
> +	return 0;
> +}
> diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
> index d5c66345ae31..76891f8e2373 100644
> --- a/tools/perf/util/callchain.h
> +++ b/tools/perf/util/callchain.h
> @@ -311,4 +311,10 @@ u64 callchain_total_hits(struct hists *hists);
>  
>  s64 callchain_avg_cycles(struct callchain_node *cnode);
>  
> +typedef int (*callchain_iter_fn)(struct callchain_cursor_node *node, void *data);
> +
> +int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
> +				    struct perf_sample *sample, int max_stack,
> +				    callchain_iter_fn cb, void *data);
> +
>  #endif	/* __PERF_CALLCHAIN_H */
> -- 
> 2.46.0.rc2.264.g509ed76dc8-goog
> 

Re: [PATCH v1] perf inject: Inject build ids for entire call chain

[Ian Rogers]

On Fri, Aug 2, 2024 at 11:23 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Ian,
>
> On Wed, Jul 31, 2024 at 08:12:49PM -0700, Ian Rogers wrote:
> > The DSO build id is injected when the dso is first encountered but the
> > checking for first encountered only looks at the sample->ip not the
> > entire callchain. Use the callchain logic to ensure all build ids are
> > inserted.
> >
> > Add a for_each callback style API to callchain with
> > sample__for_each_callchain_node. Possibly in the future such an API
> > can avoid the overhead of constructing the call chain list.
> >
> > Fixes: 454c407ec17a ("perf: add perf-inject builtin")
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Ping.

Thanks,
Ian

Re: [PATCH v1] perf inject: Inject build ids for entire call chain

[Arnaldo Carvalho de Melo]

On Mon, Aug 12, 2024 at 01:43:59PM -0700, Ian Rogers wrote:
> On Fri, Aug 2, 2024 at 11:23 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hi Ian,
> >
> > On Wed, Jul 31, 2024 at 08:12:49PM -0700, Ian Rogers wrote:
> > > The DSO build id is injected when the dso is first encountered but the
> > > checking for first encountered only looks at the sample->ip not the
> > > entire callchain. Use the callchain logic to ensure all build ids are
> > > inserted.
> > >
> > > Add a for_each callback style API to callchain with
> > > sample__for_each_callchain_node. Possibly in the future such an API
> > > can avoid the overhead of constructing the call chain list.
> > >
> > > Fixes: 454c407ec17a ("perf: add perf-inject builtin")
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> >
> > Acked-by: Namhyung Kim <namhyung@kernel.org>
> 
> Ping.

This one isn't applying, maybe a clash with the previous patch or the
constify series?

Please take a look at wat is now in tmp.perf-tools-next

- Arnaldo