* [PATCH v2 0/3] Improvements for kallsyms__parse
@ 2020-05-01 22:13 Ian Rogers
2020-05-01 22:13 ` [PATCH v2 1/3] perf bench: add kallsyms parsing Ian Rogers
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Ian Rogers @ 2020-05-01 22:13 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Thomas Gleixner, linux-kernel
Cc: Stephane Eranian, Ian Rogers
kallsyms__parse is called 4 times during perf record startup. Add a
benchmark to measure its performance. Transition it to using the api
io.h buffered reading, improving performance by ~8% or saving ~5% from
perf record start up time.
v2. Fix the err return value for success, error reported by
jolsa@redhat.com. Add clean-up for hex2u64.
Ian Rogers (3):
perf bench: add kallsyms parsing
lib kallsyms: parse using io api
lib kallsyms: move hex2u64 out of header
tools/lib/api/io.h | 3 ++
tools/lib/symbol/kallsyms.c | 86 ++++++++++++++-----------------
tools/lib/symbol/kallsyms.h | 2 -
tools/perf/bench/Build | 1 +
tools/perf/bench/bench.h | 1 +
tools/perf/bench/kallsyms-parse.c | 75 +++++++++++++++++++++++++++
tools/perf/builtin-bench.c | 1 +
tools/perf/util/symbol.c | 14 +++++
8 files changed, 133 insertions(+), 50 deletions(-)
create mode 100644 tools/perf/bench/kallsyms-parse.c
--
2.26.2.526.g744177e7f7-goog
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/3] perf bench: add kallsyms parsing 2020-05-01 22:13 [PATCH v2 0/3] Improvements for kallsyms__parse Ian Rogers @ 2020-05-01 22:13 ` Ian Rogers 2020-05-08 13:04 ` [tip: perf/core] perf bench: Add " tip-bot2 for Ian Rogers 2020-05-01 22:13 ` [PATCH v2 2/3] lib kallsyms: parse using io api Ian Rogers ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Ian Rogers @ 2020-05-01 22:13 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner, linux-kernel Cc: Stephane Eranian, Ian Rogers Add a benchmark for kallsyms parsing. Example output: Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 103.971 ms (+- 0.121 ms) Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/bench/Build | 1 + tools/perf/bench/bench.h | 1 + tools/perf/bench/kallsyms-parse.c | 75 +++++++++++++++++++++++++++++++ tools/perf/builtin-bench.c | 1 + 4 files changed, 78 insertions(+) create mode 100644 tools/perf/bench/kallsyms-parse.c diff --git a/tools/perf/bench/Build b/tools/perf/bench/Build index 042827385c87..768e408757a0 100644 --- a/tools/perf/bench/Build +++ b/tools/perf/bench/Build @@ -9,6 +9,7 @@ perf-y += futex-lock-pi.o perf-y += epoll-wait.o perf-y += epoll-ctl.o perf-y += synthesize.o +perf-y += kallsyms-parse.o perf-$(CONFIG_X86_64) += mem-memcpy-x86-64-lib.o perf-$(CONFIG_X86_64) += mem-memcpy-x86-64-asm.o diff --git a/tools/perf/bench/bench.h b/tools/perf/bench/bench.h index 4d669c803237..61cae4966cae 100644 --- a/tools/perf/bench/bench.h +++ b/tools/perf/bench/bench.h @@ -44,6 +44,7 @@ int bench_futex_lock_pi(int argc, const char **argv); int bench_epoll_wait(int argc, const char **argv); int bench_epoll_ctl(int argc, const char **argv); int bench_synthesize(int argc, const char **argv); +int bench_kallsyms_parse(int argc, const char **argv); #define BENCH_FORMAT_DEFAULT_STR "default" #define BENCH_FORMAT_DEFAULT 0 diff --git a/tools/perf/bench/kallsyms-parse.c b/tools/perf/bench/kallsyms-parse.c new file mode 100644 index 000000000000..2b0d0f980ae9 --- /dev/null +++ b/tools/perf/bench/kallsyms-parse.c @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Benchmark of /proc/kallsyms parsing. + * + * Copyright 2020 Google LLC. + */ +#include <stdlib.h> +#include "bench.h" +#include "../util/stat.h" +#include <linux/time64.h> +#include <subcmd/parse-options.h> +#include <symbol/kallsyms.h> + +static unsigned int iterations = 100; + +static const struct option options[] = { + OPT_UINTEGER('i', "iterations", &iterations, + "Number of iterations used to compute average"), + OPT_END() +}; + +static const char *const bench_usage[] = { + "perf bench internals kallsyms-parse <options>", + NULL +}; + +static int bench_process_symbol(void *arg __maybe_unused, + const char *name __maybe_unused, + char type __maybe_unused, + u64 start __maybe_unused) +{ + return 0; +} + +static int do_kallsyms_parse(void) +{ + struct timeval start, end, diff; + u64 runtime_us; + unsigned int i; + double time_average, time_stddev; + int err; + struct stats time_stats; + + init_stats(&time_stats); + + for (i = 0; i < iterations; i++) { + gettimeofday(&start, NULL); + err = kallsyms__parse("/proc/kallsyms", NULL, + bench_process_symbol); + if (err) + return err; + + gettimeofday(&end, NULL); + timersub(&end, &start, &diff); + runtime_us = diff.tv_sec * USEC_PER_SEC + diff.tv_usec; + update_stats(&time_stats, runtime_us); + } + + time_average = avg_stats(&time_stats) / USEC_PER_MSEC; + time_stddev = stddev_stats(&time_stats) / USEC_PER_MSEC; + printf(" Average kallsyms__parse took: %.3f ms (+- %.3f ms)\n", + time_average, time_stddev); + return 0; +} + +int bench_kallsyms_parse(int argc, const char **argv) +{ + argc = parse_options(argc, argv, options, bench_usage, 0); + if (argc) { + usage_with_options(bench_usage, options); + exit(EXIT_FAILURE); + } + + return do_kallsyms_parse(); +} diff --git a/tools/perf/builtin-bench.c b/tools/perf/builtin-bench.c index 11c79a8d85d6..083273209c88 100644 --- a/tools/perf/builtin-bench.c +++ b/tools/perf/builtin-bench.c @@ -78,6 +78,7 @@ static struct bench epoll_benchmarks[] = { static struct bench internals_benchmarks[] = { { "synthesize", "Benchmark perf event synthesis", bench_synthesize }, + { "kallsyms-parse", "Benchmark kallsyms parsing", bench_kallsyms_parse }, { NULL, NULL, NULL } }; -- 2.26.2.526.g744177e7f7-goog ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [tip: perf/core] perf bench: Add kallsyms parsing 2020-05-01 22:13 ` [PATCH v2 1/3] perf bench: add kallsyms parsing Ian Rogers @ 2020-05-08 13:04 ` tip-bot2 for Ian Rogers 0 siblings, 0 replies; 12+ messages in thread From: tip-bot2 for Ian Rogers @ 2020-05-08 13:04 UTC (permalink / raw) To: linux-tip-commits Cc: Ian Rogers, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Mark Rutland, Namhyung Kim, Peter Zijlstra, Stephane Eranian, Thomas Gleixner, x86, LKML The following commit has been merged into the perf/core branch of tip: Commit-ID: 51876bd45263f62083bbb823220bfb48909f313a Gitweb: https://git.kernel.org/tip/51876bd45263f62083bbb823220bfb48909f313a Author: Ian Rogers <irogers@google.com> AuthorDate: Fri, 01 May 2020 15:13:13 -07:00 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitterDate: Tue, 05 May 2020 16:35:32 -03:00 perf bench: Add kallsyms parsing Add a benchmark for kallsyms parsing. Example output: Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 103.971 ms (+- 0.121 ms) Committer testing: Test Machine: AMD Ryzen 5 3600X 6-Core Processor [root@five ~]# perf bench internals kallsyms-parse # Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 79.692 ms (+- 0.101 ms) [root@five ~]# perf stat -r5 perf bench internals kallsyms-parse # Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 80.563 ms (+- 0.079 ms) # Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 81.046 ms (+- 0.155 ms) # Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 80.874 ms (+- 0.104 ms) # Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 81.173 ms (+- 0.133 ms) # Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 81.169 ms (+- 0.074 ms) Performance counter stats for 'perf bench internals kallsyms-parse' (5 runs): 8,093.54 msec task-clock # 0.999 CPUs utilized ( +- 0.14% ) 3,165 context-switches # 0.391 K/sec ( +- 0.18% ) 10 cpu-migrations # 0.001 K/sec ( +- 23.13% ) 744 page-faults # 0.092 K/sec ( +- 0.21% ) 34,551,564,954 cycles # 4.269 GHz ( +- 0.05% ) (83.33%) 1,160,584,308 stalled-cycles-frontend # 3.36% frontend cycles idle ( +- 1.60% ) (83.33%) 14,974,323,985 stalled-cycles-backend # 43.34% backend cycles idle ( +- 0.24% ) (83.33%) 58,712,905,705 instructions # 1.70 insn per cycle # 0.26 stalled cycles per insn ( +- 0.01% ) (83.34%) 14,136,433,778 branches # 1746.632 M/sec ( +- 0.01% ) (83.33%) 141,943,217 branch-misses # 1.00% of all branches ( +- 0.04% ) (83.33%) 8.1040 +- 0.0115 seconds time elapsed ( +- 0.14% ) [root@five ~]# Signed-off-by: Ian Rogers <irogers@google.com> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lore.kernel.org/lkml/20200501221315.54715-2-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/bench/Build | 1 +- tools/perf/bench/bench.h | 1 +- tools/perf/bench/kallsyms-parse.c | 75 ++++++++++++++++++++++++++++++- tools/perf/builtin-bench.c | 1 +- 4 files changed, 78 insertions(+) create mode 100644 tools/perf/bench/kallsyms-parse.c diff --git a/tools/perf/bench/Build b/tools/perf/bench/Build index 0428273..768e408 100644 --- a/tools/perf/bench/Build +++ b/tools/perf/bench/Build @@ -9,6 +9,7 @@ perf-y += futex-lock-pi.o perf-y += epoll-wait.o perf-y += epoll-ctl.o perf-y += synthesize.o +perf-y += kallsyms-parse.o perf-$(CONFIG_X86_64) += mem-memcpy-x86-64-lib.o perf-$(CONFIG_X86_64) += mem-memcpy-x86-64-asm.o diff --git a/tools/perf/bench/bench.h b/tools/perf/bench/bench.h index 4d669c8..61cae49 100644 --- a/tools/perf/bench/bench.h +++ b/tools/perf/bench/bench.h @@ -44,6 +44,7 @@ int bench_futex_lock_pi(int argc, const char **argv); int bench_epoll_wait(int argc, const char **argv); int bench_epoll_ctl(int argc, const char **argv); int bench_synthesize(int argc, const char **argv); +int bench_kallsyms_parse(int argc, const char **argv); #define BENCH_FORMAT_DEFAULT_STR "default" #define BENCH_FORMAT_DEFAULT 0 diff --git a/tools/perf/bench/kallsyms-parse.c b/tools/perf/bench/kallsyms-parse.c new file mode 100644 index 0000000..2b0d0f9 --- /dev/null +++ b/tools/perf/bench/kallsyms-parse.c @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Benchmark of /proc/kallsyms parsing. + * + * Copyright 2020 Google LLC. + */ +#include <stdlib.h> +#include "bench.h" +#include "../util/stat.h" +#include <linux/time64.h> +#include <subcmd/parse-options.h> +#include <symbol/kallsyms.h> + +static unsigned int iterations = 100; + +static const struct option options[] = { + OPT_UINTEGER('i', "iterations", &iterations, + "Number of iterations used to compute average"), + OPT_END() +}; + +static const char *const bench_usage[] = { + "perf bench internals kallsyms-parse <options>", + NULL +}; + +static int bench_process_symbol(void *arg __maybe_unused, + const char *name __maybe_unused, + char type __maybe_unused, + u64 start __maybe_unused) +{ + return 0; +} + +static int do_kallsyms_parse(void) +{ + struct timeval start, end, diff; + u64 runtime_us; + unsigned int i; + double time_average, time_stddev; + int err; + struct stats time_stats; + + init_stats(&time_stats); + + for (i = 0; i < iterations; i++) { + gettimeofday(&start, NULL); + err = kallsyms__parse("/proc/kallsyms", NULL, + bench_process_symbol); + if (err) + return err; + + gettimeofday(&end, NULL); + timersub(&end, &start, &diff); + runtime_us = diff.tv_sec * USEC_PER_SEC + diff.tv_usec; + update_stats(&time_stats, runtime_us); + } + + time_average = avg_stats(&time_stats) / USEC_PER_MSEC; + time_stddev = stddev_stats(&time_stats) / USEC_PER_MSEC; + printf(" Average kallsyms__parse took: %.3f ms (+- %.3f ms)\n", + time_average, time_stddev); + return 0; +} + +int bench_kallsyms_parse(int argc, const char **argv) +{ + argc = parse_options(argc, argv, options, bench_usage, 0); + if (argc) { + usage_with_options(bench_usage, options); + exit(EXIT_FAILURE); + } + + return do_kallsyms_parse(); +} diff --git a/tools/perf/builtin-bench.c b/tools/perf/builtin-bench.c index 11c79a8..0832732 100644 --- a/tools/perf/builtin-bench.c +++ b/tools/perf/builtin-bench.c @@ -78,6 +78,7 @@ static struct bench epoll_benchmarks[] = { static struct bench internals_benchmarks[] = { { "synthesize", "Benchmark perf event synthesis", bench_synthesize }, + { "kallsyms-parse", "Benchmark kallsyms parsing", bench_kallsyms_parse }, { NULL, NULL, NULL } }; ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/3] lib kallsyms: parse using io api 2020-05-01 22:13 [PATCH v2 0/3] Improvements for kallsyms__parse Ian Rogers 2020-05-01 22:13 ` [PATCH v2 1/3] perf bench: add kallsyms parsing Ian Rogers @ 2020-05-01 22:13 ` Ian Rogers 2020-05-05 12:37 ` Arnaldo Carvalho de Melo ` (2 more replies) 2020-05-01 22:13 ` [PATCH v2 3/3] lib kallsyms: move hex2u64 out of header Ian Rogers 2020-05-05 12:38 ` [PATCH v2 0/3] Improvements for kallsyms__parse Arnaldo Carvalho de Melo 3 siblings, 3 replies; 12+ messages in thread From: Ian Rogers @ 2020-05-01 22:13 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner, linux-kernel Cc: Stephane Eranian, Ian Rogers Perf record will call kallsyms__parse 4 times during startup and process megabytes of data. This changes kallsyms__parse to use the io library rather than fgets to improve performance of the user code by over 8%. Before: Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 103.988 ms (+- 0.203 ms) After: Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 95.571 ms (+- 0.006 ms) For a workload like: $ perf record /bin/true Run under 'perf record -e cycles:u -g' the time goes from: Before 30.10% 1.67% perf perf [.] kallsyms__parse After 25.55% 20.04% perf perf [.] kallsyms__parse So a little under 5% of the start-up time is removed. A lot of what remains is on the kernel side, but caching kallsyms within perf would at least impact memory footprint. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/lib/api/io.h | 3 ++ tools/lib/symbol/kallsyms.c | 86 ++++++++++++++++--------------------- 2 files changed, 41 insertions(+), 48 deletions(-) diff --git a/tools/lib/api/io.h b/tools/lib/api/io.h index b7e55b5f8a4a..777c20f6b604 100644 --- a/tools/lib/api/io.h +++ b/tools/lib/api/io.h @@ -7,6 +7,9 @@ #ifndef __API_IO__ #define __API_IO__ +#include <stdlib.h> +#include <unistd.h> + struct io { /* File descriptor being read/ */ int fd; diff --git a/tools/lib/symbol/kallsyms.c b/tools/lib/symbol/kallsyms.c index 1a7a9f877095..e335ac2b9e19 100644 --- a/tools/lib/symbol/kallsyms.c +++ b/tools/lib/symbol/kallsyms.c @@ -1,7 +1,9 @@ // SPDX-License-Identifier: GPL-2.0 #include "symbol/kallsyms.h" +#include "api/io.h" #include <stdio.h> -#include <stdlib.h> +#include <sys/stat.h> +#include <fcntl.h> u8 kallsyms2elf_type(char type) { @@ -15,74 +17,62 @@ bool kallsyms__is_function(char symbol_type) return symbol_type == 'T' || symbol_type == 'W'; } -/* - * While we find nice hex chars, build a long_val. - * Return number of chars processed. - */ -int hex2u64(const char *ptr, u64 *long_val) +static void read_to_eol(struct io *io) { - char *p; + int ch; - *long_val = strtoull(ptr, &p, 16); - - return p - ptr; + for (;;) { + ch = io__get_char(io); + if (ch < 0 || ch == '\n') + return; + } } int kallsyms__parse(const char *filename, void *arg, int (*process_symbol)(void *arg, const char *name, char type, u64 start)) { - char *line = NULL; - size_t n; - int err = -1; - FILE *file = fopen(filename, "r"); - - if (file == NULL) - goto out_failure; - - err = 0; + struct io io; + char bf[BUFSIZ]; + int err; - while (!feof(file)) { - u64 start; - int line_len, len; - char symbol_type; - char *symbol_name; + io.fd = open(filename, O_RDONLY, 0); - line_len = getline(&line, &n, file); - if (line_len < 0 || !line) - break; + if (io.fd < 0) + return -1; - line[--line_len] = '\0'; /* \n */ + io__init(&io, io.fd, bf, sizeof(bf)); - len = hex2u64(line, &start); + err = 0; + while (!io.eof) { + __u64 start; + int ch; + size_t i; + char symbol_type; + char symbol_name[KSYM_NAME_LEN + 1]; - /* Skip the line if we failed to parse the address. */ - if (!len) + if (io__get_hex(&io, &start) != ' ') { + read_to_eol(&io); continue; - - len++; - if (len + 2 >= line_len) + } + symbol_type = io__get_char(&io); + if (io__get_char(&io) != ' ') { + read_to_eol(&io); continue; - - symbol_type = line[len]; - len += 2; - symbol_name = line + len; - len = line_len - len; - - if (len >= KSYM_NAME_LEN) { - err = -1; - break; } + for (i = 0; i < sizeof(symbol_name); i++) { + ch = io__get_char(&io); + if (ch < 0 || ch == '\n') + break; + symbol_name[i] = ch; + } + symbol_name[i] = '\0'; err = process_symbol(arg, symbol_name, symbol_type, start); if (err) break; } - free(line); - fclose(file); + close(io.fd); return err; - -out_failure: - return -1; } -- 2.26.2.526.g744177e7f7-goog ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/3] lib kallsyms: parse using io api 2020-05-01 22:13 ` [PATCH v2 2/3] lib kallsyms: parse using io api Ian Rogers @ 2020-05-05 12:37 ` Arnaldo Carvalho de Melo 2020-05-05 14:25 ` Jiri Olsa 2020-05-08 13:04 ` [tip: perf/core] libsymbols kallsyms: Parse " tip-bot2 for Ian Rogers 2 siblings, 0 replies; 12+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-05 12:37 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner, linux-kernel, Stephane Eranian Em Fri, May 01, 2020 at 03:13:14PM -0700, Ian Rogers escreveu: > Perf record will call kallsyms__parse 4 times during startup and process > megabytes of data. This changes kallsyms__parse to use the io library > rather than fgets to improve performance of the user code by over 8%. > > Before: > Running 'internals/kallsyms-parse' benchmark: > Average kallsyms__parse took: 103.988 ms (+- 0.203 ms) > After: > Running 'internals/kallsyms-parse' benchmark: > Average kallsyms__parse took: 95.571 ms (+- 0.006 ms) > > For a workload like: > $ perf record /bin/true > Run under 'perf record -e cycles:u -g' the time goes from: > Before > 30.10% 1.67% perf perf [.] kallsyms__parse > After > 25.55% 20.04% perf perf [.] kallsyms__parse > So a little under 5% of the start-up time is removed. A lot of what > remains is on the kernel side, but caching kallsyms within perf would > at least impact memory footprint. Applied and added this to the commit log: Committer notes: The internal/kallsyms-parse bench is run using: [root@five ~]# perf bench internals kallsyms-parse # Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 80.381 ms (+- 0.115 ms) [root@five ~]# And this pre-existing test uses these routines to parse kallsyms and then compare with the info obtained from the matching ELF symtab: [root@five ~]# perf test vmlinux 1: vmlinux symtab matches kallsyms : Ok [root@five ~]# Signed-off-by: Ian Rogers <irogers@google.com> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com> > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/lib/api/io.h | 3 ++ > tools/lib/symbol/kallsyms.c | 86 ++++++++++++++++--------------------- > 2 files changed, 41 insertions(+), 48 deletions(-) > > diff --git a/tools/lib/api/io.h b/tools/lib/api/io.h > index b7e55b5f8a4a..777c20f6b604 100644 > --- a/tools/lib/api/io.h > +++ b/tools/lib/api/io.h > @@ -7,6 +7,9 @@ > #ifndef __API_IO__ > #define __API_IO__ > > +#include <stdlib.h> > +#include <unistd.h> > + > struct io { > /* File descriptor being read/ */ > int fd; > diff --git a/tools/lib/symbol/kallsyms.c b/tools/lib/symbol/kallsyms.c > index 1a7a9f877095..e335ac2b9e19 100644 > --- a/tools/lib/symbol/kallsyms.c > +++ b/tools/lib/symbol/kallsyms.c > @@ -1,7 +1,9 @@ > // SPDX-License-Identifier: GPL-2.0 > #include "symbol/kallsyms.h" > +#include "api/io.h" > #include <stdio.h> > -#include <stdlib.h> > +#include <sys/stat.h> > +#include <fcntl.h> > > u8 kallsyms2elf_type(char type) > { > @@ -15,74 +17,62 @@ bool kallsyms__is_function(char symbol_type) > return symbol_type == 'T' || symbol_type == 'W'; > } > > -/* > - * While we find nice hex chars, build a long_val. > - * Return number of chars processed. > - */ > -int hex2u64(const char *ptr, u64 *long_val) > +static void read_to_eol(struct io *io) > { > - char *p; > + int ch; > > - *long_val = strtoull(ptr, &p, 16); > - > - return p - ptr; > + for (;;) { > + ch = io__get_char(io); > + if (ch < 0 || ch == '\n') > + return; > + } > } > > int kallsyms__parse(const char *filename, void *arg, > int (*process_symbol)(void *arg, const char *name, > char type, u64 start)) > { > - char *line = NULL; > - size_t n; > - int err = -1; > - FILE *file = fopen(filename, "r"); > - > - if (file == NULL) > - goto out_failure; > - > - err = 0; > + struct io io; > + char bf[BUFSIZ]; > + int err; > > - while (!feof(file)) { > - u64 start; > - int line_len, len; > - char symbol_type; > - char *symbol_name; > + io.fd = open(filename, O_RDONLY, 0); > > - line_len = getline(&line, &n, file); > - if (line_len < 0 || !line) > - break; > + if (io.fd < 0) > + return -1; > > - line[--line_len] = '\0'; /* \n */ > + io__init(&io, io.fd, bf, sizeof(bf)); > > - len = hex2u64(line, &start); > + err = 0; > + while (!io.eof) { > + __u64 start; > + int ch; > + size_t i; > + char symbol_type; > + char symbol_name[KSYM_NAME_LEN + 1]; > > - /* Skip the line if we failed to parse the address. */ > - if (!len) > + if (io__get_hex(&io, &start) != ' ') { > + read_to_eol(&io); > continue; > - > - len++; > - if (len + 2 >= line_len) > + } > + symbol_type = io__get_char(&io); > + if (io__get_char(&io) != ' ') { > + read_to_eol(&io); > continue; > - > - symbol_type = line[len]; > - len += 2; > - symbol_name = line + len; > - len = line_len - len; > - > - if (len >= KSYM_NAME_LEN) { > - err = -1; > - break; > } > + for (i = 0; i < sizeof(symbol_name); i++) { > + ch = io__get_char(&io); > + if (ch < 0 || ch == '\n') > + break; > + symbol_name[i] = ch; > + } > + symbol_name[i] = '\0'; > > err = process_symbol(arg, symbol_name, symbol_type, start); > if (err) > break; > } > > - free(line); > - fclose(file); > + close(io.fd); > return err; > - > -out_failure: > - return -1; > } > -- > 2.26.2.526.g744177e7f7-goog > -- - Arnaldo ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/3] lib kallsyms: parse using io api 2020-05-01 22:13 ` [PATCH v2 2/3] lib kallsyms: parse using io api Ian Rogers 2020-05-05 12:37 ` Arnaldo Carvalho de Melo @ 2020-05-05 14:25 ` Jiri Olsa 2020-05-05 14:37 ` Ian Rogers 2020-05-05 16:13 ` Arnaldo Carvalho de Melo 2020-05-08 13:04 ` [tip: perf/core] libsymbols kallsyms: Parse " tip-bot2 for Ian Rogers 2 siblings, 2 replies; 12+ messages in thread From: Jiri Olsa @ 2020-05-05 14:25 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Namhyung Kim, Thomas Gleixner, linux-kernel, Stephane Eranian On Fri, May 01, 2020 at 03:13:14PM -0700, Ian Rogers wrote: SNIP > diff --git a/tools/lib/symbol/kallsyms.c b/tools/lib/symbol/kallsyms.c > index 1a7a9f877095..e335ac2b9e19 100644 > --- a/tools/lib/symbol/kallsyms.c > +++ b/tools/lib/symbol/kallsyms.c > @@ -1,7 +1,9 @@ > // SPDX-License-Identifier: GPL-2.0 > #include "symbol/kallsyms.h" > +#include "api/io.h" > #include <stdio.h> > -#include <stdlib.h> > +#include <sys/stat.h> > +#include <fcntl.h> > > u8 kallsyms2elf_type(char type) > { > @@ -15,74 +17,62 @@ bool kallsyms__is_function(char symbol_type) > return symbol_type == 'T' || symbol_type == 'W'; > } > > -/* > - * While we find nice hex chars, build a long_val. > - * Return number of chars processed. > - */ > -int hex2u64(const char *ptr, u64 *long_val) hi, when you remove in here hex2u64, you'll break the compile: LINK perf /usr/bin/ld: perf-in.o: in function `modules__parse': /home/jolsa/linux-perf/tools/perf/util/symbol.c:607: undefined reference to `hex2u64' /usr/bin/ld: /home/jolsa/linux-perf/tools/perf/util/symbol.c:607: undefined reference to `hex2u64' /usr/bin/ld: perf-in.o: in function `dso__load_perf_map': /home/jolsa/linux-perf/tools/perf/util/symbol.c:1477: undefined reference to `hex2u64' /usr/bin/ld: /home/jolsa/linux-perf/tools/perf/util/symbol.c:1483: undefined reference to `hex2u64' collect2: error: ld returned 1 exit status make[2]: *** [Makefile.perf:629: perf] Error 1 make[1]: *** [Makefile.perf:225: sub-make] Error 2 make: *** [Makefile:70: all] Error 2 that hex2u64 move needs to come before this change jirka ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/3] lib kallsyms: parse using io api 2020-05-05 14:25 ` Jiri Olsa @ 2020-05-05 14:37 ` Ian Rogers 2020-05-05 16:13 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 12+ messages in thread From: Ian Rogers @ 2020-05-05 14:37 UTC (permalink / raw) To: Jiri Olsa Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Namhyung Kim, Thomas Gleixner, LKML, Stephane Eranian On Tue, May 5, 2020 at 7:25 AM Jiri Olsa <jolsa@redhat.com> wrote: > > On Fri, May 01, 2020 at 03:13:14PM -0700, Ian Rogers wrote: > > SNIP > > > diff --git a/tools/lib/symbol/kallsyms.c b/tools/lib/symbol/kallsyms.c > > index 1a7a9f877095..e335ac2b9e19 100644 > > --- a/tools/lib/symbol/kallsyms.c > > +++ b/tools/lib/symbol/kallsyms.c > > @@ -1,7 +1,9 @@ > > // SPDX-License-Identifier: GPL-2.0 > > #include "symbol/kallsyms.h" > > +#include "api/io.h" > > #include <stdio.h> > > -#include <stdlib.h> > > +#include <sys/stat.h> > > +#include <fcntl.h> > > > > u8 kallsyms2elf_type(char type) > > { > > @@ -15,74 +17,62 @@ bool kallsyms__is_function(char symbol_type) > > return symbol_type == 'T' || symbol_type == 'W'; > > } > > > > -/* > > - * While we find nice hex chars, build a long_val. > > - * Return number of chars processed. > > - */ > > -int hex2u64(const char *ptr, u64 *long_val) > > hi, > when you remove in here hex2u64, you'll break the compile: > > LINK perf > /usr/bin/ld: perf-in.o: in function `modules__parse': > /home/jolsa/linux-perf/tools/perf/util/symbol.c:607: undefined reference to `hex2u64' > /usr/bin/ld: /home/jolsa/linux-perf/tools/perf/util/symbol.c:607: undefined reference to `hex2u64' > /usr/bin/ld: perf-in.o: in function `dso__load_perf_map': > /home/jolsa/linux-perf/tools/perf/util/symbol.c:1477: undefined reference to `hex2u64' > /usr/bin/ld: /home/jolsa/linux-perf/tools/perf/util/symbol.c:1483: undefined reference to `hex2u64' > collect2: error: ld returned 1 exit status > make[2]: *** [Makefile.perf:629: perf] Error 1 > make[1]: *** [Makefile.perf:225: sub-make] Error 2 > make: *** [Makefile:70: all] Error 2 > > that hex2u64 move needs to come before this change Good catch. Fixed in v3. https://lore.kernel.org/lkml/20200505143625.147832-1-irogers@google.com/T/#t Thanks, Ian > jirka > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/3] lib kallsyms: parse using io api 2020-05-05 14:25 ` Jiri Olsa 2020-05-05 14:37 ` Ian Rogers @ 2020-05-05 16:13 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 12+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-05 16:13 UTC (permalink / raw) To: Jiri Olsa Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Namhyung Kim, Thomas Gleixner, linux-kernel, Stephane Eranian Em Tue, May 05, 2020 at 04:25:21PM +0200, Jiri Olsa escreveu: > On Fri, May 01, 2020 at 03:13:14PM -0700, Ian Rogers wrote: > > SNIP > > > diff --git a/tools/lib/symbol/kallsyms.c b/tools/lib/symbol/kallsyms.c > > index 1a7a9f877095..e335ac2b9e19 100644 > > --- a/tools/lib/symbol/kallsyms.c > > +++ b/tools/lib/symbol/kallsyms.c > > @@ -1,7 +1,9 @@ > > // SPDX-License-Identifier: GPL-2.0 > > #include "symbol/kallsyms.h" > > +#include "api/io.h" > > #include <stdio.h> > > -#include <stdlib.h> > > +#include <sys/stat.h> > > +#include <fcntl.h> > > > > u8 kallsyms2elf_type(char type) > > { > > @@ -15,74 +17,62 @@ bool kallsyms__is_function(char symbol_type) > > return symbol_type == 'T' || symbol_type == 'W'; > > } > > > > -/* > > - * While we find nice hex chars, build a long_val. > > - * Return number of chars processed. > > - */ > > -int hex2u64(const char *ptr, u64 *long_val) > > hi, > when you remove in here hex2u64, you'll break the compile: > > LINK perf > /usr/bin/ld: perf-in.o: in function `modules__parse': > /home/jolsa/linux-perf/tools/perf/util/symbol.c:607: undefined reference to `hex2u64' > /usr/bin/ld: /home/jolsa/linux-perf/tools/perf/util/symbol.c:607: undefined reference to `hex2u64' > /usr/bin/ld: perf-in.o: in function `dso__load_perf_map': > /home/jolsa/linux-perf/tools/perf/util/symbol.c:1477: undefined reference to `hex2u64' > /usr/bin/ld: /home/jolsa/linux-perf/tools/perf/util/symbol.c:1483: undefined reference to `hex2u64' > collect2: error: ld returned 1 exit status > make[2]: *** [Makefile.perf:629: perf] Error 1 > make[1]: *** [Makefile.perf:225: sub-make] Error 2 > make: *** [Makefile:70: all] Error 2 > > that hex2u64 move needs to come before this change I noticed this and fixed it all, was about to send this note I've added: --------------- Also we can't remove hex2u64() in this patch as this breaks the build: /usr/bin/ld: /tmp/build/perf/perf-in.o: in function `modules__parse': /home/acme/git/perf/tools/perf/util/symbol.c:607: undefined reference to `hex2u64' /usr/bin/ld: /home/acme/git/perf/tools/perf/util/symbol.c:607: undefined reference to `hex2u64' /usr/bin/ld: /tmp/build/perf/perf-in.o: in function `dso__load_perf_map': /home/acme/git/perf/tools/perf/util/symbol.c:1477: undefined reference to `hex2u64' /usr/bin/ld: /home/acme/git/perf/tools/perf/util/symbol.c:1483: undefined reference to `hex2u64' collect2: error: ld returned 1 exit status Leave it there, move it in the next patch. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [tip: perf/core] libsymbols kallsyms: Parse using io api 2020-05-01 22:13 ` [PATCH v2 2/3] lib kallsyms: parse using io api Ian Rogers 2020-05-05 12:37 ` Arnaldo Carvalho de Melo 2020-05-05 14:25 ` Jiri Olsa @ 2020-05-08 13:04 ` tip-bot2 for Ian Rogers 2 siblings, 0 replies; 12+ messages in thread From: tip-bot2 for Ian Rogers @ 2020-05-08 13:04 UTC (permalink / raw) To: linux-tip-commits Cc: Ian Rogers, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa, Mark Rutland, Namhyung Kim, Peter Zijlstra, Stephane Eranian, Thomas Gleixner, x86, LKML The following commit has been merged into the perf/core branch of tip: Commit-ID: 53df2b93441289848f5c2e76d19d1569816b2b9d Gitweb: https://git.kernel.org/tip/53df2b93441289848f5c2e76d19d1569816b2b9d Author: Ian Rogers <irogers@google.com> AuthorDate: Fri, 01 May 2020 15:13:14 -07:00 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitterDate: Tue, 05 May 2020 16:35:32 -03:00 libsymbols kallsyms: Parse using io api 'perf record' will call kallsyms__parse 4 times during startup and process megabytes of data. This changes kallsyms__parse to use the io library rather than fgets to improve performance of the user code by over 8%. Before: Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 103.988 ms (+- 0.203 ms) After: Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 95.571 ms (+- 0.006 ms) For a workload like: $ perf record /bin/true Run under 'perf record -e cycles:u -g' the time goes from: Before 30.10% 1.67% perf perf [.] kallsyms__parse After 25.55% 20.04% perf perf [.] kallsyms__parse So a little under 5% of the start-up time is removed. A lot of what remains is on the kernel side, but caching kallsyms within perf would at least impact memory footprint. Committer notes: The internal/kallsyms-parse bench is run using: [root@five ~]# perf bench internals kallsyms-parse # Running 'internals/kallsyms-parse' benchmark: Average kallsyms__parse took: 80.381 ms (+- 0.115 ms) [root@five ~]# And this pre-existing test uses these routines to parse kallsyms and then compare with the info obtained from the matching ELF symtab: [root@five ~]# perf test vmlinux 1: vmlinux symtab matches kallsyms : Ok [root@five ~]# Also we can't remove hex2u64() in this patch as this breaks the build: /usr/bin/ld: /tmp/build/perf/perf-in.o: in function `modules__parse': /home/acme/git/perf/tools/perf/util/symbol.c:607: undefined reference to `hex2u64' /usr/bin/ld: /home/acme/git/perf/tools/perf/util/symbol.c:607: undefined reference to `hex2u64' /usr/bin/ld: /tmp/build/perf/perf-in.o: in function `dso__load_perf_map': /home/acme/git/perf/tools/perf/util/symbol.c:1477: undefined reference to `hex2u64' /usr/bin/ld: /home/acme/git/perf/tools/perf/util/symbol.c:1483: undefined reference to `hex2u64' collect2: error: ld returned 1 exit status Leave it there, move it in the next patch. Signed-off-by: Ian Rogers <irogers@google.com> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lore.kernel.org/lkml/20200501221315.54715-3-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/lib/api/io.h | 3 +- tools/lib/symbol/kallsyms.c | 93 ++++++++++++++++++------------------ 2 files changed, 51 insertions(+), 45 deletions(-) diff --git a/tools/lib/api/io.h b/tools/lib/api/io.h index b7e55b5..777c20f 100644 --- a/tools/lib/api/io.h +++ b/tools/lib/api/io.h @@ -7,6 +7,9 @@ #ifndef __API_IO__ #define __API_IO__ +#include <stdlib.h> +#include <unistd.h> + struct io { /* File descriptor being read/ */ int fd; diff --git a/tools/lib/symbol/kallsyms.c b/tools/lib/symbol/kallsyms.c index 1a7a9f8..a5edc75 100644 --- a/tools/lib/symbol/kallsyms.c +++ b/tools/lib/symbol/kallsyms.c @@ -1,7 +1,9 @@ // SPDX-License-Identifier: GPL-2.0 #include "symbol/kallsyms.h" +#include "api/io.h" #include <stdio.h> -#include <stdlib.h> +#include <sys/stat.h> +#include <fcntl.h> u8 kallsyms2elf_type(char type) { @@ -9,12 +11,6 @@ u8 kallsyms2elf_type(char type) return (type == 't' || type == 'w') ? STT_FUNC : STT_OBJECT; } -bool kallsyms__is_function(char symbol_type) -{ - symbol_type = toupper(symbol_type); - return symbol_type == 'T' || symbol_type == 'W'; -} - /* * While we find nice hex chars, build a long_val. * Return number of chars processed. @@ -28,61 +24,68 @@ int hex2u64(const char *ptr, u64 *long_val) return p - ptr; } +bool kallsyms__is_function(char symbol_type) +{ + symbol_type = toupper(symbol_type); + return symbol_type == 'T' || symbol_type == 'W'; +} + +static void read_to_eol(struct io *io) +{ + int ch; + + for (;;) { + ch = io__get_char(io); + if (ch < 0 || ch == '\n') + return; + } +} + int kallsyms__parse(const char *filename, void *arg, int (*process_symbol)(void *arg, const char *name, char type, u64 start)) { - char *line = NULL; - size_t n; - int err = -1; - FILE *file = fopen(filename, "r"); - - if (file == NULL) - goto out_failure; - - err = 0; + struct io io; + char bf[BUFSIZ]; + int err; - while (!feof(file)) { - u64 start; - int line_len, len; - char symbol_type; - char *symbol_name; + io.fd = open(filename, O_RDONLY, 0); - line_len = getline(&line, &n, file); - if (line_len < 0 || !line) - break; + if (io.fd < 0) + return -1; - line[--line_len] = '\0'; /* \n */ + io__init(&io, io.fd, bf, sizeof(bf)); - len = hex2u64(line, &start); + err = 0; + while (!io.eof) { + __u64 start; + int ch; + size_t i; + char symbol_type; + char symbol_name[KSYM_NAME_LEN + 1]; - /* Skip the line if we failed to parse the address. */ - if (!len) + if (io__get_hex(&io, &start) != ' ') { + read_to_eol(&io); continue; - - len++; - if (len + 2 >= line_len) + } + symbol_type = io__get_char(&io); + if (io__get_char(&io) != ' ') { + read_to_eol(&io); continue; - - symbol_type = line[len]; - len += 2; - symbol_name = line + len; - len = line_len - len; - - if (len >= KSYM_NAME_LEN) { - err = -1; - break; } + for (i = 0; i < sizeof(symbol_name); i++) { + ch = io__get_char(&io); + if (ch < 0 || ch == '\n') + break; + symbol_name[i] = ch; + } + symbol_name[i] = '\0'; err = process_symbol(arg, symbol_name, symbol_type, start); if (err) break; } - free(line); - fclose(file); + close(io.fd); return err; - -out_failure: - return -1; } ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/3] lib kallsyms: move hex2u64 out of header 2020-05-01 22:13 [PATCH v2 0/3] Improvements for kallsyms__parse Ian Rogers 2020-05-01 22:13 ` [PATCH v2 1/3] perf bench: add kallsyms parsing Ian Rogers 2020-05-01 22:13 ` [PATCH v2 2/3] lib kallsyms: parse using io api Ian Rogers @ 2020-05-01 22:13 ` Ian Rogers 2020-05-08 13:04 ` [tip: perf/core] libsymbols kallsyms: Move " tip-bot2 for Ian Rogers 2020-05-05 12:38 ` [PATCH v2 0/3] Improvements for kallsyms__parse Arnaldo Carvalho de Melo 3 siblings, 1 reply; 12+ messages in thread From: Ian Rogers @ 2020-05-01 22:13 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner, linux-kernel Cc: Stephane Eranian, Ian Rogers hex2u64 is a helper that's out of place in kallsyms.h as not being kallsyms related. Move from kallsyms.h to the only user. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/lib/symbol/kallsyms.h | 2 -- tools/perf/util/symbol.c | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/tools/lib/symbol/kallsyms.h b/tools/lib/symbol/kallsyms.h index bd988f7b18d4..72ab9870454b 100644 --- a/tools/lib/symbol/kallsyms.h +++ b/tools/lib/symbol/kallsyms.h @@ -18,8 +18,6 @@ static inline u8 kallsyms2elf_binding(char type) return isupper(type) ? STB_GLOBAL : STB_LOCAL; } -int hex2u64(const char *ptr, u64 *long_val); - u8 kallsyms2elf_type(char type); bool kallsyms__is_function(char symbol_type); diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 8f4300492dc7..381da6b39f89 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -566,6 +566,20 @@ void dso__sort_by_name(struct dso *dso) return symbols__sort_by_name(&dso->symbol_names, &dso->symbols); } +/* + * While we find nice hex chars, build a long_val. + * Return number of chars processed. + */ +static int hex2u64(const char *ptr, u64 *long_val) +{ + char *p; + + *long_val = strtoull(ptr, &p, 16); + + return p - ptr; +} + + int modules__parse(const char *filename, void *arg, int (*process_module)(void *arg, const char *name, u64 start, u64 size)) -- 2.26.2.526.g744177e7f7-goog ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [tip: perf/core] libsymbols kallsyms: Move hex2u64 out of header 2020-05-01 22:13 ` [PATCH v2 3/3] lib kallsyms: move hex2u64 out of header Ian Rogers @ 2020-05-08 13:04 ` tip-bot2 for Ian Rogers 0 siblings, 0 replies; 12+ messages in thread From: tip-bot2 for Ian Rogers @ 2020-05-08 13:04 UTC (permalink / raw) To: linux-tip-commits Cc: Ian Rogers, Alexander Shishkin, Jiri Olsa, Mark Rutland, Namhyung Kim, Peter Zijlstra, Stephane Eranian, Thomas Gleixner, Arnaldo Carvalho de Melo, x86, LKML The following commit has been merged into the perf/core branch of tip: Commit-ID: 32add10f9597f2e4e46fa8342a83ae7bdfefd1ae Gitweb: https://git.kernel.org/tip/32add10f9597f2e4e46fa8342a83ae7bdfefd1ae Author: Ian Rogers <irogers@google.com> AuthorDate: Fri, 01 May 2020 15:13:15 -07:00 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitterDate: Tue, 05 May 2020 16:35:32 -03:00 libsymbols kallsyms: Move hex2u64 out of header hex2u64 is a helper that's out of place in kallsyms.h as not being kallsyms related. Move from kallsyms.h to the only user. Committer notes: Move it out of tools/lib/symbol/kallsyms.c as well, as we had to leave it there in the previous patch lest we break the build. Signed-off-by: Ian Rogers <irogers@google.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lore.kernel.org/lkml/20200501221315.54715-4-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/lib/symbol/kallsyms.c | 13 ------------- tools/lib/symbol/kallsyms.h | 2 -- tools/perf/util/symbol.c | 14 ++++++++++++++ 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/tools/lib/symbol/kallsyms.c b/tools/lib/symbol/kallsyms.c index a5edc75..e335ac2 100644 --- a/tools/lib/symbol/kallsyms.c +++ b/tools/lib/symbol/kallsyms.c @@ -11,19 +11,6 @@ u8 kallsyms2elf_type(char type) return (type == 't' || type == 'w') ? STT_FUNC : STT_OBJECT; } -/* - * While we find nice hex chars, build a long_val. - * Return number of chars processed. - */ -int hex2u64(const char *ptr, u64 *long_val) -{ - char *p; - - *long_val = strtoull(ptr, &p, 16); - - return p - ptr; -} - bool kallsyms__is_function(char symbol_type) { symbol_type = toupper(symbol_type); diff --git a/tools/lib/symbol/kallsyms.h b/tools/lib/symbol/kallsyms.h index bd988f7..72ab987 100644 --- a/tools/lib/symbol/kallsyms.h +++ b/tools/lib/symbol/kallsyms.h @@ -18,8 +18,6 @@ static inline u8 kallsyms2elf_binding(char type) return isupper(type) ? STB_GLOBAL : STB_LOCAL; } -int hex2u64(const char *ptr, u64 *long_val); - u8 kallsyms2elf_type(char type); bool kallsyms__is_function(char symbol_type); diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 8f43004..381da6b 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -566,6 +566,20 @@ void dso__sort_by_name(struct dso *dso) return symbols__sort_by_name(&dso->symbol_names, &dso->symbols); } +/* + * While we find nice hex chars, build a long_val. + * Return number of chars processed. + */ +static int hex2u64(const char *ptr, u64 *long_val) +{ + char *p; + + *long_val = strtoull(ptr, &p, 16); + + return p - ptr; +} + + int modules__parse(const char *filename, void *arg, int (*process_module)(void *arg, const char *name, u64 start, u64 size)) ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] Improvements for kallsyms__parse 2020-05-01 22:13 [PATCH v2 0/3] Improvements for kallsyms__parse Ian Rogers ` (2 preceding siblings ...) 2020-05-01 22:13 ` [PATCH v2 3/3] lib kallsyms: move hex2u64 out of header Ian Rogers @ 2020-05-05 12:38 ` Arnaldo Carvalho de Melo 3 siblings, 0 replies; 12+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-05 12:38 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner, linux-kernel, Stephane Eranian Em Fri, May 01, 2020 at 03:13:12PM -0700, Ian Rogers escreveu: > kallsyms__parse is called 4 times during perf record startup. Add a > benchmark to measure its performance. Transition it to using the api > io.h buffered reading, improving performance by ~8% or saving ~5% from > perf record start up time. > > v2. Fix the err return value for success, error reported by > jolsa@redhat.com. Add clean-up for hex2u64. Thanks, applied. Jiri, please let me know if I can stick your Acked-by or, better, Reviewed-by there, Namhyung, others, please consider doing the same :-) - Arnaldo > Ian Rogers (3): > perf bench: add kallsyms parsing > lib kallsyms: parse using io api > lib kallsyms: move hex2u64 out of header > > tools/lib/api/io.h | 3 ++ > tools/lib/symbol/kallsyms.c | 86 ++++++++++++++----------------- > tools/lib/symbol/kallsyms.h | 2 - > tools/perf/bench/Build | 1 + > tools/perf/bench/bench.h | 1 + > tools/perf/bench/kallsyms-parse.c | 75 +++++++++++++++++++++++++++ > tools/perf/builtin-bench.c | 1 + > tools/perf/util/symbol.c | 14 +++++ > 8 files changed, 133 insertions(+), 50 deletions(-) > create mode 100644 tools/perf/bench/kallsyms-parse.c > > -- > 2.26.2.526.g744177e7f7-goog > -- - Arnaldo ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-05-08 13:12 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-01 22:13 [PATCH v2 0/3] Improvements for kallsyms__parse Ian Rogers 2020-05-01 22:13 ` [PATCH v2 1/3] perf bench: add kallsyms parsing Ian Rogers 2020-05-08 13:04 ` [tip: perf/core] perf bench: Add " tip-bot2 for Ian Rogers 2020-05-01 22:13 ` [PATCH v2 2/3] lib kallsyms: parse using io api Ian Rogers 2020-05-05 12:37 ` Arnaldo Carvalho de Melo 2020-05-05 14:25 ` Jiri Olsa 2020-05-05 14:37 ` Ian Rogers 2020-05-05 16:13 ` Arnaldo Carvalho de Melo 2020-05-08 13:04 ` [tip: perf/core] libsymbols kallsyms: Parse " tip-bot2 for Ian Rogers 2020-05-01 22:13 ` [PATCH v2 3/3] lib kallsyms: move hex2u64 out of header Ian Rogers 2020-05-08 13:04 ` [tip: perf/core] libsymbols kallsyms: Move " tip-bot2 for Ian Rogers 2020-05-05 12:38 ` [PATCH v2 0/3] Improvements for kallsyms__parse Arnaldo Carvalho de Melo
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).