x86/dumpstack: replace dump_trace() with a new unwinder

PATCH Superseded Josh Poimboeuf 2 participants 8 messages Sep 15, 2016 – Sep 16, 2016

Subsystems (3)

x86 perf events x86 stack unwinding

lore

Share & tools

lore

Josh Poimboeuf

author

2016-09-15 15:56 UTC #0 lore raw

cover x86/dumpstack: replace dump_trace() with a new unwinder

The last version was posted as part of a much larger patch set:

https://lkml.kernel.org/r/[email protected]

People complained about the set being too big, so it was split up into more digestible pieces. All the prerequisite patches for the unwinder have now been merged.

See patch 1/6 for the background and justification for this change.

Josh Poimboeuf (6):
x86/unwind: add new unwind interface and implementations perf/x86: convert perf_callchain_kernel() to use the new unwinder x86/stacktrace: convert save_stack_trace_*() to use the new unwinder oprofile/x86: convert x86_backtrace() to use the new unwinder x86/dumpstack: convert show_trace_log_lvl() to use the new unwinder x86/dumpstack: remove dump_trace() and related callbacks

 arch/x86/events/core.c            |  33 +++-----
 arch/x86/include/asm/stacktrace.h |  46 +---------
 arch/x86/include/asm/unwind.h     |  74 ++++++++++++++++
 arch/x86/kernel/Makefile          |   6 ++
 arch/x86/kernel/dumpstack.c       | 174 +++++++++++++++++---------------------
 arch/x86/kernel/dumpstack_32.c    |  44 +---------
 arch/x86/kernel/dumpstack_64.c    |  78 +----------------
 arch/x86/kernel/stacktrace.c      |  74 +++++++---------
 arch/x86/kernel/unwind_frame.c    |  93 ++++++++++++++++++++
 arch/x86/kernel/unwind_guess.c    |  43 ++++++++++
 arch/x86/oprofile/backtrace.c     |  39 ++++-----
 11 files changed, 362 insertions(+), 342 deletions(-)
 create mode 100644 arch/x86/include/asm/unwind.h
 create mode 100644 arch/x86/kernel/unwind_frame.c
 create mode 100644 arch/x86/kernel/unwind_guess.c

Signature · 1 line

2.7.4

Josh Poimboeuf

author

2016-09-15 15:56 UTC #1 lore raw

PATCH 1/6 x86/unwind: add new unwind interface and implementations

The x86 stack dump code is a bit of a mess. dump_trace() uses callbacks, and each user of it seems to have slightly different requirements, so there are several slightly different callbacks floating around.

Also there are some upcoming features which will need more changes to the stack dump code, including the printing of stack pt_regs, reliable stack detection for live patching, and a DWARF unwinder. Each of those features would at least need more callbacks and/or callback interfaces, resulting in a much bigger mess than what we have today.

Before doing all that, we should try to clean things up and replace dump_trace() with something cleaner and more flexible.

The new unwinder is a simple state machine which was heavily inspired by a suggestion from Andy Lutomirski:

https://lkml.kernel.org/r/CALCETrUbNTqaM2LRyXGRx=kVLRPeY5A3Pc6k4TtQxF320rUT=w@mail.gmail.com

It's also similar to the libunwind API:

http://www.nongnu.org/libunwind/man/libunwind(3).html

Some if its advantages:

- Simplicity: no more callback sprawl and less code duplication.

- Flexibility: it allows the caller to stop and inspect the stack state at each step in the unwinding process.

- Modularity: the unwinder code, console stack dump code, and stack metadata analysis code are all better separated so that changing one of them shouldn't have much of an impact on any of the others.

Two implementations are added which conform to the new unwind interface:

- The frame pointer unwinder which is used for CONFIG_FRAME_POINTER=y.

- The "guess" unwinder which is used for CONFIG_FRAME_POINTER=n. This isn't an "unwinder" per se. All it does is scan the stack for kernel text addresses. But with no frame pointers, guesses are better than nothing in most cases.

