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

PATCH New Josh Poimboeuf 2 participants 15 messages Sep 16, 2016 – Sep 20, 2016

Subsystems (3)

x86 perf events x86 stack unwinding

lore

Share & tools

lore

Josh Poimboeuf

author

2016-09-16 19:18 UTC #0 lore raw

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

v2:
- rebase on latest tip
- remove NULL task pointer convention

---

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     |  73 ++++++++++++++++
 arch/x86/kernel/Makefile          |   6 ++
 arch/x86/kernel/dumpstack.c       | 170 +++++++++++++++++---------------------
 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, 357 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-16 19:18 UTC #1 lore raw

PATCH v2 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  | 73 +++++++++++++++++++++++++++++++++
 arch/x86/kernel/Makefile       |  6 +++
 arch/x86/kernel/unwind_frame.c | 93 ++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/unwind_guess.c | 43 +++++++++++++++++++
 4 files changed, 215 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 +73 -0

new file mode 100644
index 0000000..c4b6d1c
@@ -0,0 +1,73 @@
+#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)
+{
+	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

tip-bot for Josh Poimboeuf

merge bot

2016-09-20 15:01 UTC #2 lore raw

[tip:x86/asm] x86/unwind: Add new unwind interface and implementations

x86/unwind: Add new unwind interface and implementations

Commit-ID:  7c7900f89770d7fba96100d8a9e18043a1af3973
Gitweb:     http://git.kernel.org/tip/7c7900f89770d7fba96100d8a9e18043a1af3973
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Fri, 16 Sep 2016 14:18:12 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 20 Sep 2016 08:29:33 +0200

x86/unwind: Add new unwind interface and implementations

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.

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

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brian Gerst <[email protected]>

Cc Byungchul Park <[email protected]>

Cc Denys Vlasenko <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Kees Cook <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Nilay Vaish <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/6dc2f909c47533d213d0505f0a113e64585bec82.1474045023.git.jpoimboe@redhat.com open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/include/asm/unwind.h  | 73 +++++++++++++++++++++++++++++++++
 arch/x86/kernel/Makefile       |  6 +++
 arch/x86/kernel/unwind_frame.c | 93 ++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/unwind_guess.c | 43 +++++++++++++++++++
 4 files changed, 215 insertions(+)

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

new file mode 100644
index 0000000..c4b6d1c
@@ -0,0 +1,73 @@
+#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)
+{
+	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

Josh Poimboeuf

author

2016-09-16 19:19 UTC #3 lore raw

PATCH v2 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..d31735f 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, current, 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

tip-bot for Josh Poimboeuf

merge bot

2016-09-20 15:01 UTC #4 lore raw

[tip:x86/asm] perf/x86: Convert perf_callchain_kernel() to use the new unwinder

perf/x86: Convert perf_callchain_kernel() to use the new unwinder

Commit-ID:  35f4d9b32527c08c3da3982aedae5198dc663ce8
Gitweb:     http://git.kernel.org/tip/35f4d9b32527c08c3da3982aedae5198dc663ce8
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Fri, 16 Sep 2016 14:18:13 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 20 Sep 2016 08:29:33 +0200

perf/x86: Convert perf_callchain_kernel() to use the new unwinder

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

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

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brian Gerst <[email protected]>

Cc Byungchul Park <[email protected]>

Cc Denys Vlasenko <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Kees Cook <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Nilay Vaish <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/a2df0c4f09b3d438e11b41681f10b0775a819a7f.1474045023.git.jpoimboe@redhat.com open here

Signed-off-by Ingo Molnar <[email protected]>

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

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

index 477dc38..0a8bd7f 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"

@@ -2247,31 +2248,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;

@@ -2280,7 +2262,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, current, 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

Josh Poimboeuf

author

2016-09-16 19:19 UTC #5 lore raw

PATCH v2 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 23fa81e..0653788 100644
@@ -8,73 +8,59 @@
 #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, current, 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, current, regs, false);
 }
 
 void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)

@@ -82,9 +68,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 	if (!try_get_task_stack(tsk))
 		return;
 
-	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);
 
 	put_task_stack(tsk);
 }

Signature · 1 line

2.7.4

tip-bot for Josh Poimboeuf

merge bot

2016-09-20 15:02 UTC #6 lore raw

[tip:x86/asm] x86/stacktrace: Convert save_stack_trace_*() to use the new unwinder

x86/stacktrace: Convert save_stack_trace_*() to use the new unwinder

