All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] scripts/kallsyms: rename local variables in read_symbol()
@ 2020-02-02  5:09 Masahiro Yamada
  2020-02-02  5:09 ` [PATCH 2/3] scripts/kallsyms: change table to store (strcut sym_entry *) Masahiro Yamada
  2020-02-02  5:09 ` [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[] Masahiro Yamada
  0 siblings, 2 replies; 6+ messages in thread
From: Masahiro Yamada @ 2020-02-02  5:09 UTC (permalink / raw)
  To: linux-kbuild; +Cc: Masahiro Yamada, linux-kernel

I will use 'sym' for the point to struce sym_entry in the next commit.
Rename 'sym', 'stype' to 'name', 'type', which are more intuitive.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kallsyms.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 94153732ec00..5c34edd98b3e 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -176,43 +176,43 @@ static void check_symbol_range(const char *sym, unsigned long long addr,
 
 static int read_symbol(FILE *in, struct sym_entry *s)
 {
-	char sym[500], stype;
+	char name[500], type;
 	int rc;
 
-	rc = fscanf(in, "%llx %c %499s\n", &s->addr, &stype, sym);
+	rc = fscanf(in, "%llx %c %499s\n", &s->addr, &type, name);
 	if (rc != 3) {
-		if (rc != EOF && fgets(sym, 500, in) == NULL)
+		if (rc != EOF && fgets(name, 500, in) == NULL)
 			fprintf(stderr, "Read error or end of file.\n");
 		return -1;
 	}
-	if (strlen(sym) >= KSYM_NAME_LEN) {
+	if (strlen(name) >= KSYM_NAME_LEN) {
 		fprintf(stderr, "Symbol %s too long for kallsyms (%zu >= %d).\n"
 				"Please increase KSYM_NAME_LEN both in kernel and kallsyms.c\n",
-			sym, strlen(sym), KSYM_NAME_LEN);
+			name, strlen(name), KSYM_NAME_LEN);
 		return -1;
 	}
 
-	if (is_ignored_symbol(sym, stype))
+	if (is_ignored_symbol(name, type))
 		return -1;
 
 	/* Ignore most absolute/undefined (?) symbols. */
-	if (strcmp(sym, "_text") == 0)
+	if (strcmp(name, "_text") == 0)
 		_text = s->addr;
 
-	check_symbol_range(sym, s->addr, text_ranges, ARRAY_SIZE(text_ranges));
-	check_symbol_range(sym, s->addr, &percpu_range, 1);
+	check_symbol_range(name, s->addr, text_ranges, ARRAY_SIZE(text_ranges));
+	check_symbol_range(name, s->addr, &percpu_range, 1);
 
 	/* include the type field in the symbol name, so that it gets
 	 * compressed together */
-	s->len = strlen(sym) + 1;
+	s->len = strlen(name) + 1;
 	s->sym = malloc(s->len + 1);
 	if (!s->sym) {
 		fprintf(stderr, "kallsyms failure: "
 			"unable to allocate required amount of memory\n");
 		exit(EXIT_FAILURE);
 	}
-	strcpy(sym_name(s), sym);
-	s->sym[0] = stype;
+	strcpy(sym_name(s), name);
+	s->sym[0] = type;
 
 	s->percpu_absolute = 0;
 
-- 
2.17.1


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

* [PATCH 2/3] scripts/kallsyms: change table to store (strcut sym_entry *)
  2020-02-02  5:09 [PATCH 1/3] scripts/kallsyms: rename local variables in read_symbol() Masahiro Yamada
@ 2020-02-02  5:09 ` Masahiro Yamada
  2020-02-02  5:09 ` [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[] Masahiro Yamada
  1 sibling, 0 replies; 6+ messages in thread
From: Masahiro Yamada @ 2020-02-02  5:09 UTC (permalink / raw)
  To: linux-kbuild; +Cc: Masahiro Yamada, linux-kernel

The symbol table is extended every 10000 addition by using realloc(),
where data copy might occur to the new buffer.

To decrease the amount of possible data copy, let's change the table
to store the pointer.

The symbol type + symbol name part is appended at the end of
(struct sym_entry), and allocated together with the struct body.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kallsyms.c | 121 ++++++++++++++++++++++++---------------------
 1 file changed, 65 insertions(+), 56 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 5c34edd98b3e..a566d8201b56 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -33,8 +33,8 @@ struct sym_entry {
 	unsigned long long addr;
 	unsigned int len;
 	unsigned int start_pos;
-	unsigned char *sym;
 	unsigned int percpu_absolute;
+	unsigned char sym[0];
 };
 
 struct addr_range {
@@ -55,7 +55,7 @@ static struct addr_range percpu_range = {
 	"__per_cpu_start", "__per_cpu_end", -1ULL, 0
 };
 
-static struct sym_entry *table;
+static struct sym_entry **table;
 static unsigned int table_size, table_cnt;
 static int all_symbols;
 static int absolute_percpu;
@@ -174,49 +174,55 @@ static void check_symbol_range(const char *sym, unsigned long long addr,
 	}
 }
 
-static int read_symbol(FILE *in, struct sym_entry *s)
+static struct sym_entry *read_symbol(FILE *in)
 {
 	char name[500], type;
+	unsigned long long addr;
+	unsigned int len;
+	struct sym_entry *sym;
 	int rc;
 
-	rc = fscanf(in, "%llx %c %499s\n", &s->addr, &type, name);
+	rc = fscanf(in, "%llx %c %499s\n", &addr, &type, name);
 	if (rc != 3) {
 		if (rc != EOF && fgets(name, 500, in) == NULL)
 			fprintf(stderr, "Read error or end of file.\n");
-		return -1;
+		return NULL;
 	}
 	if (strlen(name) >= KSYM_NAME_LEN) {
 		fprintf(stderr, "Symbol %s too long for kallsyms (%zu >= %d).\n"
 				"Please increase KSYM_NAME_LEN both in kernel and kallsyms.c\n",
 			name, strlen(name), KSYM_NAME_LEN);
-		return -1;
+		return NULL;
 	}
 
 	if (is_ignored_symbol(name, type))
-		return -1;
+		return NULL;
 
 	/* Ignore most absolute/undefined (?) symbols. */
 	if (strcmp(name, "_text") == 0)
-		_text = s->addr;
+		_text = addr;
 
-	check_symbol_range(name, s->addr, text_ranges, ARRAY_SIZE(text_ranges));
-	check_symbol_range(name, s->addr, &percpu_range, 1);
+	check_symbol_range(name, addr, text_ranges, ARRAY_SIZE(text_ranges));
+	check_symbol_range(name, addr, &percpu_range, 1);
 
 	/* include the type field in the symbol name, so that it gets
 	 * compressed together */
-	s->len = strlen(name) + 1;
-	s->sym = malloc(s->len + 1);
-	if (!s->sym) {
+
+	len = strlen(name) + 1;
+
+	sym = malloc(sizeof(*sym) + len);
+	if (!sym) {
 		fprintf(stderr, "kallsyms failure: "
 			"unable to allocate required amount of memory\n");
 		exit(EXIT_FAILURE);
 	}
-	strcpy(sym_name(s), name);
-	s->sym[0] = type;
-
-	s->percpu_absolute = 0;
+	sym->addr = addr;
+	sym->len = len;
+	sym->sym[0] = type;
+	memcpy(sym_name(sym), name, len);
+	sym->percpu_absolute = 0;
 
-	return 0;
+	return sym;
 }
 
 static int symbol_in_range(const struct sym_entry *s,
@@ -268,12 +274,12 @@ static void shrink_table(void)
 
 	pos = 0;
 	for (i = 0; i < table_cnt; i++) {
-		if (symbol_valid(&table[i])) {
+		if (symbol_valid(table[i])) {
 			if (pos != i)
 				table[pos] = table[i];
 			pos++;
 		} else {
-			free(table[i].sym);
+			free(table[i]);
 		}
 	}
 	table_cnt = pos;
@@ -287,7 +293,15 @@ static void shrink_table(void)
 
 static void read_map(FILE *in)
 {
+	struct sym_entry *sym;
+
 	while (!feof(in)) {
+		sym = read_symbol(in);
+		if (!sym)
+			continue;
+
+		sym->start_pos = table_cnt;
+
 		if (table_cnt >= table_size) {
 			table_size += 10000;
 			table = realloc(table, sizeof(*table) * table_size);
@@ -296,10 +310,8 @@ static void read_map(FILE *in)
 				exit (1);
 			}
 		}
-		if (read_symbol(in, &table[table_cnt]) == 0) {
-			table[table_cnt].start_pos = table_cnt;
-			table_cnt++;
-		}
+
+		table[table_cnt++] = sym;
 	}
 }
 
@@ -387,27 +399,27 @@ static void write_src(void)
 			int overflow;
 
 			if (!absolute_percpu) {
-				offset = table[i].addr - relative_base;
+				offset = table[i]->addr - relative_base;
 				overflow = (offset < 0 || offset > UINT_MAX);
-			} else if (symbol_absolute(&table[i])) {
-				offset = table[i].addr;
+			} else if (symbol_absolute(table[i])) {
+				offset = table[i]->addr;
 				overflow = (offset < 0 || offset > INT_MAX);
 			} else {
-				offset = relative_base - table[i].addr - 1;
+				offset = relative_base - table[i]->addr - 1;
 				overflow = (offset < INT_MIN || offset >= 0);
 			}
 			if (overflow) {
 				fprintf(stderr, "kallsyms failure: "
 					"%s symbol value %#llx out of range in relative mode\n",
-					symbol_absolute(&table[i]) ? "absolute" : "relative",
-					table[i].addr);
+					symbol_absolute(table[i]) ? "absolute" : "relative",
+					table[i]->addr);
 				exit(EXIT_FAILURE);
 			}
 			printf("\t.long\t%#x\n", (int)offset);
-		} else if (!symbol_absolute(&table[i])) {
-			output_address(table[i].addr);
+		} else if (!symbol_absolute(table[i])) {
+			output_address(table[i]->addr);
 		} else {
-			printf("\tPTR\t%#llx\n", table[i].addr);
+			printf("\tPTR\t%#llx\n", table[i]->addr);
 		}
 	}
 	printf("\n");
@@ -437,12 +449,12 @@ static void write_src(void)
 		if ((i & 0xFF) == 0)
 			markers[i >> 8] = off;
 
-		printf("\t.byte 0x%02x", table[i].len);
-		for (k = 0; k < table[i].len; k++)
-			printf(", 0x%02x", table[i].sym[k]);
+		printf("\t.byte 0x%02x", table[i]->len);
+		for (k = 0; k < table[i]->len; k++)
+			printf(", 0x%02x", table[i]->sym[k]);
 		printf("\n");
 
-		off += table[i].len + 1;
+		off += table[i]->len + 1;
 	}
 	printf("\n");
 
@@ -496,7 +508,7 @@ static void build_initial_tok_table(void)
 	unsigned int i;
 
 	for (i = 0; i < table_cnt; i++)
-		learn_symbol(table[i].sym, table[i].len);
+		learn_symbol(table[i]->sym, table[i]->len);
 }
 
 static unsigned char *find_token(unsigned char *str, int len,
@@ -520,15 +532,15 @@ static void compress_symbols(const unsigned char *str, int idx)
 
 	for (i = 0; i < table_cnt; i++) {
 
-		len = table[i].len;
-		p1 = table[i].sym;
+		len = table[i]->len;
+		p1 = table[i]->sym;
 
 		/* find the token on the symbol */
 		p2 = find_token(p1, len, str);
 		if (!p2) continue;
 
 		/* decrease the counts for this symbol's tokens */
-		forget_symbol(table[i].sym, len);
+		forget_symbol(table[i]->sym, len);
 
 		size = len;
 
@@ -547,10 +559,10 @@ static void compress_symbols(const unsigned char *str, int idx)
 
 		} while (p2);
 
-		table[i].len = len;
+		table[i]->len = len;
 
 		/* increase the counts for this symbol's new tokens */
-		learn_symbol(table[i].sym, len);
+		learn_symbol(table[i]->sym, len);
 	}
 }
 
