[no subject]

From: Tejun Heo
Date: Wed Apr 10 2013 - 16:06:06 EST


When dumping stacktrace, frames of the stacktrace code itself aren't
interesting. Implement dump_trace_current_frame() helper which can be
used by (eventual) users of dump_trace() to determine the stack and
frame pointers of the current frame so that frames beyond that point
are ignored by dump_trace().

show_stack() and save_stack_trace[_tsk]() are updated to use the
helper and now ignore frames beyond their own. This brings
show_stack(NULL, NULL)'s behavior in line with dump_stack().

Also add dump_trace_warn_current_frame() which is used by dump_trace()
implementations to whine about callers which dump %current but don't
specify the frame to dump to catch mistakes.

The original patch just updated show_stack(). Applying it to other
users and addition of warning are suggested by Ingo.

Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
---
save_stack_trace*() were specifying NULL %bp as well. Introduced a
helper and make both dump_stack() and save_stack_trace*() use it and
added a warning message in dump_trace().

git branch also updated. This change doesn't affect other patches in
the series other than minor offset changes.

Thanks.

arch/x86/include/asm/stacktrace.h | 35 +++++++++++++++++++++++++++++++++++
arch/x86/kernel/dumpstack.c | 11 ++++++++++-
arch/x86/kernel/dumpstack_32.c | 2 ++
arch/x86/kernel/dumpstack_64.c | 2 ++
arch/x86/kernel/stacktrace.c | 12 ++++++++++--
5 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h
index 70bbe39..1abe898 100644
--- a/arch/x86/include/asm/stacktrace.h
+++ b/arch/x86/include/asm/stacktrace.h
@@ -73,14 +73,49 @@ stack_frame(struct task_struct *task, struct pt_regs *regs)
/* bp is the last reg pushed by switch_to */
return *(unsigned long *)task->thread.sp;
}
+
+/* sanity check helper for dump_trace(), see dump_trace_current_frame() */
+static inline void
+dump_trace_warn_current_frame(struct task_struct *task, struct pt_regs *regs,
+ unsigned long bp)
+{
+ if ((!task || task == current) && !regs && !bp)
+ printk(KERN_WARNING "dump_trace: %pf didn't specify neither frame nor regs for %%current\n",
+ __builtin_return_address(0));
+}
#else
static inline unsigned long
stack_frame(struct task_struct *task, struct pt_regs *regs)
{
return 0;
}
+
+static inline void
+dump_trace_warn_current_frame(struct task_struct *task, struct pt_regs *regs,
+ unsigned long bp)
+{ }
#endif

+/**
+ * dump_trace_current_frame - stack and frame pointers for the current frame
+ * @sp: output argument for the stack pointer
+ * @bp: output argument for the frame pointer
+ *
+ * When dumping %current, dump_trace() wants its caller to specify the top
+ * frame so that it doesn't end up showing unncessary traces into stack
+ * dumping machinery. This helper can be used to determine @sp and @bp to
+ * pass to dump_trace() when dumping %current. This functions is
+ * __always_inline so that it records the frame of the caller.
+ */
+static __always_inline
+void dump_trace_current_frame(unsigned long **sp, unsigned long *bp)
+{
+ unsigned long stack;
+
+ *sp = &stack;
+ *bp = stack_frame(current, NULL);
+}
+
extern void
show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack, unsigned long bp, char *log_lvl);
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index c8797d5..6e5e3ab 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -176,7 +176,16 @@ void show_trace(struct task_struct *task, struct pt_regs *regs,

void show_stack(struct task_struct *task, unsigned long *sp)
{
- show_stack_log_lvl(task, NULL, sp, 0, "");
+ unsigned long bp = 0;
+
+ /*
+ * Stack frames below this one aren't interesting. Don't show them
+ * if we're printing for %current.
+ */
+ if (!sp && (!task || task == current))
+ dump_trace_current_frame(&sp, &bp);
+
+ show_stack_log_lvl(task, NULL, sp, bp, "");
}

/*
diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
index 1038a41..8cc7c4a 100644
--- a/arch/x86/kernel/dumpstack_32.c
+++ b/arch/x86/kernel/dumpstack_32.c
@@ -23,6 +23,8 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
{
int graph = 0;

+ dump_trace_warn_current_frame(task, regs, bp);
+
if (!task)
task = current;

diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index b653675..349a8a9 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -123,6 +123,8 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
int graph = 0;
unsigned long dummy;

+ dump_trace_warn_current_frame(task, regs, bp);
+
if (!task)
task = current;

diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index fdd0c64..0908a50 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -60,7 +60,10 @@ static const struct stacktrace_ops save_stack_ops_nosched = {
*/
void save_stack_trace(struct stack_trace *trace)
{
- dump_trace(current, NULL, NULL, 0, &save_stack_ops, trace);
+ unsigned long *sp, bp;
+
+ dump_trace_current_frame(&sp, &bp);
+ dump_trace(current, NULL, sp, bp, &save_stack_ops, trace);
if (trace->nr_entries < trace->max_entries)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
@@ -75,7 +78,12 @@ void save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)

void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
{
- dump_trace(tsk, NULL, NULL, 0, &save_stack_ops_nosched, trace);
+ unsigned long *sp = NULL, bp = 0;
+
+ if (!tsk || tsk == current)
+ dump_trace_current_frame(&sp, &bp);
+
+ dump_trace(tsk, NULL, sp, bp, &save_stack_ops_nosched, trace);
if (trace->nr_entries < trace->max_entries)
trace->entries[trace->nr_entries++] = ULONG_MAX;
}
--
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/