linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] perf inject: Add a command line option to specify build ids
@ 2022-07-07 21:56 Raul Silvera
  2022-07-08 18:50 ` Namhyung Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Raul Silvera @ 2022-07-07 21:56 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim
  Cc: linux-perf-users, linux-kernel, Raul Silvera

This commit adds the option --known-build-ids to perf inject.
It allows the user to explicitly specify the build id for a given
path, instead of retrieving it from the current system. This is
useful in cases where a perf.data file is processed on a different
system from where it was collected, or if some of the binaries are
no longer available.

The build ids and paths are specified in pairs in the command line.
Using the file:// specifier, build ids can be loaded from a file
directly generated by perf buildid-list. This is convenient to copy
build ids from one perf.data file to another.

** Example: In this example we use perf record to create two
perf.data files, one with build ids and another without, and use
perf buildid-list and perf inject to copy the build ids from the
first file to the second.

 $ perf record ls /tmp
 $ perf record --no-buildid -o perf.data.no-buildid ls /tmp
 $ perf buildid-list > /tmp/build-ids.txt
 $ perf inject -b --known-build-ids='file:///tmp/build-ids.txt' \
        -i perf.data.no-buildid -o perf.data.buildid

Signed-off-by: Raul Silvera <rsilvera@google.com>
---
 V1 -> V2: Cleaned up patch description, deleted the strlist during
           cleanup, and updated validation of the build id strings

 tools/perf/builtin-inject.c | 60 +++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index a75bf11585b5..4efb992ed1a0 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -21,6 +21,7 @@
 #include "util/data.h"
 #include "util/auxtrace.h"
 #include "util/jit.h"
+#include "util/string2.h"
 #include "util/symbol.h"
 #include "util/synthetic-events.h"
 #include "util/thread.h"
@@ -35,6 +36,7 @@
 
 #include <linux/list.h>
 #include <linux/string.h>
+#include <ctype.h>
 #include <errno.h>
 #include <signal.h>
 
@@ -59,6 +61,8 @@ struct perf_inject {
 	struct itrace_synth_opts itrace_synth_opts;
 	char			event_copy[PERF_SAMPLE_MAX_SIZE];
 	struct perf_file_section secs[HEADER_FEAT_BITS];
+	const char		*known_build_ids_source;
+	struct strlist		*known_build_ids;
 };
 
 struct event_entry {
@@ -570,9 +574,45 @@ static int dso__read_build_id(struct dso *dso)
 	return dso->has_build_id ? 0 : -1;
 }
 
+static bool perf_inject__lookup_known_build_id(struct perf_inject *inject,
+					       struct dso *dso)
+{
+	struct str_node *pos;
+	int bid_len;
+
+	strlist__for_each_entry(pos, inject->known_build_ids) {
+		const char *build_id, *dso_name;
+
+		build_id = skip_spaces(pos->s);
+		dso_name = strchr(build_id, ' ');
+		if (dso_name == NULL)
+			continue;
+		bid_len = dso_name - pos->s;
+		dso_name = skip_spaces(dso_name);
+		if (strcmp(dso->long_name, dso_name))
+			continue;
+		if (bid_len % 2 != 0 || bid_len >= SBUILD_ID_SIZE)
+			return false;
+		for (int ix = 0; 2 * ix + 1 < bid_len; ++ix) {
+			if (!isxdigit(build_id[2 * ix]) ||
+			    !isxdigit(build_id[2 * ix + 1]))
+				return false;
+
+			dso->bid.data[ix] = (hex(build_id[2 * ix]) << 4 |
+					     hex(build_id[2 * ix + 1]));
+		}
+		dso->bid.size = bid_len / 2;
+		dso->has_build_id = 1;
+		return true;
+	}
+	return false;
+}
+
 static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
 				struct machine *machine, u8 cpumode, u32 flags)
 {
+	struct perf_inject *inject = container_of(tool, struct perf_inject,
+						  tool);
 	int err;
 
 	if (is_anon_memory(dso->long_name) || flags & MAP_HUGETLB)
@@ -580,6 +620,10 @@ static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
 	if (is_no_dso_memory(dso->long_name))
 		return 0;
 
+	if (inject->known_build_ids != NULL &&
+	    perf_inject__lookup_known_build_id(inject, dso))
+		return 1;
+
 	if (dso__read_build_id(dso) < 0) {
 		pr_debug("no build_id found for %s\n", dso->long_name);
 		return -1;
@@ -1082,6 +1126,9 @@ int cmd_inject(int argc, const char **argv)
 			    "Inject build-ids into the output stream"),
 		OPT_BOOLEAN(0, "buildid-all", &inject.build_id_all,
 			    "Inject build-ids of all DSOs into the output stream"),
+		OPT_STRING(0, "known-build-ids", &inject.known_build_ids_source,
+			   "buildid path [buildid path...]",
+			   "build-ids to use for specific files"),
 		OPT_STRING('i', "input", &inject.input_name, "file",
 			   "input file name"),
 		OPT_STRING('o', "output", &inject.output.path, "file",
@@ -1215,6 +1262,18 @@ int cmd_inject(int argc, const char **argv)
 		 */
 		inject.tool.ordered_events = true;
 		inject.tool.ordering_requires_timestamps = true;
+		if (inject.known_build_ids_source != NULL) {
+			struct strlist *known_build_ids;
+
+			known_build_ids = strlist__new(
+			    inject.known_build_ids_source, NULL);
+
+			if (known_build_ids == NULL) {
+				pr_err("Couldn't parse known build ids.\n");
+				goto out_delete;
+			}
+			inject.known_build_ids = known_build_ids;
+		}
 	}
 
 	if (inject.sched_stat) {
@@ -1241,6 +1300,7 @@ int cmd_inject(int argc, const char **argv)
 	ret = __cmd_inject(&inject);
 
 out_delete:
+	strlist__delete(inject.known_build_ids);
 	zstd_fini(&(inject.session->zstd_data));
 	perf_session__delete(inject.session);
 out_close_output:
-- 
2.37.0.rc0.161.g10f37bed90-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] perf inject: Add a command line option to specify build ids
  2022-07-07 21:56 [PATCH v2] perf inject: Add a command line option to specify build ids Raul Silvera
@ 2022-07-08 18:50 ` Namhyung Kim
  2022-07-09  0:21   ` Raul Silvera
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2022-07-08 18:50 UTC (permalink / raw)
  To: Raul Silvera
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, linux-perf-users,
	linux-kernel

Hi Raul,

On Thu, Jul 7, 2022 at 2:56 PM Raul Silvera <rsilvera@google.com> wrote:
>
> This commit adds the option --known-build-ids to perf inject.
> It allows the user to explicitly specify the build id for a given
> path, instead of retrieving it from the current system. This is
> useful in cases where a perf.data file is processed on a different
> system from where it was collected, or if some of the binaries are
> no longer available.
>
> The build ids and paths are specified in pairs in the command line.
> Using the file:// specifier, build ids can be loaded from a file
> directly generated by perf buildid-list. This is convenient to copy
> build ids from one perf.data file to another.
>
> ** Example: In this example we use perf record to create two
> perf.data files, one with build ids and another without, and use
> perf buildid-list and perf inject to copy the build ids from the
> first file to the second.
>
>  $ perf record ls /tmp
>  $ perf record --no-buildid -o perf.data.no-buildid ls /tmp
>  $ perf buildid-list > /tmp/build-ids.txt
>  $ perf inject -b --known-build-ids='file:///tmp/build-ids.txt' \
>         -i perf.data.no-buildid -o perf.data.buildid

Please add documentation for new options.  Some nit pickings below.

>
> Signed-off-by: Raul Silvera <rsilvera@google.com>
> ---
>  V1 -> V2: Cleaned up patch description, deleted the strlist during
>            cleanup, and updated validation of the build id strings
>
>  tools/perf/builtin-inject.c | 60 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index a75bf11585b5..4efb992ed1a0 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -21,6 +21,7 @@
>  #include "util/data.h"
>  #include "util/auxtrace.h"
>  #include "util/jit.h"
> +#include "util/string2.h"
>  #include "util/symbol.h"
>  #include "util/synthetic-events.h"
>  #include "util/thread.h"
> @@ -35,6 +36,7 @@
>
>  #include <linux/list.h>
>  #include <linux/string.h>
> +#include <ctype.h>
>  #include <errno.h>
>  #include <signal.h>
>
> @@ -59,6 +61,8 @@ struct perf_inject {
>         struct itrace_synth_opts itrace_synth_opts;
>         char                    event_copy[PERF_SAMPLE_MAX_SIZE];
>         struct perf_file_section secs[HEADER_FEAT_BITS];
> +       const char              *known_build_ids_source;

Nit: It doesn't need to be here.  You can use a local variable in
cmd_inject for parsing.


> +       struct strlist          *known_build_ids;
>  };
>
>  struct event_entry {
> @@ -570,9 +574,45 @@ static int dso__read_build_id(struct dso *dso)
>         return dso->has_build_id ? 0 : -1;
>  }
>
> +static bool perf_inject__lookup_known_build_id(struct perf_inject *inject,
> +                                              struct dso *dso)
> +{
> +       struct str_node *pos;
> +       int bid_len;
> +
> +       strlist__for_each_entry(pos, inject->known_build_ids) {
> +               const char *build_id, *dso_name;
> +
> +               build_id = skip_spaces(pos->s);
> +               dso_name = strchr(build_id, ' ');
> +               if (dso_name == NULL)
> +                       continue;
> +               bid_len = dso_name - pos->s;
> +               dso_name = skip_spaces(dso_name);
> +               if (strcmp(dso->long_name, dso_name))
> +                       continue;
> +               if (bid_len % 2 != 0 || bid_len >= SBUILD_ID_SIZE)
> +                       return false;
> +               for (int ix = 0; 2 * ix + 1 < bid_len; ++ix) {
> +                       if (!isxdigit(build_id[2 * ix]) ||
> +                           !isxdigit(build_id[2 * ix + 1]))
> +                               return false;
> +
> +                       dso->bid.data[ix] = (hex(build_id[2 * ix]) << 4 |
> +                                            hex(build_id[2 * ix + 1]));
> +               }
> +               dso->bid.size = bid_len / 2;
> +               dso->has_build_id = 1;
> +               return true;
> +       }
> +       return false;
> +}
> +
>  static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
>                                 struct machine *machine, u8 cpumode, u32 flags)
>  {
> +       struct perf_inject *inject = container_of(tool, struct perf_inject,
> +                                                 tool);
>         int err;
>
>         if (is_anon_memory(dso->long_name) || flags & MAP_HUGETLB)
> @@ -580,6 +620,10 @@ static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
>         if (is_no_dso_memory(dso->long_name))
>                 return 0;
>
> +       if (inject->known_build_ids != NULL &&
> +           perf_inject__lookup_known_build_id(inject, dso))
> +               return 1;
> +
>         if (dso__read_build_id(dso) < 0) {
>                 pr_debug("no build_id found for %s\n", dso->long_name);
>                 return -1;
> @@ -1082,6 +1126,9 @@ int cmd_inject(int argc, const char **argv)
>                             "Inject build-ids into the output stream"),
>                 OPT_BOOLEAN(0, "buildid-all", &inject.build_id_all,
>                             "Inject build-ids of all DSOs into the output stream"),
> +               OPT_STRING(0, "known-build-ids", &inject.known_build_ids_source,
> +                          "buildid path [buildid path...]",
> +                          "build-ids to use for specific files"),
>                 OPT_STRING('i', "input", &inject.input_name, "file",
>                            "input file name"),
>                 OPT_STRING('o', "output", &inject.output.path, "file",
> @@ -1215,6 +1262,18 @@ int cmd_inject(int argc, const char **argv)
>                  */
>                 inject.tool.ordered_events = true;
>                 inject.tool.ordering_requires_timestamps = true;
> +               if (inject.known_build_ids_source != NULL) {
> +                       struct strlist *known_build_ids;

Nit: I think you can use inject.known_build_ids directly.

Thanks,
Namhyung


> +
> +                       known_build_ids = strlist__new(
> +                           inject.known_build_ids_source, NULL);
> +
> +                       if (known_build_ids == NULL) {
> +                               pr_err("Couldn't parse known build ids.\n");
> +                               goto out_delete;
> +                       }
> +                       inject.known_build_ids = known_build_ids;
> +               }
>         }
>
>         if (inject.sched_stat) {
> @@ -1241,6 +1300,7 @@ int cmd_inject(int argc, const char **argv)
>         ret = __cmd_inject(&inject);
>
>  out_delete:
> +       strlist__delete(inject.known_build_ids);
>         zstd_fini(&(inject.session->zstd_data));
>         perf_session__delete(inject.session);
>  out_close_output:
> --
> 2.37.0.rc0.161.g10f37bed90-goog
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] perf inject: Add a command line option to specify build ids
  2022-07-08 18:50 ` Namhyung Kim
