| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 5 May 2020 09:37:02 -0300
From: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
linux-kernel@...r.kernel.org, Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v2 2/3] lib kallsyms: parse using io api
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@...e ~]# perf bench internals kallsyms-parse
# Running 'internals/kallsyms-parse' benchmark:
Average kallsyms__parse took: 80.381 ms (+- 0.115 ms)
[root@...e ~]#
And this pre-existing test uses these routines to parse kallsyms and
then compare with the info obtained from the matching ELF symtab:
[root@...e ~]# perf test vmlinux
1: vmlinux symtab matches kallsyms : Ok
[root@...e ~]#
Signed-off-by: Ian Rogers <irogers@...gle.com>
Tested-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> Signed-off-by: Ian Rogers <irogers@...gle.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
Powered by blists - more mailing lists