@@ -606,8 +618,8 @@ static void insert_real_symbols_in_table(void)
 	unsigned int i, j, c;
 
 	for (i = 0; i < table_cnt; i++) {
-		for (j = 0; j < table[i].len; j++) {
-			c = table[i].sym[j];
+		for (j = 0; j < table[i]->len; j++) {
+			c = table[i]->sym[j];
 			best_table[c][0]=c;
 			best_table_len[c]=1;
 		}
@@ -660,13 +672,10 @@ static int may_be_linker_script_provide_symbol(const struct sym_entry *se)
 
 static int compare_symbols(const void *a, const void *b)
 {
-	const struct sym_entry *sa;
-	const struct sym_entry *sb;
+	const struct sym_entry *sa = *(const struct sym_entry **)a;
+	const struct sym_entry *sb = *(const struct sym_entry **)b;
 	int wa, wb;
 
-	sa = a;
-	sb = b;
-
 	/* sort by address first */
 	if (sa->addr > sb->addr)
 		return 1;
@@ -697,7 +706,7 @@ static int compare_symbols(const void *a, const void *b)
 
 static void sort_symbols(void)
 {
-	qsort(table, table_cnt, sizeof(struct sym_entry), compare_symbols);
+	qsort(table, table_cnt, sizeof(table[0]), compare_symbols);
 }
 
 static void make_percpus_absolute(void)
@@ -705,14 +714,14 @@ static void make_percpus_absolute(void)
 	unsigned int i;
 
 	for (i = 0; i < table_cnt; i++)
-		if (symbol_in_range(&table[i], &percpu_range, 1)) {
+		if (symbol_in_range(table[i], &percpu_range, 1)) {
 			/*
 			 * Keep the 'A' override for percpu symbols to
 			 * ensure consistent behavior compared to older
 			 * versions of this tool.
 			 */
-			table[i].sym[0] = 'A';
-			table[i].percpu_absolute = 1;
+			table[i]->sym[0] = 'A';
+			table[i]->percpu_absolute = 1;
 		}
 }
 
@@ -722,12 +731,12 @@ static void record_relative_base(void)
 	unsigned int i;
 
 	for (i = 0; i < table_cnt; i++)
-		if (!symbol_absolute(&table[i])) {
+		if (!symbol_absolute(table[i])) {
 			/*
 			 * The table is sorted by address.
 			 * Take the first non-absolute symbol value.
 			 */
-			relative_base = table[i].addr;
+			relative_base = table[i]->addr;
 			return;
 		}
 }
-- 
2.17.1


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

* [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[]
  2020-02-02  5:09 [PATCH 1/3] scripts/kallsyms: rename local variables in read_symbol() Masahiro Yamada
  2020-02-02  5:09 ` [PATCH 2/3] scripts/kallsyms: change table to store (strcut sym_entry *) Masahiro Yamada
@ 2020-02-02  5:09 ` Masahiro Yamada
  2020-02-03  1:28   ` Masami Hiramatsu
  2020-02-03  3:26   ` Justin Capella
  1 sibling, 2 replies; 6+ messages in thread
From: Masahiro Yamada @ 2020-02-02  5:09 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Masahiro Yamada, Greg Kroah-Hartman, Marc Zyngier,
	Masami Hiramatsu, Thomas Gleixner, Will Deacon, linux-kernel

kallsyms_token_table[] only contains ASCII characters. It should be
char instead of u8.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 kernel/kallsyms.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 136ce049c4ad..53f84f685841 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -44,7 +44,7 @@ __attribute__((weak, section(".rodata")));
 extern const unsigned long kallsyms_relative_base
 __attribute__((weak, section(".rodata")));
 
-extern const u8 kallsyms_token_table[] __weak;
+extern const char kallsyms_token_table[] __weak;
 extern const u16 kallsyms_token_index[] __weak;
 
 extern const unsigned int kallsyms_markers[] __weak;
@@ -58,7 +58,8 @@ static unsigned int kallsyms_expand_symbol(unsigned int off,
 					   char *result, size_t maxlen)
 {
 	int len, skipped_first = 0;
-	const u8 *tptr, *data;
+	const char *tptr;
+	const u8 *data;
 
 	/* Get the compressed symbol length from the first symbol byte. */
 	data = &kallsyms_names[off];
-- 
2.17.1


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

* Re: [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[]
  2020-02-02  5:09 ` [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[] Masahiro Yamada
@ 2020-02-03  1:28   ` Masami Hiramatsu
  2020-02-03  3:26   ` Justin Capella
  1 sibling, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2020-02-03  1:28 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Greg Kroah-Hartman, Marc Zyngier, Masami Hiramatsu,
	Thomas Gleixner, Will Deacon, linux-kernel

On Sun,  2 Feb 2020 14:09:22 +0900
Masahiro Yamada <masahiroy@kernel.org> wrote:

> kallsyms_token_table[] only contains ASCII characters. It should be
> char instead of u8.

Indeed.

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  kernel/kallsyms.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index 136ce049c4ad..53f84f685841 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -44,7 +44,7 @@ __attribute__((weak, section(".rodata")));
>  extern const unsigned long kallsyms_relative_base
>  __attribute__((weak, section(".rodata")));
>  
> -extern const u8 kallsyms_token_table[] __weak;
> +extern const char kallsyms_token_table[] __weak;
>  extern const u16 kallsyms_token_index[] __weak;
>  
>  extern const unsigned int kallsyms_markers[] __weak;
> @@ -58,7 +58,8 @@ static unsigned int kallsyms_expand_symbol(unsigned int off,
>  					   char *result, size_t maxlen)
>  {
>  	int len, skipped_first = 0;
> -	const u8 *tptr, *data;
> +	const char *tptr;
> +	const u8 *data;
>  
>  	/* Get the compressed symbol length from the first symbol byte. */
>  	data = &kallsyms_names[off];
> -- 
> 2.17.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

* Re: [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[]
  2020-02-02  5:09 ` [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[] Masahiro Yamada
  2020-02-03  1:28   ` Masami Hiramatsu
@ 2020-02-03  3:26   ` Justin Capella
  2020-02-03  5:19     ` Masahiro Yamada
  1 sibling, 1 reply; 6+ messages in thread
From: Justin Capella @ 2020-02-03  3:26 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kbuild mailing list, Greg Kroah-Hartman, Marc Zyngier,
	Masami Hiramatsu, Thomas Gleixner, Will Deacon, linux-kernel

unsigned char maybe? I understand printable chars unlikely to cause
any trouble and probably a non issue, but the domain is different for
char, and while I don't know of supported implementations where sizeof
char isn't a byte, I think it's a possibility and perhaps why u8 was
in use?

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

* Re: [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[]
  2020-02-03  3:26   ` Justin Capella
@ 2020-02-03  5:19     ` Masahiro Yamada
  0 siblings, 0 replies; 6+ messages in thread
From: Masahiro Yamada @ 2020-02-03  5:19 UTC (permalink / raw)
  To: Justin Capella
  Cc: Linux Kbuild mailing list, Greg Kroah-Hartman, Marc Zyngier,
	Masami Hiramatsu, Thomas Gleixner, Will Deacon,
	Linux Kernel Mailing List

On Mon, Feb 3, 2020 at 12:26 PM Justin Capella <justincapella@gmail.com> wrote:
>
> unsigned char maybe? I understand printable chars unlikely to cause
> any trouble and probably a non issue, but the domain is different for
> char, and while I don't know of supported implementations where sizeof
> char isn't a byte, I think it's a possibility and perhaps why u8 was
> in use?


As I said in the commit description,
this only contains ASCII, which is 7-bit code.

We do not care the signed-ness of char.


kallsyms_names[] has been u8 since the
following old commit:


commit e10392112d315c45f054c22c862e3a7ae27d17d4
Author: Paulo Marques <pmarques@grupopie.com>
Date:   Mon Oct 18 17:59:03 2004 -0700

    [PATCH] kallsyms data size reduction / lookup speedup



Probably, this is just coding mistake.



-- 
Best Regards
Masahiro Yamada

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

end of thread, other threads:[~2020-02-03  5:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-02  5:09 [PATCH 1/3] scripts/kallsyms: rename local variables in read_symbol() Masahiro Yamada
2020-02-02  5:09 ` [PATCH 2/3] scripts/kallsyms: change table to store (strcut sym_entry *) Masahiro Yamada
2020-02-02  5:09 ` [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[] Masahiro Yamada
2020-02-03  1:28   ` Masami Hiramatsu
2020-02-03  3:26   ` Justin Capella
2020-02-03  5:19     ` Masahiro Yamada

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.