@ 2022-07-09  0:21   ` Raul Silvera
  0 siblings, 0 replies; 3+ messages in thread
From: Raul Silvera @ 2022-07-09  0:21 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, linux-perf-users,
	linux-kernel

Thank you. I just uploaded a new version addressing this feedback.

Raul E. Silvera
Software Engineer
waymo.com

On Fri, Jul 8, 2022 at 11:51 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Raul,
>
> On Thu, Jul 7, 2022 at 2:56 PM Raul Silvera <rsilvera@google.com> wrote:
> >
> > This commit adds the option --known-build-ids to perf inject.
> > It allows the user to explicitly specify the build id for a given
> > path, instead of retrieving it from the current system. This is
> > useful in cases where a perf.data file is processed on a different
> > system from where it was collected, or if some of the binaries are
> > no longer available.
> >
> > The build ids and paths are specified in pairs in the command line.
> > Using the file:// specifier, build ids can be loaded from a file
> > directly generated by perf buildid-list. This is convenient to copy
> > build ids from one perf.data file to another.
> >
> > ** Example: In this example we use perf record to create two
> > perf.data files, one with build ids and another without, and use
> > perf buildid-list and perf inject to copy the build ids from the
> > first file to the second.
> >
> >  $ perf record ls /tmp
> >  $ perf record --no-buildid -o perf.data.no-buildid ls /tmp
> >  $ perf buildid-list > /tmp/build-ids.txt
> >  $ perf inject -b --known-build-ids='file:///tmp/build-ids.txt' \
> >         -i perf.data.no-buildid -o perf.data.buildid
>
> Please add documentation for new options.  Some nit pickings below.
>
> >
> > Signed-off-by: Raul Silvera <rsilvera@google.com>
> > ---
> >  V1 -> V2: Cleaned up patch description, deleted the strlist during
> >            cleanup, and updated validation of the build id strings
> >
> >  tools/perf/builtin-inject.c | 60 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 60 insertions(+)
> >
> > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > index a75bf11585b5..4efb992ed1a0 100644
> > --- a/tools/perf/builtin-inject.c
> > +++ b/tools/perf/builtin-inject.c
> > @@ -21,6 +21,7 @@
> >  #include "util/data.h"
> >  #include "util/auxtrace.h"
> >  #include "util/jit.h"
> > +#include "util/string2.h"
> >  #include "util/symbol.h"
> >  #include "util/synthetic-events.h"
> >  #include "util/thread.h"
> > @@ -35,6 +36,7 @@
> >
> >  #include <linux/list.h>
> >  #include <linux/string.h>
> > +#include <ctype.h>
> >  #include <errno.h>
> >  #include <signal.h>
> >
> > @@ -59,6 +61,8 @@ struct perf_inject {
> >         struct itrace_synth_opts itrace_synth_opts;
> >         char                    event_copy[PERF_SAMPLE_MAX_SIZE];
> >         struct perf_file_section secs[HEADER_FEAT_BITS];
> > +       const char              *known_build_ids_source;
>
> Nit: It doesn't need to be here.  You can use a local variable in
> cmd_inject for parsing.
>
>
> > +       struct strlist          *known_build_ids;
> >  };
> >
> >  struct event_entry {
> > @@ -570,9 +574,45 @@ static int dso__read_build_id(struct dso *dso)
> >         return dso->has_build_id ? 0 : -1;
> >  }
> >
> > +static bool perf_inject__lookup_known_build_id(struct perf_inject *inject,
> > +                                              struct dso *dso)
> > +{
> > +       struct str_node *pos;
> > +       int bid_len;
> > +
> > +       strlist__for_each_entry(pos, inject->known_build_ids) {
> > +               const char *build_id, *dso_name;
> > +
> > +               build_id = skip_spaces(pos->s);
> > +               dso_name = strchr(build_id, ' ');
> > +               if (dso_name == NULL)
> > +                       continue;
> > +               bid_len = dso_name - pos->s;
> > +               dso_name = skip_spaces(dso_name);
> > +               if (strcmp(dso->long_name, dso_name))
> > +                       continue;
> > +               if (bid_len % 2 != 0 || bid_len >= SBUILD_ID_SIZE)
> > +                       return false;
> > +               for (int ix = 0; 2 * ix + 1 < bid_len; ++ix) {
> > +                       if (!isxdigit(build_id[2 * ix]) ||
> > +                           !isxdigit(build_id[2 * ix + 1]))
> > +                               return false;
> > +
> > +                       dso->bid.data[ix] = (hex(build_id[2 * ix]) << 4 |
> > +                                            hex(build_id[2 * ix + 1]));
> > +               }
> > +               dso->bid.size = bid_len / 2;
> > +               dso->has_build_id = 1;
> > +               return true;
> > +       }
> > +       return false;
> > +}
> > +
> >  static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
> >                                 struct machine *machine, u8 cpumode, u32 flags)
> >  {
> > +       struct perf_inject *inject = container_of(tool, struct perf_inject,
> > +                                                 tool);
> >         int err;
> >
> >         if (is_anon_memory(dso->long_name) || flags & MAP_HUGETLB)
> > @@ -580,6 +620,10 @@ static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
> >         if (is_no_dso_memory(dso->long_name))
> >                 return 0;
> >
> > +       if (inject->known_build_ids != NULL &&
> > +           perf_inject__lookup_known_build_id(inject, dso))
> > +               return 1;
> > +
> >         if (dso__read_build_id(dso) < 0) {
> >                 pr_debug("no build_id found for %s\n", dso->long_name);
> >                 return -1;
> > @@ -1082,6 +1126,9 @@ int cmd_inject(int argc, const char **argv)
> >                             "Inject build-ids into the output stream"),
> >                 OPT_BOOLEAN(0, "buildid-all", &inject.build_id_all,
> >                             "Inject build-ids of all DSOs into the output stream"),
> > +               OPT_STRING(0, "known-build-ids", &inject.known_build_ids_source,
> > +                          "buildid path [buildid path...]",
> > +                          "build-ids to use for specific files"),
> >                 OPT_STRING('i', "input", &inject.input_name, "file",
> >                            "input file name"),
> >                 OPT_STRING('o', "output", &inject.output.path, "file",
> > @@ -1215,6 +1262,18 @@ int cmd_inject(int argc, const char **argv)
> >                  */
> >                 inject.tool.ordered_events = true;
> >                 inject.tool.ordering_requires_timestamps = true;
> > +               if (inject.known_build_ids_source != NULL) {
> > +                       struct strlist *known_build_ids;
>
> Nit: I think you can use inject.known_build_ids directly.
>
> Thanks,
> Namhyung
>
>
> > +
> > +                       known_build_ids = strlist__new(
> > +                           inject.known_build_ids_source, NULL);
> > +
> > +                       if (known_build_ids == NULL) {
> > +                               pr_err("Couldn't parse known build ids.\n");
> > +                               goto out_delete;
> > +                       }
> > +                       inject.known_build_ids = known_build_ids;
> > +               }
> >         }
> >
> >         if (inject.sched_stat) {
> > @@ -1241,6 +1300,7 @@ int cmd_inject(int argc, const char **argv)
> >         ret = __cmd_inject(&inject);
> >
> >  out_delete:
> > +       strlist__delete(inject.known_build_ids);
> >         zstd_fini(&(inject.session->zstd_data));
> >         perf_session__delete(inject.session);
> >  out_close_output:
> > --
> > 2.37.0.rc0.161.g10f37bed90-goog
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-07-09  0:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-07 21:56 [PATCH v2] perf inject: Add a command line option to specify build ids Raul Silvera
2022-07-08 18:50 ` Namhyung Kim
2022-07-09  0:21   ` Raul Silvera

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).