linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kallsyms, tracing: output more proper symbol name
@ 2009-03-11  9:37 Lai Jiangshan
  2009-03-11  9:46 ` Ingo Molnar
  0 siblings, 1 reply; 14+ messages in thread
From: Lai Jiangshan @ 2009-03-11  9:37 UTC (permalink / raw)
  To: Sam Ravnborg, Ingo Molnar, Andrew Morton, Steven Rostedt,
	Frederic Weisbecker, LKML


Impact: bugfix, output reliable result

Debug tools(dump_stack(), ftrace...) are like to print out symbols.
But it is always print out the first aliased symbol.(Aliased symbols
are symbols with the same address), and the first aliased symbol is
sometime not proper.

# echo function_graph > current_tracer
# cat trace
......
 1)   1.923 us    |    select_nohz_load_balancer();
 1) + 76.692 us   |  }
 1)               |  default_idle() {
 1)   ==========> |    __irqentry_text_start() {
 1)   0.000 us    |      native_apic_mem_write();
 1)               |      irq_enter() {
 1)   0.000 us    |        idle_cpu();
 1)               |        tick_check_idle() {
 1)   0.000 us    |          tick_check_oneshot_broadcast();
 1)               |          tick_nohz_stop_idle() {
......

It's very embarrassing, it ouputs "__irqentry_text_start()",
*actually, it should output "smp_apic_timer_interrupt()"*.
(these two symbol are the same address, but "__irqentry_text_start"
is deemed to the first aliased symbol by scripts/kallsyms)

This patch puts symbols like "__irqentry_text_start" to the second
aliased symbols. And a more proper symbol name becomes the first.

A table is added in scripts/kallsyms.c, and the symbols in this table
have lower priority than other symbols(which are the same address).

This table is statically defined in scripts/kallsyms.c, is not
automatically generated by a script when kernel is being built.
It's for these reasons:
	This table is(will be) updated very infrequently.
	This table is short.
	I don't want to add complexity to kernel-building

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index ad2434b..96717dd 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -500,11 +500,14 @@ static void optimize_token_table(void)
 	optimize_result();
 }
 
+static void prepare_hide_symbols(void);
+static int is_hide_symbol(const char *symbol);
+
 static int compare_symbols(const void *a, const void *b)
 {
 	const struct sym_entry *sa;
 	const struct sym_entry *sb;
-	int wa, wb;
+	int wa, wb, ha, hb;
 
 	sa = a;
 	sb = b;
@@ -521,12 +524,18 @@ static int compare_symbols(const void *a, const void *b)
 	if (wa != wb)
 		return wa - wb;
 
+	ha = is_hide_symbol((char *)sa->sym + 1);
+	hb = is_hide_symbol((char *)sb->sym + 1);
+	if (ha != hb)
+		return ha - hb;
+
 	/* sort by initial order, so that other symbols are left undisturbed */
 	return sa->start_pos - sb->start_pos;
 }
 
 static void sort_symbols(void)
 {
+	prepare_hide_symbols();
 	qsort(table, table_cnt, sizeof(struct sym_entry), compare_symbols);
 }
 
@@ -556,3 +565,125 @@ int main(int argc, char **argv)
 
 	return 0;
 }
+
+#define VMLINUX_SYMBOL(_sym_) #_sym_
+
+static const char *hide_symbols[] = {
+	/* misc symbols */
+	"_text",
+	"_stext",
+	"_etext",
+	"_sinittext",
+	"_einittext",
+
+	/* symbols from include/asm-generic/vmlinux.lds.h */
+	VMLINUX_SYMBOL(__start_mcount_loc),
+	VMLINUX_SYMBOL(__stop_mcount_loc),
+	VMLINUX_SYMBOL(__start_annotated_branch_profile),
+	VMLINUX_SYMBOL(__stop_annotated_branch_profile),
+	VMLINUX_SYMBOL(__start_branch_profile),
+	VMLINUX_SYMBOL(__stop_branch_profile),
+	VMLINUX_SYMBOL(__start___markers),
+	VMLINUX_SYMBOL(__stop___markers),
+	VMLINUX_SYMBOL(__start___tracepoints),
+	VMLINUX_SYMBOL(__stop___tracepoints),
+	VMLINUX_SYMBOL(__start_rodata),
+	VMLINUX_SYMBOL(__start_pci_fixups_early),
+	VMLINUX_SYMBOL(__end_pci_fixups_early),
+	VMLINUX_SYMBOL(__start_pci_fixups_header),
+	VMLINUX_SYMBOL(__end_pci_fixups_header),
+	VMLINUX_SYMBOL(__start_pci_fixups_final),
+	VMLINUX_SYMBOL(__end_pci_fixups_final),
+	VMLINUX_SYMBOL(__start_pci_fixups_enable),
+	VMLINUX_SYMBOL(__end_pci_fixups_enable),
+	VMLINUX_SYMBOL(__start_pci_fixups_resume),
+	VMLINUX_SYMBOL(__end_pci_fixups_resume),
+	VMLINUX_SYMBOL(__start_pci_fixups_resume_early),
+	VMLINUX_SYMBOL(__end_pci_fixups_resume_early),
+	VMLINUX_SYMBOL(__start_pci_fixups_suspend),
+	VMLINUX_SYMBOL(__end_pci_fixups_suspend),
+	VMLINUX_SYMBOL(__start_builtin_fw),
+	VMLINUX_SYMBOL(__end_builtin_fw),
+	VMLINUX_SYMBOL(__start_rio_route_ops),
+	VMLINUX_SYMBOL(__end_rio_route_ops),
+	VMLINUX_SYMBOL(__start___ksymtab),
+	VMLINUX_SYMBOL(__stop___ksymtab),
+	VMLINUX_SYMBOL(__start___ksymtab_gpl),
+	VMLINUX_SYMBOL(__stop___ksymtab_gpl),
+	VMLINUX_SYMBOL(__start___ksymtab_unused),
+	VMLINUX_SYMBOL(__stop___ksymtab_unused),
+	VMLINUX_SYMBOL(__start___ksymtab_unused_gpl),
+	VMLINUX_SYMBOL(__stop___ksymtab_unused_gpl),
+	VMLINUX_SYMBOL(__start___ksymtab_gpl_future),
+	VMLINUX_SYMBOL(__stop___ksymtab_gpl_future),
+	VMLINUX_SYMBOL(__start___kcrctab),
+	VMLINUX_SYMBOL(__stop___kcrctab),
+	VMLINUX_SYMBOL(__start___kcrctab_gpl),
+	VMLINUX_SYMBOL(__stop___kcrctab_gpl),
+	VMLINUX_SYMBOL(__start___kcrctab_unused),
+	VMLINUX_SYMBOL(__stop___kcrctab_unused),
+	VMLINUX_SYMBOL(__start___kcrctab_unused_gpl),
+	VMLINUX_SYMBOL(__stop___kcrctab_unused_gpl),
+	VMLINUX_SYMBOL(__start___kcrctab_gpl_future),
+	VMLINUX_SYMBOL(__stop___kcrctab_gpl_future),
+	VMLINUX_SYMBOL(__start___param),
+	VMLINUX_SYMBOL(__stop___param),
+	VMLINUX_SYMBOL(__end_rodata),
+	VMLINUX_SYMBOL(__security_initcall_start),
+	VMLINUX_SYMBOL(__security_initcall_end),
+	VMLINUX_SYMBOL(__sched_text_start),
+	VMLINUX_SYMBOL(__sched_text_end),
+	VMLINUX_SYMBOL(__lock_text_start),
+	VMLINUX_SYMBOL(__lock_text_end),
+	VMLINUX_SYMBOL(__kprobes_text_start),
+	VMLINUX_SYMBOL(__kprobes_text_end),
+	VMLINUX_SYMBOL(__irqentry_text_start),
+	VMLINUX_SYMBOL(__irqentry_text_end),
+	VMLINUX_SYMBOL(__start___verbose_strings),
+	VMLINUX_SYMBOL(__stop___verbose_strings),
+	VMLINUX_SYMBOL(__start___verbose),
+	VMLINUX_SYMBOL(__stop___verbose),
+	VMLINUX_SYMBOL(__start___bug_table),
+	VMLINUX_SYMBOL(__stop___bug_table),
+	VMLINUX_SYMBOL(__tracedata_start),
+	VMLINUX_SYMBOL(__tracedata_end),
+	VMLINUX_SYMBOL(__start_notes),
+	VMLINUX_SYMBOL(__stop_notes),
+	VMLINUX_SYMBOL(__early_initcall_end),
+	VMLINUX_SYMBOL(__per_cpu_start),
+	VMLINUX_SYMBOL(__per_cpu_end)
+};
+
+
+static int cmp_str(const void *p1, const void *p2)
+{
+	const char *str1 = *(const char * const *)p1;
+	const char *str2 = *(const char * const *)p2;
+
+	return strcmp(str1, str2);
+}
+
+static void prepare_hide_symbols(void)
+{
+	qsort(hide_symbols, sizeof(hide_symbols) / sizeof(hide_symbols[0]),
+			sizeof(hide_symbols[0]), cmp_str);
+}
+
+static int is_hide_symbol(const char *symbol)
+{
+	int low = 0;
+	int high = sizeof(hide_symbols) / sizeof(hide_symbols[0]);
+
+	while (high - low > 0) {
+		int mid = low + (high - low) / 2;
+		int eq = strcmp(hide_symbols[mid], symbol);
+		if (eq == 0)
+			return 1;
+		else if (eq < 0)
+			low = mid + 1;
+		else
+			high = mid;
+	}
+	return 0;
+}
+





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

* Re: [PATCH] kallsyms, tracing: output more proper symbol name
  2009-03-11  9:37 [PATCH] kallsyms, tracing: output more proper symbol name Lai Jiangshan
@ 2009-03-11  9:46 ` Ingo Molnar
  2009-03-11 10:29   ` Frederic Weisbecker
  2009-03-11 13:05   ` Paulo Marques
  0 siblings, 2 replies; 14+ messages in thread
