[tip:core/stacktrace] symbols, stacktrace: look up init symbols after module symbols

[tip:core/stacktrace] symbols, stacktrace: look up init symbols after module symbols

[Ingo Molnar]

Commit-ID:  4a44bac1f98223ed77e47bf3b42fcfd10cddd85f
Gitweb:     http://git.kernel.org/tip/4a44bac1f98223ed77e47bf3b42fcfd10cddd85f
Author:     Ingo Molnar <mingo@elte.hu>
AuthorDate: Thu, 19 Mar 2009 13:21:44 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 19 Mar 2009 13:38:35 +0100

symbols, stacktrace: look up init symbols after module symbols

Impact: fix incomplete stacktraces

I noticed such weird stacktrace entries in lockdep dumps:

[    0.285956] {HARDIRQ-ON-W} state was registered at:
[    0.285956]   [<ffffffff802bce90>] mark_irqflags+0xbe/0x125
[    0.285956]   [<ffffffff802bf2fd>] __lock_acquire+0x674/0x82d
[    0.285956]   [<ffffffff802bf5b2>] lock_acquire+0xfc/0x128
[    0.285956]   [<ffffffff8135b636>] rt_spin_lock+0xc8/0xd0
[    0.285956]   [<ffffffffffffffff>] 0xffffffffffffffff

The stacktrace entry is cut off after rt_spin_lock.

After much debugging i found out that stacktrace entries that
belong to init symbols dont get printed out, due to commit:

  a2da405: module: Don't report discarded init pages as kernel text.

The reason is this check added to core_kernel_text():

-       if (addr >= (unsigned long)_sinittext &&
+       if (system_state == SYSTEM_BOOTING &&
+           addr >= (unsigned long)_sinittext &&
            addr <= (unsigned long)_einittext)
                return 1;

This will discard inittext symbols even though their symbol table
is still present and even though stacktraces done while the system
was booting up might still be relevant.

To not reintroduce the (not well-specified) bug addressed in that
commit, first do a module symbols lookup, then a final init-symbols
lookup.

This will work fine on architectures that have separate address
spaces for modules (such as x86) - and should not crash any other
architectures either.

Acked-by: Peter Zijlstra <peterz@infradead.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
LKML-Reference: <new-discussion>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/extable.c |   25 ++++++++++++++++++++++---
 1 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/kernel/extable.c b/kernel/extable.c
index e136ed8..c46da6a 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -41,6 +41,14 @@ const struct exception_table_entry *search_exception_tables(unsigned long addr)
 	return e;
 }
 
+static inline int init_kernel_text(unsigned long addr)
+{
+	if (addr >= (unsigned long)_sinittext &&
+	    addr <= (unsigned long)_einittext)
+		return 1;
+	return 0;
+}
+
 __notrace_funcgraph int core_kernel_text(unsigned long addr)
 {
 	if (addr >= (unsigned long)_stext &&
@@ -48,8 +56,7 @@ __notrace_funcgraph int core_kernel_text(unsigned long addr)
 		return 1;
 
 	if (system_state == SYSTEM_BOOTING &&
-	    addr >= (unsigned long)_sinittext &&
-	    addr <= (unsigned long)_einittext)
+	    init_kernel_text(addr))
 		return 1;
 	return 0;
 }
@@ -58,7 +65,19 @@ __notrace_funcgraph int __kernel_text_address(unsigned long addr)
 {
 	if (core_kernel_text(addr))
 		return 1;
-	return __module_text_address(addr) != NULL;
+	if (__module_text_address(addr))
+		return 1;
+	/*
+	 * There might be init symbols in saved stacktraces.
+	 * Give those symbols a chance to be printed in
+	 * backtraces (such as lockdep traces).
+	 *
+	 * Since we are after the module-symbols check, there's
+	 * no danger of address overlap:
+	 */
+	if (init_kernel_text(addr))
+		return 1;
+	return 0;
 }
 
 int kernel_text_address(unsigned long addr)

Re: [tip:core/stacktrace] symbols, stacktrace: look up init symbols after module symbols

[Rusty Russell]

On Thursday 19 March 2009 22:24:35 Ingo Molnar wrote:
>   a2da405: module: Don't report discarded init pages as kernel text.
> 
> The reason is this check added to core_kernel_text():
> 
> -       if (addr >= (unsigned long)_sinittext &&
> +       if (system_state == SYSTEM_BOOTING &&
> +           addr >= (unsigned long)_sinittext &&
>             addr <= (unsigned long)_einittext)
>                 return 1;
> 
> This will discard inittext symbols even though their symbol table
> is still present and even though stacktraces done while the system
> was booting up might still be relevant.
> 
> To not reintroduce the (not well-specified) bug addressed in that
> commit, first do a module symbols lookup, then a final init-symbols
> lookup.
> 
> This will work fine on architectures that have separate address
> spaces for modules (such as x86) - and should not crash any other
> architectures either.

Returning "1" all the time won't crash them either, AFAICT, but it's still
misleading if kernel_text_address() isn't reliable.  It makes me
uncomfortable.

Does every kernel_text_address() caller want this behavior, or should we
have a was_ever_text_address()?

Rusty.