Signed-off-by Josh Poimboeuf <[email protected]>

---
 arch/x86/include/asm/unwind.h  | 74 +++++++++++++++++++++++++++++++++
 arch/x86/kernel/Makefile       |  6 +++
 arch/x86/kernel/unwind_frame.c | 93 ++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/unwind_guess.c | 43 +++++++++++++++++++
 4 files changed, 216 insertions(+)
 create mode 100644 arch/x86/include/asm/unwind.h
 create mode 100644 arch/x86/kernel/unwind_frame.c
 create mode 100644 arch/x86/kernel/unwind_guess.c

arch/x86/include/asm/unwind.h new file +74 -0

new file mode 100644
index 0000000..7927944
@@ -0,0 +1,74 @@
+#ifndef _ASM_X86_UNWIND_H
+#define _ASM_X86_UNWIND_H
+
+#include <linux/sched.h>
+#include <linux/ftrace.h>
+#include <asm/ptrace.h>
+#include <asm/stacktrace.h>
+
+struct unwind_state {
+	struct stack_info stack_info;
+	unsigned long stack_mask;
+	struct task_struct *task;
+	int graph_idx;
+#ifdef CONFIG_FRAME_POINTER
+	unsigned long *bp;
+#else
+	unsigned long *sp;
+#endif
+};
+
+void __unwind_start(struct unwind_state *state, struct task_struct *task,
+		    struct pt_regs *regs, unsigned long *first_frame);
+
+bool unwind_next_frame(struct unwind_state *state);
+
+static inline bool unwind_done(struct unwind_state *state)
+{
+	return state->stack_info.type == STACK_TYPE_UNKNOWN;
+}
+
+static inline
+void unwind_start(struct unwind_state *state, struct task_struct *task,
+		  struct pt_regs *regs, unsigned long *first_frame)
+{
+	task = task ? : current;
+	first_frame = first_frame ? : get_stack_pointer(task, regs);
+
+	__unwind_start(state, task, regs, first_frame);
+}
+
+#ifdef CONFIG_FRAME_POINTER
+
+static inline
+unsigned long *unwind_get_return_address_ptr(struct unwind_state *state)
+{
+	if (unwind_done(state))
+		return NULL;
+
+	return state->bp + 1;
+}
+
+unsigned long unwind_get_return_address(struct unwind_state *state);
+
+#else /* !CONFIG_FRAME_POINTER */
+
+static inline
+unsigned long *unwind_get_return_address_ptr(struct unwind_state *state)
+{
+	return NULL;
+}
+
+static inline
+unsigned long unwind_get_return_address(struct unwind_state *state)
+{
+	if (unwind_done(state))
+		return 0;
+
+	return ftrace_graph_ret_addr(state->task, &state->graph_idx,
+				     *state->sp, state->sp);
+}
+
+#endif /* CONFIG_FRAME_POINTER */
+
+#endif /* _ASM_X86_UNWIND_H */

arch/x86/kernel/Makefile +6 -0

index 0503f5b..45257cf 100644
@@ -125,6 +125,12 @@ obj-$(CONFIG_EFI)			+= sysfb_efi.o
 obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o
 obj-$(CONFIG_TRACING)			+= tracepoint.o
 
+ifdef CONFIG_FRAME_POINTER
+obj-y					+= unwind_frame.o
+else
+obj-y					+= unwind_guess.o
+endif
+
 ###
 # 64 bit specific files
 ifeq ($(CONFIG_X86_64),y)

arch/x86/kernel/unwind_frame.c new file +93 -0

93 lines not on this page · raw message

arch/x86/kernel/unwind_guess.c new file +43 -0

43 lines not on this page · raw message

Signature · 1 line

2.7.4

Josh Poimboeuf

author

2016-09-15 15:56 UTC #2 lore raw

PATCH 2/6 perf/x86: convert perf_callchain_kernel() to use the new unwinder

In reply to Josh Poimboeuf

Convert perf_callchain_kernel() to use the new unwinder. dump_trace() has been deprecated.