From: Ingo Molnar @ 2009-03-11  9:46 UTC (permalink / raw)
  To: Lai Jiangshan
  Cc: Sam Ravnborg, Andrew Morton, Steven Rostedt, Frederic Weisbecker, LKML


* Lai Jiangshan <laijs@cn.fujitsu.com> wrote:

> +#define VMLINUX_SYMBOL(_sym_) #_sym_
> +
> +static const char *hide_symbols[] = {
> +	/* misc symbols */
> +	"_text",
> +	"_stext",
> +	"_etext",
> +	"_sinittext",
> +	"_einittext",
> +
> +	/* symbols from include/asm-generic/vmlinux.lds.h */
> +	VMLINUX_SYMBOL(__start_mcount_loc),
> +	VMLINUX_SYMBOL(__stop_mcount_loc),
> +	VMLINUX_SYMBOL(__start_annotated_branch_profile),
> +	VMLINUX_SYMBOL(__stop_annotated_branch_profile),
> +	VMLINUX_SYMBOL(__start_branch_profile),
> +	VMLINUX_SYMBOL(__stop_branch_profile),
> +	VMLINUX_SYMBOL(__start___markers),
> +	VMLINUX_SYMBOL(__stop___markers),
> +	VMLINUX_SYMBOL(__start___tracepoints),
> +	VMLINUX_SYMBOL(__stop___tracepoints),
> +	VMLINUX_SYMBOL(__start_rodata),
> +	VMLINUX_SYMBOL(__start_pci_fixups_early),
> +	VMLINUX_SYMBOL(__end_pci_fixups_early),
> +	VMLINUX_SYMBOL(__start_pci_fixups_header),
> +	VMLINUX_SYMBOL(__end_pci_fixups_header),
> +	VMLINUX_SYMBOL(__start_pci_fixups_final),
> +	VMLINUX_SYMBOL(__end_pci_fixups_final),
> +	VMLINUX_SYMBOL(__start_pci_fixups_enable),
> +	VMLINUX_SYMBOL(__end_pci_fixups_enable),
> +	VMLINUX_SYMBOL(__start_pci_fixups_resume),
> +	VMLINUX_SYMBOL(__end_pci_fixups_resume),
> +	VMLINUX_SYMBOL(__start_pci_fixups_resume_early),
> +	VMLINUX_SYMBOL(__end_pci_fixups_resume_early),
> +	VMLINUX_SYMBOL(__start_pci_fixups_suspend),
> +	VMLINUX_SYMBOL(__end_pci_fixups_suspend),
> +	VMLINUX_SYMBOL(__start_builtin_fw),
> +	VMLINUX_SYMBOL(__end_builtin_fw),
> +	VMLINUX_SYMBOL(__start_rio_route_ops),
> +	VMLINUX_SYMBOL(__end_rio_route_ops),
> +	VMLINUX_SYMBOL(__start___ksymtab),
> +	VMLINUX_SYMBOL(__stop___ksymtab),
> +	VMLINUX_SYMBOL(__start___ksymtab_gpl),
> +	VMLINUX_SYMBOL(__stop___ksymtab_gpl),
> +	VMLINUX_SYMBOL(__start___ksymtab_unused),
> +	VMLINUX_SYMBOL(__stop___ksymtab_unused),
> +	VMLINUX_SYMBOL(__start___ksymtab_unused_gpl),
> +	VMLINUX_SYMBOL(__stop___ksymtab_unused_gpl),
> +	VMLINUX_SYMBOL(__start___ksymtab_gpl_future),
> +	VMLINUX_SYMBOL(__stop___ksymtab_gpl_future),
> +	VMLINUX_SYMBOL(__start___kcrctab),
> +	VMLINUX_SYMBOL(__stop___kcrctab),
> +	VMLINUX_SYMBOL(__start___kcrctab_gpl),
> +	VMLINUX_SYMBOL(__stop___kcrctab_gpl),
> +	VMLINUX_SYMBOL(__start___kcrctab_unused),
> +	VMLINUX_SYMBOL(__stop___kcrctab_unused),
> +	VMLINUX_SYMBOL(__start___kcrctab_unused_gpl),
> +	VMLINUX_SYMBOL(__stop___kcrctab_unused_gpl),
> +	VMLINUX_SYMBOL(__start___kcrctab_gpl_future),
> +	VMLINUX_SYMBOL(__stop___kcrctab_gpl_future),
> +	VMLINUX_SYMBOL(__start___param),
> +	VMLINUX_SYMBOL(__stop___param),
> +	VMLINUX_SYMBOL(__end_rodata),
> +	VMLINUX_SYMBOL(__security_initcall_start),
> +	VMLINUX_SYMBOL(__security_initcall_end),
> +	VMLINUX_SYMBOL(__sched_text_start),
> +	VMLINUX_SYMBOL(__sched_text_end),
> +	VMLINUX_SYMBOL(__lock_text_start),
> +	VMLINUX_SYMBOL(__lock_text_end),
> +	VMLINUX_SYMBOL(__kprobes_text_start),
> +	VMLINUX_SYMBOL(__kprobes_text_end),
> +	VMLINUX_SYMBOL(__irqentry_text_start),
> +	VMLINUX_SYMBOL(__irqentry_text_end),
> +	VMLINUX_SYMBOL(__start___verbose_strings),
> +	VMLINUX_SYMBOL(__stop___verbose_strings),
> +	VMLINUX_SYMBOL(__start___verbose),
> +	VMLINUX_SYMBOL(__stop___verbose),
> +	VMLINUX_SYMBOL(__start___bug_table),
> +	VMLINUX_SYMBOL(__stop___bug_table),
> +	VMLINUX_SYMBOL(__tracedata_start),
> +	VMLINUX_SYMBOL(__tracedata_end),
> +	VMLINUX_SYMBOL(__start_notes),
> +	VMLINUX_SYMBOL(__stop_notes),
> +	VMLINUX_SYMBOL(__early_initcall_end),
> +	VMLINUX_SYMBOL(__per_cpu_start),
> +	VMLINUX_SYMBOL(__per_cpu_end)
> +};

