2019-11-23 16:08:41

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 00/16] scripts/kallsyms: various cleanups and optimizations

Changes in v2:
- Add a new patch that shrinks the table before the sort.

Masahiro Yamada (16):
scripts/kallsyms: remove unneeded #ifndef ARRAY_SIZE
scripts/kallsyms: fix definitely-lost memory leak
scripts/kallsyms: shrink table before sorting it
scripts/kallsyms: set relative_base more effectively
scripts/kallsyms: remove redundant is_arm_mapping_symbol()
scripts/kallsyms: remove unneeded length check for prefix matching
scripts/kallsyms: add sym_name() to mitigate cast ugliness
scripts/kallsyms: replace prefix_underscores_count() with strspn()
scripts/kallsyms: make find_token() return (unsigned char *)
scripts/kallsyms: add const qualifiers where possible
scripts/kallsyms: skip ignored symbols very early
scripts/kallsyms: move more patterns to the ignored_prefixes array
scripts/kallsyms: move ignored symbol types to is_ignored_symbol()
scripts/kallsyms: make check_symbol_range() void function
scripts/kallsyms: put check_symbol_range() calls close together
scripts/kallsyms: remove redundant initializers

scripts/kallsyms.c | 287 ++++++++++++++++++++++-----------------------
1 file changed, 142 insertions(+), 145 deletions(-)

--
2.17.1


2019-11-23 16:08:55

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 06/16] scripts/kallsyms: remove unneeded length check for prefix matching

l <= strlen(sym_name) is unnecessary for prefix matching.
strncmp() will do.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Changes in v2: None

scripts/kallsyms.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 14a50c8d3f34..a57636c6f84f 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -246,8 +246,7 @@ static int symbol_valid(struct sym_entry *s)
for (i = 0; special_prefixes[i]; i++) {
int l = strlen(special_prefixes[i]);

- if (l <= strlen(sym_name) &&
- strncmp(sym_name, special_prefixes[i], l) == 0)
+ if (strncmp(sym_name, special_prefixes[i], l) == 0)
return 0;
}

--
2.17.1

2019-11-23 16:09:14

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 03/16] scripts/kallsyms: shrink table before sorting it

Currently, build_initial_tok_table() trims unused symbols, which is
called after sort_symbol().

It is not efficient to sort the huge table that contains unused entries.
Shrink the table before sorting it.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Changes in v2:
- New patch

scripts/kallsyms.c | 49 +++++++++++++++++++++++++++-------------------
1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 79641874d860..de986eda41a6 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -268,6 +268,30 @@ static int symbol_valid(struct sym_entry *s)
return 1;
}

+/* remove all the invalid symbols from the table */
+static void shrink_table(void)
+{
+ unsigned int i, pos;
+
+ pos = 0;
+ for (i = 0; i < table_cnt; i++) {
+ if (symbol_valid(&table[i])) {
+ if (pos != i)
+ table[pos] = table[i];
+ pos++;
+ } else {
+ free(table[i].sym);
+ }
+ }
+ table_cnt = pos;
+
+ /* When valid symbol is not registered, exit to error */
+ if (!table_cnt) {
+ fprintf(stderr, "No valid symbol.\n");
+ exit(1);
+ }
+}
+
static void read_map(FILE *in)
{
while (!feof(in)) {
@@ -475,23 +499,13 @@ static void forget_symbol(unsigned char *symbol, int len)
token_profit[ symbol[i] + (symbol[i + 1] << 8) ]--;
}

-/* remove all the invalid symbols from the table and do the initial token count */
+/* do the initial token count */
static void build_initial_tok_table(void)
{
- unsigned int i, pos;
+ unsigned int i;

- pos = 0;
- for (i = 0; i < table_cnt; i++) {
- if ( symbol_valid(&table[i]) ) {
- if (pos != i)
- table[pos] = table[i];
- learn_symbol(table[pos].sym, table[pos].len);
- pos++;
- } else {
- free(table[i].sym);
- }
- }
- table_cnt = pos;
+ for (i = 0; i < table_cnt; i++)
+ learn_symbol(table[i].sym, table[i].len);
}

static void *find_token(unsigned char *str, int len, unsigned char *token)
@@ -614,12 +628,6 @@ static void optimize_token_table(void)

insert_real_symbols_in_table();

- /* When valid symbol is not registered, exit to error */
- if (!table_cnt) {
- fprintf(stderr, "No valid symbol.\n");
- exit(1);
- }
-
optimize_result();
}