Signed-off-by Josh Poimboeuf <[email protected]>

---
 arch/x86/events/core.c | 33 ++++++++++-----------------------
 1 file changed, 10 insertions(+), 23 deletions(-)

arch/x86/events/core.c +10 ~~-23~~

index dd3a1dc..b409e7c 100644
@@ -37,6 +37,7 @@
 #include <asm/timer.h>
 #include <asm/desc.h>
 #include <asm/ldt.h>
+#include <asm/unwind.h>
 
 #include "perf_event.h"

@@ -2267,31 +2268,12 @@ void arch_perf_update_userpage(struct perf_event *event,
 	cyc2ns_read_end(data);
 }
 
-/*
- * callchain support
- */
-
-static int backtrace_stack(void *data, const char *name)
-{
-	return 0;
-}
-
-static int backtrace_address(void *data, unsigned long addr, int reliable)
-{
-	struct perf_callchain_entry_ctx *entry = data;
-
-	return perf_callchain_store(entry, addr);
-}
-
-static const struct stacktrace_ops backtrace_ops = {
-	.stack			= backtrace_stack,
-	.address		= backtrace_address,
-	.walk_stack		= print_context_stack_bp,
-};
-
 void
 perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
 {
+	struct unwind_state state;
+	unsigned long addr;
+
 	if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
 		/* TODO: We don't support guest os callchain now */
 		return;

@@ -2300,7 +2282,12 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *re
 	if (perf_callchain_store(entry, regs->ip))
 		return;
 
-	dump_trace(NULL, regs, NULL, 0, &backtrace_ops, entry);
+	for (unwind_start(&state, NULL, regs, NULL); !unwind_done(&state);
+	     unwind_next_frame(&state)) {
+		addr = unwind_get_return_address(&state);
+		if (!addr || perf_callchain_store(entry, addr))
+			return;
+	}
 }
 
 static inline int

Signature · 1 line

2.7.4

Josh Poimboeuf

author

2016-09-15 15:56 UTC #3 lore raw

PATCH 3/6 x86/stacktrace: convert save_stack_trace_*() to use the new unwinder

In reply to Josh Poimboeuf

Convert save_stack_trace_*() to use the new unwinder. dump_trace() has been deprecated.

Signed-off-by Josh Poimboeuf <[email protected]>

---
 arch/x86/kernel/stacktrace.c | 74 +++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 45 deletions(-)

arch/x86/kernel/stacktrace.c +29 ~~-45~~

index 785aef1..a168e7e 100644
@@ -8,80 +8,64 @@
 #include <linux/export.h>
 #include <linux/uaccess.h>
 #include <asm/stacktrace.h>
+#include <asm/unwind.h>
 
-static int save_stack_stack(void *data, const char *name)
+static int save_stack_address(struct stack_trace *trace, unsigned long addr,
+			      bool nosched)
 {
-	return 0;
-}
-
-static int
-__save_stack_address(void *data, unsigned long addr, bool reliable, bool nosched)
-{
-	struct stack_trace *trace = data;
-#ifdef CONFIG_FRAME_POINTER
-	if (!reliable)
-		return 0;
-#endif
 	if (nosched && in_sched_functions(addr))
 		return 0;
+
 	if (trace->skip > 0) {
 		trace->skip--;
 		return 0;
 	}
-	if (trace->nr_entries < trace->max_entries) {
-		trace->entries[trace->nr_entries++] = addr;
-		return 0;
-	} else {
-		return -1; /* no more room, stop walking the stack */
-	}
-}
 
-static int save_stack_address(void *data, unsigned long addr, int reliable)
-{
-	return __save_stack_address(data, addr, reliable, false);
+	if (trace->nr_entries >= trace->max_entries)
+		return -1;
+
+	trace->entries[trace->nr_entries++] = addr;
+	return 0;
 }
 