I like how you try to solve this at symbol table generation 
time.

Instead of this hardcoded table, couldnt we use some more 
flexible and more future-proof method? Such as ordering 
same-address symbols by underscores:

  [same address]

  non-underscore symbols first        XYZ
  single-undescroe symbols second     _XYZ
  double-underscore symbols third     __XYZ

that way the scheme would be more or less self-maintaining as an 
underscore already carries a "this is a special, internal 
symbol" notion.

	Ingo

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

* Re: [PATCH] kallsyms, tracing: output more proper symbol name
  2009-03-11  9:46 ` Ingo Molnar
@ 2009-03-11 10:29   ` Frederic Weisbecker
  2009-03-11 13:05   ` Paulo Marques
  1 sibling, 0 replies; 14+ messages in thread
From: Frederic Weisbecker @ 2009-03-11 10:29 UTC (permalink / raw)
  To: Ingo Molnar, Lai Jiangshan
  Cc: Sam Ravnborg, Andrew Morton, Steven Rostedt, LKML

On Wed, Mar 11, 2009 at 10:46:21AM +0100, Ingo Molnar wrote:
> 
> * Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
> 
> > +#define VMLINUX_SYMBOL(_sym_) #_sym_
> > +
> > +static const char *hide_symbols[] = {
> > +	/* misc symbols */
> > +	"_text",
> > +	"_stext",
> > +	"_etext",
> > +	"_sinittext",
> > +	"_einittext",
> > +
> > +	/* symbols from include/asm-generic/vmlinux.lds.h */
> > +	VMLINUX_SYMBOL(__start_mcount_loc),
> > +	VMLINUX_SYMBOL(__stop_mcount_loc),
> > +	VMLINUX_SYMBOL(__start_annotated_branch_profile),
> > +	VMLINUX_SYMBOL(__stop_annotated_branch_profile),
> > +	VMLINUX_SYMBOL(__start_branch_profile),
> > +	VMLINUX_SYMBOL(__stop_branch_profile),
> > +	VMLINUX_SYMBOL(__start___markers),
> > +	VMLINUX_SYMBOL(__stop___markers),
> > +	VMLINUX_SYMBOL(__start___tracepoints),
> > +	VMLINUX_SYMBOL(__stop___tracepoints),
> > +	VMLINUX_SYMBOL(__start_rodata),
> > +	VMLINUX_SYMBOL(__start_pci_fixups_early),
> > +	VMLINUX_SYMBOL(__end_pci_fixups_early),
> > +	VMLINUX_SYMBOL(__start_pci_fixups_header),
> > +	VMLINUX_SYMBOL(__end_pci_fixups_header),
> > +	VMLINUX_SYMBOL(__start_pci_fixups_final),
> > +	VMLINUX_SYMBOL(__end_pci_fixups_final),
> > +	VMLINUX_SYMBOL(__start_pci_fixups_enable),
> > +	VMLINUX_SYMBOL(__end_pci_fixups_enable),
> > +	VMLINUX_SYMBOL(__start_pci_fixups_resume),
> > +	VMLINUX_SYMBOL(__end_pci_fixups_resume),
> > +	VMLINUX_SYMBOL(__start_pci_fixups_resume_early),
> > +	VMLINUX_SYMBOL(__end_pci_fixups_resume_early),
> > +	VMLINUX_SYMBOL(__start_pci_fixups_suspend),
> > +	VMLINUX_SYMBOL(__end_pci_fixups_suspend),
> > +	VMLINUX_SYMBOL(__start_builtin_fw),
> > +	VMLINUX_SYMBOL(__end_builtin_fw),
> > +	VMLINUX_SYMBOL(__start_rio_route_ops),
> > +	VMLINUX_SYMBOL(__end_rio_route_ops),
> > +	VMLINUX_SYMBOL(__start___ksymtab),
> > +	VMLINUX_SYMBOL(__stop___ksymtab),
> > +	VMLINUX_SYMBOL(__start___ksymtab_gpl),
> > +	VMLINUX_SYMBOL(__stop___ksymtab_gpl),
> > +	VMLINUX_SYMBOL(__start___ksymtab_unused),
> > +	VMLINUX_SYMBOL(__stop___ksymtab_unused),
> > +	VMLINUX_SYMBOL(__start___ksymtab_unused_gpl),
> > +	VMLINUX_SYMBOL(__stop___ksymtab_unused_gpl),
> > +	VMLINUX_SYMBOL(__start___ksymtab_gpl_future),
> > +	VMLINUX_SYMBOL(__stop___ksymtab_gpl_future),
> > +	VMLINUX_SYMBOL(__start___kcrctab),
> > +	VMLINUX_SYMBOL(__stop___kcrctab),
> > +	VMLINUX_SYMBOL(__start___kcrctab_gpl),
> > +	VMLINUX_SYMBOL(__stop___kcrctab_gpl),
> > +	VMLINUX_SYMBOL(__start___kcrctab_unused),
> > +	VMLINUX_SYMBOL(__stop___kcrctab_unused),
> > +	VMLINUX_SYMBOL(__start___kcrctab_unused_gpl),
> > +	VMLINUX_SYMBOL(__stop___kcrctab_unused_gpl),
> > +	VMLINUX_SYMBOL(__start___kcrctab_gpl_future),
> > +	VMLINUX_SYMBOL(__stop___kcrctab_gpl_future),
> > +	VMLINUX_SYMBOL(__start___param),
> > +	VMLINUX_SYMBOL(__stop___param),
> > +	VMLINUX_SYMBOL(__end_rodata),
> > +	VMLINUX_SYMBOL(__security_initcall_start),
> > +	VMLINUX_SYMBOL(__security_initcall_end),
> > +	VMLINUX_SYMBOL(__sched_text_start),
> > +	VMLINUX_SYMBOL(__sched_text_end),
> > +	VMLINUX_SYMBOL(__lock_text_start),
> > +	VMLINUX_SYMBOL(__lock_text_end),
> > +	VMLINUX_SYMBOL(__kprobes_text_start),
> > +	VMLINUX_SYMBOL(__kprobes_text_end),
> > +	VMLINUX_SYMBOL(__irqentry_text_start),
> > +	VMLINUX_SYMBOL(__irqentry_text_end),
> > +	VMLINUX_SYMBOL(__start___verbose_strings),
> > +	VMLINUX_SYMBOL(__stop___verbose_strings),
> > +	VMLINUX_SYMBOL(__start___verbose),
> > +	VMLINUX_SYMBOL(__stop___verbose),
> > +	VMLINUX_SYMBOL(__start___bug_table),
> > +	VMLINUX_SYMBOL(__stop___bug_table),
> > +	VMLINUX_SYMBOL(__tracedata_start),
> > +	VMLINUX_SYMBOL(__tracedata_end),
> > +	VMLINUX_SYMBOL(__start_notes),
> > +	VMLINUX_SYMBOL(__stop_notes),
> > +	VMLINUX_SYMBOL(__early_initcall_end),
> > +	VMLINUX_SYMBOL(__per_cpu_start),
> > +	VMLINUX_SYMBOL(__per_cpu_end)
> > +};
> 
> I like how you try to solve this at symbol table generation 
> time.
> 
> Instead of this hardcoded table, couldnt we use some more 
> flexible and more future-proof method? Such as ordering 
> same-address symbols by underscores:
> 
>   [same address]
> 
>   non-underscore symbols first        XYZ
>   single-undescroe symbols second     _XYZ
>   double-underscore symbols third     __XYZ
> 
> that way the scheme would be more or less self-maintaining as an 
> underscore already carries a "this is a special, internal 
> symbol" notion.
> 
> 	Ingo

Thanks Lai!
I didn't know how to solve properly this problem.
But indeed, the underscore priority sorted symbols seems to me a good
and scalable idea.


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

* Re: [PATCH] kallsyms, tracing: output more proper symbol name
  2009-03-11  9:46 ` Ingo Molnar
  2009-03-11 10:29   ` Frederic Weisbecker
@ 2009-03-11 13:05   ` Paulo Marques
  2009-03-12  2:43     ` Lai Jiangshan
  1 sibling, 1 reply; 14+ messages in thread
From: Paulo Marques @ 2009-03-11 13:05 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Lai Jiangshan, Sam Ravnborg, Andrew Morton, Steven Rostedt,
	Frederic Weisbecker, LKML

Ingo Molnar wrote:
> * Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
> 
>> +#define VMLINUX_SYMBOL(_sym_) #_sym_
>> +
>> +static const char *hide_symbols[] = {
>> +	/* misc symbols */
>> +	"_text",
>> +	"_stext",
>> +	"_etext",
>> +	"_sinittext",
>> +	"_einittext",
>> +
>> +	/* symbols from include/asm-generic/vmlinux.lds.h */
>> +	VMLINUX_SYMBOL(__start_mcount_loc),
>> +	VMLINUX_SYMBOL(__stop_mcount_loc),
[...]
>> +	VMLINUX_SYMBOL(__early_initcall_end),
>> +	VMLINUX_SYMBOL(__per_cpu_start),
>> +	VMLINUX_SYMBOL(__per_cpu_end)
>> +};
> 
> I like how you try to solve this at symbol table generation 
> time.

Yes, this is the proper way to do it.

> Instead of this hardcoded table, couldnt we use some more 
> flexible and more future-proof method? Such as ordering 
> same-address symbols by underscores:
> 
>   [same address]
> 
>   non-underscore symbols first        XYZ
>   single-undescroe symbols second     _XYZ
>   double-underscore symbols third     __XYZ
> 
> that way the scheme would be more or less self-maintaining as an 
> underscore already carries a "this is a special, internal 
> symbol" notion.

I agree with this approach, but to be on the side we should skim through
the alias and see if this works for most symbols or not.

I just did this for the symbols on my running kernel and it seems to
work. The hierarchy for '_' and '__' is in fact necessary:

> ffffffff80447418 T __sched_text_start
> ffffffff80447418 t sleep_on_common
> ffffffff80449830 T __lock_text_start
> ffffffff80449830 T __sched_text_end
> ffffffff80449830 T _spin_trylock
> ffffffff80453d50 R __stop___ex_table
> ffffffff80453d50 T __start_notes
> ffffffff8045b000 R __start_rodata
> ffffffff8045b000 R linux_banner

or that "_spin_trylock" might be displayed incorrectly as
"__sched_text_end" or something like that.

-- 
Paulo Marques - www.grupopie.com

"Who is general Failure and why is he reading my disk?"

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

* Re: [PATCH] kallsyms, tracing: output more proper symbol name
  2009-03-11 13:05   ` Paulo Marques