Commit-ID:  49a612c6b06defbd6e6d334c683fea28006728e3
Gitweb:     http://git.kernel.org/tip/49a612c6b06defbd6e6d334c683fea28006728e3
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Fri, 16 Sep 2016 14:18:14 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 20 Sep 2016 08:29:33 +0200

x86/stacktrace: Convert save_stack_trace_*() to use the new unwinder

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

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

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brian Gerst <[email protected]>

Cc Byungchul Park <[email protected]>

Cc Denys Vlasenko <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Kees Cook <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Nilay Vaish <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/815494c627d89887db0ce56ceffd58ad16ee6c21.1474045023.git.jpoimboe@redhat.com open here

Signed-off-by Ingo Molnar <[email protected]>

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

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

index 23fa81e..0653788 100644
@@ -8,73 +8,59 @@
 #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, current, 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, current, regs, false);
 }
 
 void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)

@@ -82,9 +68,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 	if (!try_get_task_stack(tsk))
 		return;
 
-	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);
 
 	put_task_stack(tsk);
 }

Josh Poimboeuf

author

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

PATCH v2 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..a2488b6e 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, current, 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

tip-bot for Josh Poimboeuf

merge bot

2016-09-20 15:02 UTC #8 lore raw

[tip:x86/asm] oprofile/x86: Convert x86_backtrace() to use the new unwinder

oprofile/x86: Convert x86_backtrace() to use the new unwinder

Commit-ID:  ec2ad9ccf12dc965cad2d367a4063f68d6561a6b
Gitweb:     http://git.kernel.org/tip/ec2ad9ccf12dc965cad2d367a4063f68d6561a6b
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Fri, 16 Sep 2016 14:18:15 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 20 Sep 2016 08:29:34 +0200

oprofile/x86: Convert x86_backtrace() to use the new unwinder

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

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

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brian Gerst <[email protected]>

Cc Byungchul Park <[email protected]>

Cc Denys Vlasenko <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Kees Cook <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Nilay Vaish <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Robert Richter <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/412df8927705795e8ea60cffcf89a79e010713b1.1474045023.git.jpoimboe@redhat.com open here

Signed-off-by Ingo Molnar <[email protected]>

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

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

index 7539148..a2488b6 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, current, 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;
 	}

Josh Poimboeuf

author

2016-09-16 19:19 UTC #9 lore raw

PATCH v2 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       | 126 ++++++++++++++++++++++++++++----------
 arch/x86/kernel/dumpstack_32.c    |   9 ++-
 arch/x86/kernel/dumpstack_64.c    |   9 ++-
 4 files changed, 107 insertions(+), 47 deletions(-)

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

index ed2be1b..c9ccf06 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 +95 ~~-31~~

158 lines not on this page · raw message

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

index 4ff0008..e476eb7 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;

@@ -181,7 +180,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);
 
 	put_task_stack(task);
 }

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

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

index 008a298..4e9f2cf 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;

@@ -255,7 +254,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);
 
 	put_task_stack(task);
 }

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

Signature · 1 line

2.7.4

tip-bot for Josh Poimboeuf

merge bot

2016-09-20 15:02 UTC #10 lore raw

[tip:x86/asm] x86/dumpstack: Convert show_trace_log_lvl() to use the new unwinder

x86/dumpstack: Convert show_trace_log_lvl() to use the new unwinder

Commit-ID:  e18bcccd1a4ecb41e99678e002ef833586185bf1
Gitweb:     http://git.kernel.org/tip/e18bcccd1a4ecb41e99678e002ef833586185bf1
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Fri, 16 Sep 2016 14:18:16 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 20 Sep 2016 08:29:34 +0200

x86/dumpstack: Convert show_trace_log_lvl() to use the new unwinder

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

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

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.

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

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brian Gerst <[email protected]>

Cc Byungchul Park <[email protected]>

Cc Denys Vlasenko <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Kees Cook <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Nilay Vaish <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/703b5998604c712a1f801874b43f35d6dac52ede.1474045023.git.jpoimboe@redhat.com open here

Signed-off-by Ingo Molnar <[email protected]>

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

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

index ed2be1b..c9ccf06 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 +95 ~~-31~~

158 lines not on this page · raw message

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

index 4ff0008..e476eb7 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;

@@ -181,7 +180,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);
 
 	put_task_stack(task);
 }

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

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

index 008a298..4e9f2cf 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;

@@ -255,7 +254,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);
 
 	put_task_stack(task);
 }

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

Josh Poimboeuf

author

2016-09-20 15:53 UTC #11 lore raw

PATCH 1/1 x86/dumpstack: fix show_stack() task pointer regression

Quoted 8 lines · tip-bot for Josh Poimboeuf