-static int
-save_stack_address_nosched(void *data, unsigned long addr, int reliable)
+static void __save_stack_trace(struct stack_trace *trace,
+			       struct task_struct *task, struct pt_regs *regs,
+			       bool nosched)
 {
-	return __save_stack_address(data, addr, reliable, true);
-}
+	struct unwind_state state;
+	unsigned long addr;
 
-static const struct stacktrace_ops save_stack_ops = {
-	.stack		= save_stack_stack,
-	.address	= save_stack_address,
-	.walk_stack	= print_context_stack,
-};
+	if (regs)
+		save_stack_address(trace, regs->ip, nosched);
 
-static const struct stacktrace_ops save_stack_ops_nosched = {
-	.stack		= save_stack_stack,
-	.address	= save_stack_address_nosched,
-	.walk_stack	= print_context_stack,
-};
+	for (unwind_start(&state, task, regs, NULL); !unwind_done(&state);
+	     unwind_next_frame(&state)) {
+		addr = unwind_get_return_address(&state);
+		if (!addr || save_stack_address(trace, addr, nosched))
+			break;
+	}
+
+	if (trace->nr_entries < trace->max_entries)
+		trace->entries[trace->nr_entries++] = ULONG_MAX;
+}
 
 /*
  * Save stack-backtrace addresses into a stack_trace buffer.
  */
 void save_stack_trace(struct stack_trace *trace)
 {
-	dump_trace(current, NULL, NULL, 0, &save_stack_ops, trace);
-	if (trace->nr_entries < trace->max_entries)
-		trace->entries[trace->nr_entries++] = ULONG_MAX;
+	__save_stack_trace(trace, NULL, NULL, false);
 }
 EXPORT_SYMBOL_GPL(save_stack_trace);
 
 void save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)
 {
-	dump_trace(current, regs, NULL, 0, &save_stack_ops, trace);
-	if (trace->nr_entries < trace->max_entries)
-		trace->entries[trace->nr_entries++] = ULONG_MAX;
+	__save_stack_trace(trace, NULL, regs, false);
 }
 
 void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 {
-	dump_trace(tsk, NULL, NULL, 0, &save_stack_ops_nosched, trace);
-	if (trace->nr_entries < trace->max_entries)
-		trace->entries[trace->nr_entries++] = ULONG_MAX;
+	__save_stack_trace(trace, tsk, NULL, true);
 }
 EXPORT_SYMBOL_GPL(save_stack_trace_tsk);

Signature · 1 line

2.7.4

Josh Poimboeuf

author

2016-09-15 15:56 UTC #4 lore raw

PATCH 4/6 oprofile/x86: convert x86_backtrace() to use the new unwinder

In reply to Josh Poimboeuf

Convert oprofile's x86_backtrace() to use the new unwinder. dump_trace() has been deprecated.

Cc Robert Richter <[email protected]>

Signed-off-by Josh Poimboeuf <[email protected]>

---
 arch/x86/oprofile/backtrace.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

arch/x86/oprofile/backtrace.c +17 ~~-22~~

index 7539148..f28ac1a 100644
@@ -16,27 +16,7 @@
 
 #include <asm/ptrace.h>
 #include <asm/stacktrace.h>
-
-static int backtrace_stack(void *data, const char *name)
-{
-	/* Yes, we want all stacks */
-	return 0;
-}
-
-static int backtrace_address(void *data, unsigned long addr, int reliable)
-{
-	unsigned int *depth = data;
-
-	if ((*depth)--)
-		oprofile_add_trace(addr);
-	return 0;
-}
-
-static struct stacktrace_ops backtrace_ops = {
-	.stack		= backtrace_stack,
-	.address	= backtrace_address,
-	.walk_stack	= print_context_stack,
-};
+#include <asm/unwind.h>
 
 #ifdef CONFIG_COMPAT
 static struct stack_frame_ia32 *