@ 2009-03-12  2:43     ` Lai Jiangshan
  2009-03-12 15:32       ` Paulo Marques
  0 siblings, 1 reply; 14+ messages in thread
From: Lai Jiangshan @ 2009-03-12  2:43 UTC (permalink / raw)
  To: Paulo Marques, Ingo Molnar, Sam Ravnborg, Andrew Morton,
	Steven Rostedt, Frederic Weisbecker
  Cc: LKML

Paulo Marques wrote:
> Ingo Molnar wrote:
>> I like how you try to solve this at symbol table generation 
>> time.
> 
> Yes, this is the proper way to do it.
> 
>> Instead of this hardcoded table, couldnt we use some more 
>> flexible and more future-proof method? Such as ordering 
>> same-address symbols by underscores:
>>
>>   [same address]
>>
>>   non-underscore symbols first        XYZ
>>   single-undescroe symbols second     _XYZ
>>   double-underscore symbols third     __XYZ
>>
>> that way the scheme would be more or less self-maintaining as an 
>> underscore already carries a "this is a special, internal 
>> symbol" notion.
> 
> I agree with this approach, but to be on the side we should skim through
> the alias and see if this works for most symbols or not.
> 
> I just did this for the symbols on my running kernel and it seems to
> work. The hierarchy for '_' and '__' is in fact necessary:
> 
>> ffffffff80447418 T __sched_text_start
>> ffffffff80447418 t sleep_on_common
>> ffffffff80449830 T __lock_text_start
>> ffffffff80449830 T __sched_text_end
>> ffffffff80449830 T _spin_trylock
>> ffffffff80453d50 R __stop___ex_table
>> ffffffff80453d50 T __start_notes
>> ffffffff8045b000 R __start_rodata
>> ffffffff8045b000 R linux_banner
> 
> or that "_spin_trylock" might be displayed incorrectly as
> "__sched_text_end" or something like that.
> 

sort by underscore may be incorrect. A lots of symbol in .c
file is '_' or '__' prefix, human are happy to see these symbol.

Symbols in my table(in V1 patch) are linker-script-provide symbols.
Aliased symbols mostly come from linker script. Now, v2 patch's ordering:

	[same address]
	symbol defined in .c or .S
	symbol defined in linker script

From: Lai Jiangshan <laijs@cn.fujitsu.com>

Impact: bugfix, output reliable result

Debug tools(dump_stack(), ftrace...) are like to print out symbols.
But it is always print out the first aliased symbol.(Aliased symbols
are symbols with the same address), and the first aliased symbol is
sometime not proper.

# echo function_graph > current_tracer
# cat trace
......
 1)   1.923 us    |    select_nohz_load_balancer();
 1) + 76.692 us   |  }
 1)               |  default_idle() {
 1)   ==========> |    __irqentry_text_start() {
 1)   0.000 us    |      native_apic_mem_write();
 1)               |      irq_enter() {
 1)   0.000 us    |        idle_cpu();
 1)               |        tick_check_idle() {
 1)   0.000 us    |          tick_check_oneshot_broadcast();
 1)               |          tick_nohz_stop_idle() {
......

It's very embarrassing, it ouputs "__irqentry_text_start()",
actually, it should output "smp_apic_timer_interrupt()".
(these two symbol are the same address, but "__irqentry_text_start"
is deemed to the first aliased symbol by scripts/kallsyms)

This patch puts symbols like "__irqentry_text_start" to the second
aliased symbols. And a more proper symbol name becomes the first.

Aliased symbols mostly come from linker script. The solution is
guessing "is this symbol defined in linker script", the symbols
defined in linker script will not become the first aliased symbol.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index ad2434b..c6924d4 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -500,11 +500,46 @@ static void optimize_token_table(void)
 	optimize_result();
 }
 
+/* guess for "linker script provide" symbol */
+static int may_be_linker_script_provide_symbol(const struct sym_entry *se)
+{
+	const char *symbol = (char *)se->sym + 1;
+	int len = se->len - 1;
+
+	if (len < 8)
+		return 0;
+
+	if (symbol[0] != '_' || symbol[1] != '_')
+		return 0;
+
+	/* __start_XXXXX */
+	if (!memcmp(symbol + 2, "start_", 6))
+		return 1;
+
+	/* __stop_XXXXX */
+	if (!memcmp(symbol + 2, "stop_", 5))
+		return 1;
+
+	/* __end_XXXXX */
+	if (!memcmp(symbol + 2, "end_", 4))
+		return 1;
+
+	/* __XXXXX_start */
+	if (!memcmp(symbol + len - 6, "_start", 6))
+		return 1;
+
+	/* __XXXXX_end */
+	if (!memcmp(symbol + len - 4, "_end", 4))
+		return 1;
+
+	return 0;
+}
+
 static int compare_symbols(const void *a, const void *b)
 {
 	const struct sym_entry *sa;
 	const struct sym_entry *sb;
-	int wa, wb;
+	int wa, wb, la, lb;
 
 	sa = a;
 	sb = b;
@@ -521,6 +556,12 @@ static int compare_symbols(const void *a, const void *b)
 	if (wa != wb)
 		return wa - wb;
 
+	/* sort by "linker script provide" type */
+	la = may_be_linker_script_provide_symbol(sa);
+	lb = may_be_linker_script_provide_symbol(sb);
+	if (la != lb)
+		return la - lb;
+
 	/* sort by initial order, so that other symbols are left undisturbed */
 	return sa->start_pos - sb->start_pos;
 }


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