Commit-ID:  e18bcccd1a4ecb41e99678e002ef833586185bf1
Gitweb:     http://git.kernel.org/tip/e18bcccd1a4ecb41e99678e002ef833586185bf1
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Fri, 16 Sep 2016 14:18:16 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 20 Sep 2016 08:29:34 +0200

x86/dumpstack: Convert show_trace_log_lvl() to use the new unwinder

Ingo,

My apologies, I introduced a small regression with this patch -- another rebase gone bad. Here's the fix:

---

From: Josh Poimboeuf <[email protected]>
Subject: [PATCH] x86/dumpstack: fix show_stack() task pointer regression

With the following commit:

e18bcccd1a4e ("x86/dumpstack: Convert show_trace_log_lvl() to use the new unwinder")

The task pointer argument to show_stack_log_lvl() in show_stack() was inadvertently changed to 'current'.

Fixes e18bcccd1a4e ("x86/dumpstack: Convert show_trace_log_lvl() to use the new unwinder")

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

---
 arch/x86/kernel/dumpstack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/kernel/dumpstack.c +1 -1

index 999de3b..9b7cf5c 100644
@@ -164,7 +164,7 @@ void show_stack(struct task_struct *task, unsigned long *sp)
 	if (!sp && task == current)
 		sp = get_stack_pointer(current, NULL);
 
-	show_stack_log_lvl(current, NULL, sp, "");
+	show_stack_log_lvl(task, NULL, sp, "");
 }
 
 void show_stack_regs(struct pt_regs *regs)

Signature · 1 line

2.7.4

tip-bot for Josh Poimboeuf

merge bot

2016-09-20 21:50 UTC #12 lore raw

[tip:x86/asm] x86/dumpstack: Fix show_stack() task pointer regression

x86/dumpstack: Fix show_stack() task pointer regression

Commit-ID:  71f5443ebb1227c22e8decbcd28a1ea6deaf8257
Gitweb:     http://git.kernel.org/tip/71f5443ebb1227c22e8decbcd28a1ea6deaf8257
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Tue, 20 Sep 2016 10:53:40 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 20 Sep 2016 23:36:37 +0200

x86/dumpstack: Fix show_stack() task pointer regression

With the following commit:

e18bcccd1a4e ("x86/dumpstack: Convert show_trace_log_lvl() to use the new unwinder")

The task pointer argument to show_stack_log_lvl() in show_stack() was inadvertently changed to 'current'.

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

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brian Gerst <[email protected]>

Cc Denys Vlasenko <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc [email protected]

Cc tip-bot for Josh Poimboeuf <[email protected]>

Fixes e18bcccd1a4e ("x86/dumpstack: Convert show_trace_log_lvl() to use the new unwinder")

Link http://lkml.kernel.org/r/20160920155340.yhewlx7vmgmov5fb@treble open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/kernel/dumpstack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/kernel/dumpstack.c +1 -1

index 999de3b..9b7cf5c 100644
@@ -164,7 +164,7 @@ void show_stack(struct task_struct *task, unsigned long *sp)
 	if (!sp && task == current)
 		sp = get_stack_pointer(current, NULL);
 
-	show_stack_log_lvl(current, NULL, sp, "");
+	show_stack_log_lvl(task, NULL, sp, "");
 }
 
 void show_stack_regs(struct pt_regs *regs)

Josh Poimboeuf

author

2016-09-16 19:19 UTC #13 lore raw

PATCH v2 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 c9ccf06..37f2e0b 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 e476eb7..06eb322 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

tip-bot for Josh Poimboeuf

merge bot

2016-09-20 15:03 UTC #14 lore raw

[tip:x86/asm] x86/dumpstack: Remove dump_trace() and related callbacks

x86/dumpstack: Remove dump_trace() and related callbacks

Commit-ID:  c8fe4609827aedc9c4b45de80e7cdc8ccfa8541b
Gitweb:     http://git.kernel.org/tip/c8fe4609827aedc9c4b45de80e7cdc8ccfa8541b
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Fri, 16 Sep 2016 14:18:17 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 20 Sep 2016 08:29:34 +0200

x86/dumpstack: Remove dump_trace() and related callbacks

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

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brian Gerst <[email protected]>

Cc Byungchul Park <[email protected]>

Cc Denys Vlasenko <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Kees Cook <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Nilay Vaish <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/5b97da3572b40b5a4d8e185cf2429308d0987a13.1474045023.git.jpoimboe@redhat.com open here

Signed-off-by Ingo Molnar <[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 c9ccf06..37f2e0b 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 e476eb7..06eb322 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