@@ -113,14 +93,29 @@ x86_backtrace(struct pt_regs * const regs, unsigned int depth)
 	struct stack_frame *head = (struct stack_frame *)frame_pointer(regs);
 
 	if (!user_mode(regs)) {
+		struct unwind_state state;
+		unsigned long addr;
+
 		if (!depth)
 			return;
 
 		oprofile_add_trace(regs->ip);
+
 		if (!--depth)
 			return;
 
-		dump_trace(NULL, regs, NULL, 0, &backtrace_ops, &depth);
+		for (unwind_start(&state, NULL, regs, NULL);
+		     !unwind_done(&state); unwind_next_frame(&state)) {
+			addr = unwind_get_return_address(&state);
+			if (!addr)
+				break;
+
+			oprofile_add_trace(addr);
+
+			if (!--depth)
+				break;
+		}
+
 		return;
 	}

Signature · 1 line

2.7.4

Josh Poimboeuf

author

2016-09-15 15:56 UTC #5 lore raw

PATCH 5/6 x86/dumpstack: convert show_trace_log_lvl() to use the new unwinder

In reply to Josh Poimboeuf

Convert show_trace_log_lvl() to use the new unwinder. dump_trace() has been deprecated.

show_trace_log_lvl() is special compared to other users of the unwinder. It's the only place where both reliable *and* unreliable addresses are needed. With frame pointers enabled, most callers of the unwinder don't want to know about unreliable addresses. But in this case, when we're dumping the stack to the console because something presumably went wrong, the unreliable addresses are useful:

- They show stale data on the stack which can provide useful clues.

- If something goes wrong with the unwinder, or if frame pointers are corrupt or missing, all the stack addresses still get shown.

So in order to show all addresses on the stack, and at the same time figure out which addresses are reliable, we have to do the scanning and the unwinding in parallel.

The scanning is done with the help of get_stack_info() to traverse the stacks. The unwinding is done separately by the new unwinder.

In theory we could simplify show_trace_log_lvl() by instead pushing some of this logic into the unwind code. But then we would need some kind of "fake" frame logic in the unwinder which would add a lot of complexity and wouldn't be worth it in order to support only one user.

Another benefit of this approach is that once we have a DWARF unwinder, we should be able to just plug it in with minimal impact to this code.

Another change here is that callers of show_trace_log_lvl() don't need to provide the 'bp' argument. The unwinder already finds the relevant frame pointer by unwinding until it reaches the first frame after the provided stack pointer.

Signed-off-by Josh Poimboeuf <[email protected]>

---
 arch/x86/include/asm/stacktrace.h |  10 ++-
 arch/x86/kernel/dumpstack.c       | 130 +++++++++++++++++++++++++++++---------
 arch/x86/kernel/dumpstack_32.c    |   9 ++-
 arch/x86/kernel/dumpstack_64.c    |   9 ++-
 4 files changed, 111 insertions(+), 47 deletions(-)

arch/x86/include/asm/stacktrace.h +4 -6

index 780a83e..1f21b48 100644
@@ -119,13 +119,11 @@ get_stack_pointer(struct task_struct *task, struct pt_regs *regs)
 	return (unsigned long *)task->thread.sp;
 }
 
-extern void
-show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
-		   unsigned long *stack, unsigned long bp, char *log_lvl);
+void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
+			unsigned long *stack, char *log_lvl);
 
-extern void
-show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
-		   unsigned long *sp, unsigned long bp, char *log_lvl);
+void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
+			unsigned long *sp, char *log_lvl);
 
 extern unsigned int code_bytes;

arch/x86/kernel/dumpstack.c +99 ~~-31~~

160 lines not on this page · raw message

arch/x86/kernel/dumpstack_32.c +4 -5

index 2d65cfa..246d7f6 100644
@@ -156,9 +156,8 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
 }
 EXPORT_SYMBOL(dump_trace);
 
-void
-show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
-		   unsigned long *sp, unsigned long bp, char *log_lvl)
+void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
+			unsigned long *sp, char *log_lvl)
 {
 	unsigned long *stack;
 	int i;

@@ -178,7 +177,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
 		touch_nmi_watchdog();
 	}
 	pr_cont("\n");