* Re: [PATCH] kallsyms, tracing: output more proper symbol name
  2009-03-12  2:43     ` Lai Jiangshan
@ 2009-03-12 15:32       ` Paulo Marques
  2009-03-13  7:10         ` [PATCH V3] " Lai Jiangshan
  0 siblings, 1 reply; 14+ messages in thread
From: Paulo Marques @ 2009-03-12 15:32 UTC (permalink / raw)
  To: Lai Jiangshan
  Cc: Ingo Molnar, Sam Ravnborg, Andrew Morton, Steven Rostedt,
	Frederic Weisbecker, LKML

Lai Jiangshan wrote:
> Paulo Marques wrote:
>> Ingo Molnar wrote:
>>> I like how you try to solve this at symbol table generation 
>>> time.
>> Yes, this is the proper way to do it.
>>
>>> Instead of this hardcoded table, couldnt we use some more 
>>> flexible and more future-proof method? Such as ordering 
>>> same-address symbols by underscores:
>>>
>>>   [same address]
>>>
>>>   non-underscore symbols first        XYZ
>>>   single-undescroe symbols second     _XYZ
>>>   double-underscore symbols third     __XYZ
>>>
>>> that way the scheme would be more or less self-maintaining as an 
>>> underscore already carries a "this is a special, internal 
>>> symbol" notion.
>> I agree with this approach, but to be on the side we should skim through
>> the alias and see if this works for most symbols or not.
>>
>> I just did this for the symbols on my running kernel and it seems to
>> work. The hierarchy for '_' and '__' is in fact necessary:
>>
>>> ffffffff80447418 T __sched_text_start
>>> ffffffff80447418 t sleep_on_common
>>> ffffffff80449830 T __lock_text_start
>>> ffffffff80449830 T __sched_text_end
>>> ffffffff80449830 T _spin_trylock
>>> ffffffff80453d50 R __stop___ex_table
>>> ffffffff80453d50 T __start_notes
>>> ffffffff8045b000 R __start_rodata
>>> ffffffff8045b000 R linux_banner
>> or that "_spin_trylock" might be displayed incorrectly as
>> "__sched_text_end" or something like that.
>>
> 
> sort by underscore may be incorrect. A lots of symbol in .c
> file is '_' or '__' prefix, human are happy to see these symbol.

Since this is just a secondary criteria that applies only to _aliased_
symbols, this shouldn't be a big problem. Symbols with different
addresses are properly placed in the correct order.

> Symbols in my table(in V1 patch) are linker-script-provide symbols.
> Aliased symbols mostly come from linker script. Now, v2 patch's ordering:
> 
> 	[same address]
> 	symbol defined in .c or .S
> 	symbol defined in linker script
> 
> From: Lai Jiangshan <laijs@cn.fujitsu.com>
> 
[...]
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index ad2434b..c6924d4 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -500,11 +500,46 @@ static void optimize_token_table(void)
>  	optimize_result();
>  }
>  
> +/* guess for "linker script provide" symbol */
> +static int may_be_linker_script_provide_symbol(const struct sym_entry *se)
> +{
> +	const char *symbol = (char *)se->sym + 1;
> +	int len = se->len - 1;
> +
> +	if (len < 8)
> +		return 0;
> +
> +	if (symbol[0] != '_' || symbol[1] != '_')
> +		return 0;
> +
> +	/* __start_XXXXX */
> +	if (!memcmp(symbol + 2, "start_", 6))
> +		return 1;
> +
> +	/* __stop_XXXXX */
> +	if (!memcmp(symbol + 2, "stop_", 5))
> +		return 1;
> +
> +	/* __end_XXXXX */
> +	if (!memcmp(symbol + 2, "end_", 4))
> +		return 1;
> +
> +	/* __XXXXX_start */
> +	if (!memcmp(symbol + len - 6, "_start", 6))
> +		return 1;
> +
> +	/* __XXXXX_end */
> +	if (!memcmp(symbol + len - 4, "_end", 4))
> +		return 1;
> +
> +	return 0;
> +}
> +
>  static int compare_symbols(const void *a, const void *b)
>  {
>  	const struct sym_entry *sa;
>  	const struct sym_entry *sb;
> -	int wa, wb;
> +	int wa, wb, la, lb;
>  
>  	sa = a;
>  	sb = b;
> @@ -521,6 +556,12 @@ static int compare_symbols(const void *a, const void *b)
>  	if (wa != wb)
>  		return wa - wb;
>  
> +	/* sort by "linker script provide" type */
> +	la = may_be_linker_script_provide_symbol(sa);
> +	lb = may_be_linker_script_provide_symbol(sb);
> +	if (la != lb)
> +		return la - lb;
> +
>  	/* sort by initial order, so that other symbols are left undisturbed */
>  	return sa->start_pos - sb->start_pos;
>  }

I would probably still place another test to compare the number of
underscores, after both symbols are found to be equal in the "linker
script provided" criteria.

The way it is now, for my current kallsyms table:

> ffffffff80200000 A _text
> ffffffff80200000 T startup_64
> ffffffff80209000 T _stext
> ffffffff80209000 t init_post

a "startup_64" address would display as "_text" and a "init_post"
address would display as "_stext".

You can add all the stext/etext symbols as special cases to the
"may_be_linker_script_provide_symbol" function (like it was on your v1
patch), but I'm just afraid that we'll find more cases in the future
that are not automatically caught by these rules...

-- 
Paulo Marques - www.grupopie.com

"All generalizations are false."

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

* [PATCH V3] kallsyms, tracing: output more proper symbol name
  2009-03-12 15:32       ` Paulo Marques