@@ -756,6 +764,7 @@ int main(int argc, char **argv)
usage();

read_map(stdin);
+ shrink_table();
if (absolute_percpu)
make_percpus_absolute();
if (base_relative)
--
2.17.1

2019-11-23 16:09:54

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 14/16] scripts/kallsyms: make check_symbol_range() void function

There is no more reason to check the return value of
check_symbol_range().

Signed-off-by: Masahiro Yamada <[email protected]>
---

Changes in v2: None

scripts/kallsyms.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index d90a6133d7b8..f4d5f131556d 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -155,8 +155,8 @@ static bool is_ignored_symbol(const char *name, char type)
return false;
}

-static int check_symbol_range(const char *sym, unsigned long long addr,
- struct addr_range *ranges, int entries)
+static void check_symbol_range(const char *sym, unsigned long long addr,
+ struct addr_range *ranges, int entries)
{
size_t i;
struct addr_range *ar;
@@ -166,14 +166,12 @@ static int check_symbol_range(const char *sym, unsigned long long addr,

if (strcmp(sym, ar->start_sym) == 0) {
ar->start = addr;
- return 0;
+ return;
} else if (strcmp(sym, ar->end_sym) == 0) {
ar->end = addr;
- return 0;
+ return;
}
}
-
- return 1;
}

static int read_symbol(FILE *in, struct sym_entry *s)
@@ -200,9 +198,8 @@ static int read_symbol(FILE *in, struct sym_entry *s)
/* Ignore most absolute/undefined (?) symbols. */
if (strcmp(sym, "_text") == 0)
_text = s->addr;
- else if (check_symbol_range(sym, s->addr, text_ranges,
- ARRAY_SIZE(text_ranges)) == 0)
- /* nothing to do */;
+
+ check_symbol_range(sym, s->addr, text_ranges, ARRAY_SIZE(text_ranges));

/* include the type field in the symbol name, so that it gets
* compressed together */
--
2.17.1

2019-11-23 16:10:04

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 09/16] scripts/kallsyms: make find_token() return (unsigned char *)

The callers of this function expect (unsigned char *). I do not see
a good reason to make this function return (void *).

Signed-off-by: Masahiro Yamada <[email protected]>
---

Changes in v2: None

scripts/kallsyms.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 89cc7c098c51..274a77bfbd63 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -503,7 +503,8 @@ static void build_initial_tok_table(void)
learn_symbol(table[i].sym, table[i].len);
}

-static void *find_token(unsigned char *str, int len, unsigned char *token)
+static unsigned char *find_token(unsigned char *str, int len,
+ unsigned char *token)
{
int i;

--
2.17.1

2019-12-07 22:21:27

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH v2 03/16] scripts/kallsyms: shrink table before sorting it

Hi,

On Sat, Nov 23, 2019 at 8:05 AM Masahiro Yamada
<[email protected]> wrote:
>
> Currently, build_initial_tok_table() trims unused symbols, which is
> called after sort_symbol().
>
> It is not efficient to sort the huge table that contains unused entries.
> Shrink the table before sorting it.
>
> Signed-off-by: Masahiro Yamada <[email protected]>

This started showing warnings on some 32-bit ARM platforms, due to
kallsyms_relative_base changing:

kallsyms_relative_base:
- PTR _text - 0
+ PTR _text - 0xfffffffffffffe20

The assembler started complaining:

.tmp_kallsyms1.S: Assembler messages:
.tmp_kallsyms1.S:15315: Warning: right operand is a bignum; integer 0 assumed

Also, I clearly see different output with this patch reverted and
applied; I would expect no actual difference if it was correct.

Can we please revert this for 5.5 while this is being sorted out?

To reproduce, build for example am200epdkit_defconfig for ARCH=arm. I
see it with GCC 8.2.0, binutils 2.30.0.


Thanks,

-Olof