-	show_trace_log_lvl(task, regs, sp, bp, log_lvl);
+	show_trace_log_lvl(task, regs, sp, log_lvl);
 }

@@ -200,7 +199,7 @@ void show_regs(struct pt_regs *regs)
 		u8 *ip;
 
 		pr_emerg("Stack:\n");
-		show_stack_log_lvl(NULL, regs, NULL, 0, KERN_EMERG);
+		show_stack_log_lvl(NULL, regs, NULL, KERN_EMERG);
 
 		pr_emerg("Code:");

arch/x86/kernel/dumpstack_64.c +4 -5

index 8cb6004..922aeac 100644
@@ -209,9 +209,8 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
 }
 EXPORT_SYMBOL(dump_trace);
 
-void
-show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
-		   unsigned long *sp, unsigned long bp, char *log_lvl)
+void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
+			unsigned long *sp, char *log_lvl)
 {
 	unsigned long *irq_stack_end;
 	unsigned long *irq_stack;

@@ -252,7 +251,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
 	}
 
 	pr_cont("\n");
-	show_trace_log_lvl(task, regs, sp, bp, log_lvl);
+	show_trace_log_lvl(task, regs, sp, log_lvl);
 }
 
 void show_regs(struct pt_regs *regs)

@@ -273,7 +272,7 @@ void show_regs(struct pt_regs *regs)
 		u8 *ip;
 
 		printk(KERN_DEFAULT "Stack:\n");
-		show_stack_log_lvl(NULL, regs, NULL, 0, KERN_DEFAULT);
+		show_stack_log_lvl(NULL, regs, NULL, KERN_DEFAULT);
 
 		printk(KERN_DEFAULT "Code: ");

Signature · 1 line

2.7.4

Josh Poimboeuf

author

2016-09-15 15:56 UTC #6 lore raw

PATCH 6/6 x86/dumpstack: remove dump_trace() and related callbacks

In reply to Josh Poimboeuf

All previous users of dump_trace() have been converted to use the new unwind interfaces, so we can remove it and the related print_context_stack() and print_context_stack_bp() callback functions.

Signed-off-by Josh Poimboeuf <[email protected]>

---
 arch/x86/include/asm/stacktrace.h | 36 ----------------
 arch/x86/kernel/dumpstack.c       | 86 ---------------------------------------
 arch/x86/kernel/dumpstack_32.c    | 35 ----------------
 arch/x86/kernel/dumpstack_64.c    | 69 -------------------------------
 4 files changed, 226 deletions(-)

arch/x86/include/asm/stacktrace.h +0 ~~-36~~

index 1f21b48..c7797fe 100644
@@ -45,42 +45,6 @@ static inline bool on_stack(struct stack_info *info, void *addr, size_t len)
 
 extern int kstack_depth_to_print;
 
-struct thread_info;
-struct stacktrace_ops;
-
-typedef unsigned long (*walk_stack_t)(struct task_struct *task,
-				      unsigned long *stack,
-				      unsigned long bp,
-				      const struct stacktrace_ops *ops,
-				      void *data,
-				      struct stack_info *info,
-				      int *graph);
-
-extern unsigned long
-print_context_stack(struct task_struct *task,
-		    unsigned long *stack, unsigned long bp,
-		    const struct stacktrace_ops *ops, void *data,
-		    struct stack_info *info, int *graph);
-
-extern unsigned long
-print_context_stack_bp(struct task_struct *task,
-		       unsigned long *stack, unsigned long bp,
-		       const struct stacktrace_ops *ops, void *data,
-		       struct stack_info *info, int *graph);
-
-/* Generic stack tracer with callbacks */
-
-struct stacktrace_ops {
-	int (*address)(void *data, unsigned long address, int reliable);
-	/* On negative return stop dumping */
-	int (*stack)(void *data, const char *name);
-	walk_stack_t	walk_stack;
-};
-
-void dump_trace(struct task_struct *tsk, struct pt_regs *regs,
-		unsigned long *stack, unsigned long bp,
-		const struct stacktrace_ops *ops, void *data);
-
 #ifdef CONFIG_X86_32
 #define STACKSLOTS_PER_LINE 8
 #else