@ 2009-03-13  7:10         ` Lai Jiangshan
  2009-03-13  9:27           ` Lai Jiangshan
  2009-03-14  8:57           ` [tip:tracing/ftrace] " Lai Jiangshan
  0 siblings, 2 replies; 14+ messages in thread
From: Lai Jiangshan @ 2009-03-13  7:10 UTC (permalink / raw)
  To: Paulo Marques
  Cc: Ingo Molnar, Sam Ravnborg, Andrew Morton, Steven Rostedt,
	Frederic Weisbecker, LKML

Paulo Marques wrote:
> I would probably still place another test to compare the number of
> underscores, after both symbols are found to be equal in the "linker
> script provided" criteria.
> 
> The way it is now, for my current kallsyms table:
> 
>> ffffffff80200000 A _text
>> ffffffff80200000 T startup_64
>> ffffffff80209000 T _stext
>> ffffffff80209000 t init_post
> 
> a "startup_64" address would display as "_text" and a "init_post"
> address would display as "_stext".
> 
> You can add all the stext/etext symbols as special cases to the
> "may_be_linker_script_provide_symbol" function (like it was on your v1
> patch), but I'm just afraid that we'll find more cases in the future
> that are not automatically caught by these rules...
> 

Subject:  kallsyms, tracing: output more proper symbol name

Impact: bugfix, output reliable result

Debug tools(dump_stack(), ftrace...) are like to print out symbols.
But it is always print out the first aliased symbol.(Aliased symbols
are symbols with the same address), and the first aliased symbol is
sometime not proper.

# echo function_graph > current_tracer
# cat trace
......
 1)   1.923 us    |    select_nohz_load_balancer();
 1) + 76.692 us   |  }
 1)               |  default_idle() {
 1)   ==========> |    __irqentry_text_start() {
 1)   0.000 us    |      native_apic_mem_write();
 1)               |      irq_enter() {
 1)   0.000 us    |        idle_cpu();
 1)               |        tick_check_idle() {
 1)   0.000 us    |          tick_check_oneshot_broadcast();
 1)               |          tick_nohz_stop_idle() {
......

It's very embarrassing, it ouputs "__irqentry_text_start()",
actually, it should output "smp_apic_timer_interrupt()".
(these two symbol are the same address, but "__irqentry_text_start"
is deemed to the first aliased symbol by scripts/kallsyms)

This patch puts symbols like "__irqentry_text_start" to the second
aliased symbols. And a more proper symbol name becomes the first.

Aliased symbols mostly come from linker script. The solution is
guessing "is this symbol defined in linker script", the symbols
defined in linker script will not become the first aliased symbol.

And if symbols are found to be equal in this "linker script provided"
criteria, symbols are sorted by the number of prefix underscores.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index ad2434b..6654cbe 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -500,6 +500,51 @@ static void optimize_token_table(void)
 	optimize_result();
 }
 
+/* guess for "linker script provide" symbol */
+static int may_be_linker_script_provide_symbol(const struct sym_entry *se)
+{
+	const char *symbol = (char *)se->sym + 1;
+	int len = se->len - 1;
+
+	if (len < 8)
+		return 0;
+
+	if (symbol[0] != '_' || symbol[1] != '_')
+		return 0;
+
+	/* __start_XXXXX */
+	if (!memcmp(symbol + 2, "start_", 6))
+		return 1;
+
+	/* __stop_XXXXX */
+	if (!memcmp(symbol + 2, "stop_", 5))
+		return 1;
+
+	/* __end_XXXXX */
+	if (!memcmp(symbol + 2, "end_", 4))
+		return 1;
+
+	/* __XXXXX_start */
+	if (!memcmp(symbol + len - 6, "_start", 6))
+		return 1;
+
+	/* __XXXXX_end */
+	if (!memcmp(symbol + len - 4, "_end", 4))
+		return 1;
+
+	return 0;
+}
+
+static int prefix_underscores_count(const char *str)
+{
+	const char *tail = str;
+
+	while (*tail != '_')
+		tail++;
+
+	return tail - str;
+}
+
 static int compare_symbols(const void *a, const void *b)
 {
 	const struct sym_entry *sa;
@@ -521,6 +566,18 @@ static int compare_symbols(const void *a, const void *b)
 	if (wa != wb)
 		return wa - wb;
 
+	/* sort by "linker script provide" type */
+	wa = may_be_linker_script_provide_symbol(sa);
+	wb = may_be_linker_script_provide_symbol(sb);
+	if (wa != wb)
+		return wa - wb;
+
+	/* sort by the number of prefix underscores */
+	wa = prefix_underscores_count((const char *)sa->sym + 1);
+	wb = prefix_underscores_count((const char *)sb->sym + 1);
+	if (wa != wb)
+		return wa - wb;
+
 	/* sort by initial order, so that other symbols are left undisturbed */
 	return sa->start_pos - sb->start_pos;
 }

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

* Re: [PATCH V3] kallsyms, tracing: output more proper symbol name
  2009-03-13  7:10         ` [PATCH V3] " Lai Jiangshan
@ 2009-03-13  9:27           ` Lai Jiangshan
  2009-03-13 19:25             ` Paulo Marques
  2009-03-13 19:40             ` Sam Ravnborg
  2009-03-14  8:57           ` [tip:tracing/ftrace] " Lai Jiangshan
  1 sibling, 2 replies; 14+ messages in thread
From: Lai Jiangshan @ 2009-03-13  9:27 UTC (permalink / raw)
  To: Lai Jiangshan
  Cc: Paulo Marques, Ingo Molnar, Sam Ravnborg, Andrew Morton,
	Steven Rostedt, Frederic Weisbecker, LKML

Lai Jiangshan wrote:
> Paulo Marques wrote:
>> I would probably still place another test to compare the number of
>> underscores, after both symbols are found to be equal in the "linker
>> script provided" criteria.
>>
>> The way it is now, for my current kallsyms table:
>>
>>> ffffffff80200000 A _text
>>> ffffffff80200000 T startup_64
>>> ffffffff80209000 T _stext
>>> ffffffff80209000 t init_post
>> a "startup_64" address would display as "_text" and a "init_post"
>> address would display as "_stext".
>>
>> You can add all the stext/etext symbols as special cases to the
>> "may_be_linker_script_provide_symbol" function (like it was on your v1
>> patch), but I'm just afraid that we'll find more cases in the future
>> that are not automatically caught by these rules...
>>
> 

[resend, previous v3 has some mistakes]

Subject:  kallsyms, tracing: output more proper symbol name

Impact: bugfix, output reliable result

Debug tools(dump_stack(), ftrace...) are like to print out symbols.
But it is always print out the first aliased symbol.(Aliased symbols
are symbols with the same address), and the first aliased symbol is
sometime not proper.

# echo function_graph > current_tracer
# cat trace
......
 1)   1.923 us    |    select_nohz_load_balancer();
 1) + 76.692 us   |  }
 1)               |  default_idle() {
 1)   ==========> |    __irqentry_text_start() {
 1)   0.000 us    |      native_apic_mem_write();
 1)               |      irq_enter() {
 1)   0.000 us    |        idle_cpu();
 1)               |        tick_check_idle() {
 1)   0.000 us    |          tick_check_oneshot_broadcast();
 1)               |          tick_nohz_stop_idle() {
......

It's very embarrassing, it ouputs "__irqentry_text_start()",
actually, it should output "smp_apic_timer_interrupt()".
(these two symbol are the same address, but "__irqentry_text_start"
is deemed to the first aliased symbol by scripts/kallsyms)

This patch puts symbols like "__irqentry_text_start" to the second
aliased symbols. And a more proper symbol name becomes the first.

Aliased symbols mostly come from linker script. The solution is
guessing "is this symbol defined in linker script", the symbols
defined in linker script will not become the first aliased symbol.

And if symbols are found to be equal in this "linker script provided"
criteria, symbols are sorted by the number of prefix underscores.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index ad2434b..b187edc 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -500,6 +500,51 @@ static void optimize_token_table(void)
 	optimize_result();
 }
 
+/* guess for "linker script provide" symbol */
+static int may_be_linker_script_provide_symbol(const struct sym_entry *se)
+{
+	const char *symbol = (char *)se->sym + 1;
+	int len = se->len - 1;
+
+	if (len < 8)
+		return 0;
+
+	if (symbol[0] != '_' || symbol[1] != '_')
+		return 0;
+
+	/* __start_XXXXX */
+	if (!memcmp(symbol + 2, "start_", 6))
+		return 1;
+
+	/* __stop_XXXXX */
+	if (!memcmp(symbol + 2, "stop_", 5))
+		return 1;
+
+	/* __end_XXXXX */
+	if (!memcmp(symbol + 2, "end_", 4))
+		return 1;
+
+	/* __XXXXX_start */
+	if (!memcmp(symbol + len - 6, "_start", 6))
+		return 1;
+
+	/* __XXXXX_end */
+	if (!memcmp(symbol + len - 4, "_end", 4))
+		return 1;
+
+	return 0;
+}
+
+static int prefix_underscores_count(const char *str)
+{
+	const char *tail = str;
+
+	while (*tail == '_')
+		tail++;
+
+	return tail - str;
+}
+
 static int compare_symbols(const void *a, const void *b)
 {
 	const struct sym_entry *sa;
@@ -521,6 +566,18 @@ static int compare_symbols(const void *a, const void *b)
 	if (wa != wb)
 		return wa - wb;
 
+	/* sort by "linker script provide" type */
+	wa = may_be_linker_script_provide_symbol(sa);
+	wb = may_be_linker_script_provide_symbol(sb);
+	if (wa != wb)
+		return wa - wb;
+
+	/* sort by the number of prefix underscores */
+	wa = prefix_underscores_count((const char *)sa->sym + 1);
+	wb = prefix_underscores_count((const char *)sb->sym + 1);
+	if (wa != wb)
+		return wa - wb;
+
 	/* sort by initial order, so that other symbols are left undisturbed */
 	return sa->start_pos - sb->start_pos;
 }


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