arch/x86/kernel/dumpstack.c +0 ~~-86~~

92 lines not on this page · raw message

arch/x86/kernel/dumpstack_32.c +0 ~~-35~~

index 246d7f6..c98c79a 100644
@@ -121,41 +121,6 @@ unknown:
 	return -EINVAL;
 }
 
-void dump_trace(struct task_struct *task, struct pt_regs *regs,
-		unsigned long *stack, unsigned long bp,
-		const struct stacktrace_ops *ops, void *data)
-{
-	unsigned long visit_mask = 0;
-	int graph = 0;
-
-	task = task ? : current;
-	stack = stack ? : get_stack_pointer(task, regs);
-	bp = bp ? : (unsigned long)get_frame_pointer(task, regs);
-
-	for (;;) {
-		const char *begin_str, *end_str;
-		struct stack_info info;
-
-		if (get_stack_info(stack, task, &info, &visit_mask))
-			break;
-
-		stack_type_str(info.type, &begin_str, &end_str);
-
-		if (begin_str && ops->stack(data, begin_str) < 0)
-			break;
-
-		bp = ops->walk_stack(task, stack, bp, ops, data, &info, &graph);
-
-		if (end_str && ops->stack(data, end_str) < 0)
-			break;
-
-		stack = info.next_sp;
-
-		touch_nmi_watchdog();
-	}
-}
-EXPORT_SYMBOL(dump_trace);
-
 void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
 			unsigned long *sp, char *log_lvl)
 {

arch/x86/kernel/dumpstack_64.c +0 ~~-69~~

75 lines not on this page · raw message

Signature · 1 line

2.7.4

Ingo Molnar

2016-09-16 09:16 UTC #7 lore raw

In reply to Josh Poimboeuf

Quoted 18 lines · Josh Poimboeuf

The last version was posted as part of a much larger patch set:
https://lkml.kernel.org/r/[email protected]
People complained about the set being too big, so it was split up into more digestible pieces. All the prerequisite patches for the unwinder have now been merged.
See patch 1/6 for the background and justification for this change.
Josh Poimboeuf (6):
x86/unwind: add new unwind interface and implementations perf/x86: convert perf_callchain_kernel() to use the new unwinder x86/stacktrace: convert save_stack_trace_*() to use the new unwinder oprofile/x86: convert x86_backtrace() to use the new unwinder x86/dumpstack: convert show_trace_log_lvl() to use the new unwinder x86/dumpstack: remove dump_trace() and related callbacks

Quoted 15 lines

 arch/x86/events/core.c            |  33 +++-----
 arch/x86/include/asm/stacktrace.h |  46 +---------
 arch/x86/include/asm/unwind.h     |  74 ++++++++++++++++
 arch/x86/kernel/Makefile          |   6 ++
 arch/x86/kernel/dumpstack.c       | 174 +++++++++++++++++---------------------
 arch/x86/kernel/dumpstack_32.c    |  44 +---------
 arch/x86/kernel/dumpstack_64.c    |  78 +----------------
 arch/x86/kernel/stacktrace.c      |  74 +++++++---------
 arch/x86/kernel/unwind_frame.c    |  93 ++++++++++++++++++++
 arch/x86/kernel/unwind_guess.c    |  43 ++++++++++
 arch/x86/oprofile/backtrace.c     |  39 ++++-----
 11 files changed, 362 insertions(+), 342 deletions(-)
 create mode 100644 arch/x86/include/asm/unwind.h
 create mode 100644 arch/x86/kernel/unwind_frame.c
 create mode 100644 arch/x86/kernel/unwind_guess.c

Looks good to me, but could you please rebase it on top of very latestest -tip, which has these bits from Andy included:

1959a60182f4 x86/dumpstack: Pin the target stack when dumping it cb76c9398240 x86/dumpstack: Add get_stack_info() interface

... which create new conflicts with your series.

Thanks,

Ingo