* Re: [PATCH V3] kallsyms, tracing: output more proper symbol name
  2009-03-13  9:27           ` Lai Jiangshan
@ 2009-03-13 19:25             ` Paulo Marques
  2009-03-13 19:40             ` Sam Ravnborg
  1 sibling, 0 replies; 14+ messages in thread
From: Paulo Marques @ 2009-03-13 19:25 UTC (permalink / raw)
  To: Lai Jiangshan
  Cc: Ingo Molnar, Sam Ravnborg, Andrew Morton, Steven Rostedt,
	Frederic Weisbecker, LKML

Lai Jiangshan wrote:
> [...]
> [resend, previous v3 has some mistakes]
> 
> Subject:  kallsyms, tracing: output more proper symbol name
> 
> Impact: bugfix, output reliable result
> 
> Debug tools(dump_stack(), ftrace...) are like to print out symbols.
> But it is always print out the first aliased symbol.(Aliased symbols
> are symbols with the same address), and the first aliased symbol is
> sometime not proper.
> [...]
> Aliased symbols mostly come from linker script. The solution is
> guessing "is this symbol defined in linker script", the symbols
> defined in linker script will not become the first aliased symbol.
> 
> And if symbols are found to be equal in this "linker script provided"
> criteria, symbols are sorted by the number of prefix underscores.
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> [...]
> +static int prefix_underscores_count(const char *str)
> +{
> +	const char *tail = str;
> +
> +	while (*tail == '_')
> +		tail++;
> +
> +	return tail - str;
> +}
> +
>  static int compare_symbols(const void *a, const void *b)
>  {
>  	const struct sym_entry *sa;
> @@ -521,6 +566,18 @@ static int compare_symbols(const void *a, const void *b)
>  	if (wa != wb)
>  		return wa - wb;
>  
> +	/* sort by "linker script provide" type */
> +	wa = may_be_linker_script_provide_symbol(sa);
> +	wb = may_be_linker_script_provide_symbol(sb);
> +	if (wa != wb)
> +		return wa - wb;
> +
> +	/* sort by the number of prefix underscores */
> +	wa = prefix_underscores_count((const char *)sa->sym + 1);
> +	wb = prefix_underscores_count((const char *)sb->sym + 1);
> +	if (wa != wb)
> +		return wa - wb;
> +
>  	/* sort by initial order, so that other symbols are left undisturbed */
>  	return sa->start_pos - sb->start_pos;
>  }

The code seems fine. I have no time to compile and test it right now,
but there are no obvious problems as far as I can see.

Reviewed-by: Paulo Marques <pmarques@grupopie.com>

-- 
Paulo Marques - www.grupopie.com

"Measure with laser, mark with chalk, cut with chainsaw."

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

* Re: [PATCH V3] kallsyms, tracing: output more proper symbol name
  2009-03-13  9:27           ` Lai Jiangshan
  2009-03-13 19:25             ` Paulo Marques
@ 2009-03-13 19:40             ` Sam Ravnborg
  2009-03-13 20:07               ` Andrew Morton
  1 sibling, 1 reply; 14+ messages in thread
From: Sam Ravnborg @ 2009-03-13 19:40 UTC (permalink / raw)
  To: Lai Jiangshan
  Cc: Paulo Marques, Ingo Molnar, Andrew Morton, Steven Rostedt,
	Frederic Weisbecker, LKML

On Fri, Mar 13, 2009 at 05:27:32PM +0800, Lai Jiangshan wrote:
> Lai Jiangshan wrote:
> > Paulo Marques wrote:
> >> I would probably still place another test to compare the number of
> >> underscores, after both symbols are found to be equal in the "linker
> >> script provided" criteria.
> >>
> >> The way it is now, for my current kallsyms table:
> >>
> >>> ffffffff80200000 A _text
> >>> ffffffff80200000 T startup_64
> >>> ffffffff80209000 T _stext
> >>> ffffffff80209000 t init_post
> >> a "startup_64" address would display as "_text" and a "init_post"
> >> address would display as "_stext".
> >>
> >> You can add all the stext/etext symbols as special cases to the
> >> "may_be_linker_script_provide_symbol" function (like it was on your v1
> >> patch), but I'm just afraid that we'll find more cases in the future
> >> that are not automatically caught by these rules...
> >>
> > 
> 
> [resend, previous v3 has some mistakes]
> 
> Subject:  kallsyms, tracing: output more proper symbol name
> 
> Impact: bugfix, output reliable result
> 
> Debug tools(dump_stack(), ftrace...) are like to print out symbols.
> But it is always print out the first aliased symbol.(Aliased symbols
> are symbols with the same address), and the first aliased symbol is
> sometime not proper.
> 
> # echo function_graph > current_tracer
> # cat trace
> ......
>  1)   1.923 us    |    select_nohz_load_balancer();
>  1) + 76.692 us   |  }
>  1)               |  default_idle() {
>  1)   ==========> |    __irqentry_text_start() {
>  1)   0.000 us    |      native_apic_mem_write();
>  1)               |      irq_enter() {
>  1)   0.000 us    |        idle_cpu();
>  1)               |        tick_check_idle() {
>  1)   0.000 us    |          tick_check_oneshot_broadcast();
>  1)               |          tick_nohz_stop_idle() {
> ......
> 
> It's very embarrassing, it ouputs "__irqentry_text_start()",
> actually, it should output "smp_apic_timer_interrupt()".
> (these two symbol are the same address, but "__irqentry_text_start"
> is deemed to the first aliased symbol by scripts/kallsyms)
> 
> This patch puts symbols like "__irqentry_text_start" to the second
> aliased symbols. And a more proper symbol name becomes the first.
> 
> Aliased symbols mostly come from linker script. The solution is
> guessing "is this symbol defined in linker script", the symbols
> defined in linker script will not become the first aliased symbol.
> 
> And if symbols are found to be equal in this "linker script provided"
> criteria, symbols are sorted by the number of prefix underscores.
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>

Looks good to me.

Acked-by: Sam Ravnborg <sam@ravnborg.org>

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

* Re: [PATCH V3] kallsyms, tracing: output more proper symbol name
  2009-03-13 19:40             ` Sam Ravnborg
@ 2009-03-13 20:07               ` Andrew Morton
  2009-03-13 20:34                 ` Paulo Marques
  2009-03-14  8:54                 ` Ingo Molnar
  0 siblings, 2 replies; 14+ messages in thread
From: Andrew Morton @ 2009-03-13 20:07 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: laijs, pmarques, mingo, srostedt, fweisbec, linux-kernel

On Fri, 13 Mar 2009 20:40:13 +0100
Sam Ravnborg <sam@ravnborg.org> wrote:

> > This patch puts symbols like "__irqentry_text_start" to the second
> > aliased symbols. And a more proper symbol name becomes the first.
> > 
> > Aliased symbols mostly come from linker script. The solution is
> > guessing "is this symbol defined in linker script", the symbols
> > defined in linker script will not become the first aliased symbol.
> > 
> > And if symbols are found to be equal in this "linker script provided"
> > criteria, symbols are sorted by the number of prefix underscores.
> > 
> > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> 
> Looks good to me.
> 
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

So... do we need this in 2.6.29?

I'm about to jump on a plane and shall be a bit intermittent for a week.

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

* Re: [PATCH V3] kallsyms, tracing: output more proper symbol name
  2009-03-13 20:07               ` Andrew Morton
@ 2009-03-13 20:34                 ` Paulo Marques
  2009-03-14  8:54                 ` Ingo Molnar
  1 sibling, 0 replies; 14+ messages in thread
From: Paulo Marques @ 2009-03-13 20:34 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sam Ravnborg, laijs, mingo, srostedt, fweisbec, linux-kernel

Andrew Morton wrote:
> On Fri, 13 Mar 2009 20:40:13 +0100
> Sam Ravnborg <sam@ravnborg.org> wrote:
> 
>>> This patch puts symbols like "__irqentry_text_start" to the second
>>> aliased symbols. And a more proper symbol name becomes the first.
>>>
>>> Aliased symbols mostly come from linker script. The solution is
>>> guessing "is this symbol defined in linker script", the symbols
>>> defined in linker script will not become the first aliased symbol.
>>>
>>> And if symbols are found to be equal in this "linker script provided"
>>> criteria, symbols are sorted by the number of prefix underscores.
>>>
>>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>> Looks good to me.
>>
>> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
> So... do we need this in 2.6.29?

IMHO, no. I think the way the kernel handles aliased symbols hasn't
changed in a very long time so one more release shouldn't make a big
difference.

Even if the risk of the patch breaking something is small, taking that
chance for such a small gain this late in the 2.6.29 cycle is just not
worth it.

-- 
Paulo Marques - www.grupopie.com

"You're just jealous because the voices only talk to me."

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

* Re: [PATCH V3] kallsyms, tracing: output more proper symbol name
  2009-03-13 20:07               ` Andrew Morton
  2009-03-13 20:34                 ` Paulo Marques
@ 2009-03-14  8:54                 ` Ingo Molnar
  1 sibling, 0 replies; 14+ messages in thread
From: Ingo Molnar @ 2009-03-14  8:54 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sam Ravnborg, laijs, pmarques, srostedt, fweisbec, linux-kernel


* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Fri, 13 Mar 2009 20:40:13 +0100
> Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > > This patch puts symbols like "__irqentry_text_start" to the second
> > > aliased symbols. And a more proper symbol name becomes the first.
> > > 
> > > Aliased symbols mostly come from linker script. The solution is
> > > guessing "is this symbol defined in linker script", the symbols
> > > defined in linker script will not become the first aliased symbol.
> > > 
> > > And if symbols are found to be equal in this "linker script provided"
> > > criteria, symbols are sorted by the number of prefix underscores.
> > > 
> > > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> > 
> > Looks good to me.
> > 
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
> So... do we need this in 2.6.29?

no. This got raised in the context of new tracing changes queued 
up for 2.6.30.

	Ingo

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

* [tip:tracing/ftrace] kallsyms, tracing: output more proper symbol name
  2009-03-13  7:10         ` [PATCH V3] " Lai Jiangshan
  2009-03-13  9:27           ` Lai Jiangshan
@ 2009-03-14  8:57           ` Lai Jiangshan
  1 sibling, 0 replies; 14+ messages in thread
From: Lai Jiangshan @ 2009-03-14  8:57 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, sam, pmarques, tglx, laijs, mingo

Commit-ID:  b478b782e110fdb4135caa3062b6d687e989d994
Gitweb:     http://git.kernel.org/tip/b478b782e110fdb4135caa3062b6d687e989d994
Author:     Lai Jiangshan <laijs@cn.fujitsu.com>
AuthorDate: Fri, 13 Mar 2009 15:10:26 +0800
Commit:     Ingo Molnar <mingo@elte.hu>
CommitDate: Sat, 14 Mar 2009 09:55:04 +0100

kallsyms, tracing: output more proper symbol name

Impact: bugfix, output more reliable symbol lookup result

Debug tools(dump_stack(), ftrace...) are like to print out symbols.
But it is always print out the first aliased symbol.(Aliased symbols
are symbols with the same address), and the first aliased symbol is
sometime not proper.

 # echo function_graph > current_tracer
 # cat trace
.....
 1)   1.923 us    |    select_nohz_load_balancer();
 1) + 76.692 us   |  }
 1)               |  default_idle() {
 1)   ==========> |    __irqentry_text_start() {
 1)   0.000 us    |      native_apic_mem_write();
 1)               |      irq_enter() {
 1)   0.000 us    |        idle_cpu();
 1)               |        tick_check_idle() {
 1)   0.000 us    |          tick_check_oneshot_broadcast();
 1)               |          tick_nohz_stop_idle() {
.....

It's very embarrassing, it ouputs "__irqentry_text_start()",
actually, it should output "smp_apic_timer_interrupt()".
(these two symbol are the same address, but "__irqentry_text_start"
is deemed to the first aliased symbol by scripts/kallsyms)

This patch puts symbols like "__irqentry_text_start" to the second
aliased symbols. And a more proper symbol name becomes the first.

Aliased symbols mostly come from linker script. The solution is
guessing "is this symbol defined in linker script", the symbols
defined in linker script will not become the first aliased symbol.

And if symbols are found to be equal in this "linker script provided"
criteria, symbols are sorted by the number of prefix underscores.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Paulo Marques <pmarques@grupopie.com>
LKML-Reference: <49BA06E2.7080807@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 scripts/kallsyms.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index ad2434b..6654cbe 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -500,6 +500,51 @@ static void optimize_token_table(void)
 	optimize_result();
 }
 
+/* guess for "linker script provide" symbol */
+static int may_be_linker_script_provide_symbol(const struct sym_entry *se)
+{
+	const char *symbol = (char *)se->sym + 1;
+	int len = se->len - 1;
+
+	if (len < 8)
+		return 0;
+
+	if (symbol[0] != '_' || symbol[1] != '_')
+		return 0;
+
+	/* __start_XXXXX */
+	if (!memcmp(symbol + 2, "start_", 6))
+		return 1;
+
+	/* __stop_XXXXX */
+	if (!memcmp(symbol + 2, "stop_", 5))
+		return 1;
+
+	/* __end_XXXXX */
+	if (!memcmp(symbol + 2, "end_", 4))
+		return 1;
+
+	/* __XXXXX_start */
+	if (!memcmp(symbol + len - 6, "_start", 6))
+		return 1;
+
+	/* __XXXXX_end */
+	if (!memcmp(symbol + len - 4, "_end", 4))
+		return 1;
+
+	return 0;
+}
+
+static int prefix_underscores_count(const char *str)
+{
+	const char *tail = str;
+
+	while (*tail != '_')
+		tail++;
+
+	return tail - str;
+}
+
 static int compare_symbols(const void *a, const void *b)
 {
 	const struct sym_entry *sa;
@@ -521,6 +566,18 @@ static int compare_symbols(const void *a, const void *b)
 	if (wa != wb)
 		return wa - wb;
 
+	/* sort by "linker script provide" type */
+	wa = may_be_linker_script_provide_symbol(sa);
+	wb = may_be_linker_script_provide_symbol(sb);
+	if (wa != wb)
+		return wa - wb;
+
+	/* sort by the number of prefix underscores */
+	wa = prefix_underscores_count((const char *)sa->sym + 1);
+	wb = prefix_underscores_count((const char *)sb->sym + 1);
+	if (wa != wb)
+		return wa - wb;
+
 	/* sort by initial order, so that other symbols are left undisturbed */
 	return sa->start_pos - sb->start_pos;
 }

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

end of thread, other threads:[~2009-03-14  8:58 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-11  9:37 [PATCH] kallsyms, tracing: output more proper symbol name Lai Jiangshan
2009-03-11  9:46 ` Ingo Molnar
2009-03-11 10:29   ` Frederic Weisbecker
2009-03-11 13:05   ` Paulo Marques
2009-03-12  2:43     ` Lai Jiangshan
2009-03-12 15:32       ` Paulo Marques
2009-03-13  7:10         ` [PATCH V3] " Lai Jiangshan
2009-03-13  9:27           ` Lai Jiangshan
2009-03-13 19:25             ` Paulo Marques
2009-03-13 19:40             ` Sam Ravnborg
2009-03-13 20:07               ` Andrew Morton
2009-03-13 20:34                 ` Paulo Marques
2009-03-14  8:54                 ` Ingo Molnar
2009-03-14  8:57           ` [tip:tracing/ftrace] " Lai Jiangshan

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