All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@elte.hu>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	"H. Peter Anvin" <hpa@zytor.com>, Brian Gerst <brgerst@gmail.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: [PATCH 4/4 v2] x86: Clean up dumpstack_64.c code
Date: Fri, 05 Aug 2011 21:11:06 -0400	[thread overview]
Message-ID: <20110806012354.684598995@goodmis.org> (raw)
In-Reply-To: 20110806011102.721109343@goodmis.org

[-- Attachment #1: 0004-x86-Clean-up-dumpstack_64.c-code.patch --]
[-- Type: text/plain, Size: 5089 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

The dump_trace() function in dumpstack_64.c is hard to follow.
The test for exception stack is processed differently than the
test for irq stack, and the normal stack is outside completely.

By restructuring this code to have all the stacks determined by
a single function that returns an enum of the following:

 STACK_IS_NORMAL
 STACK_IS_EXCEPTION
 STACK_IS_IRQ
 STACK_IS_UNKNOWN

and has the logic of each within a switch statement.
This should make the code much easier to read and understand.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/kernel/dumpstack_64.c |  121 ++++++++++++++++++++++++++++------------
 1 files changed, 85 insertions(+), 36 deletions(-)

diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index e71c98d..d0b2ace 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -132,6 +132,45 @@ fixup_bp_irq_link(unsigned long bp, unsigned long *stack,
 	return bp;
 }
 
+static const unsigned long irq_stack_size =
+	(IRQ_STACK_SIZE - 64) / sizeof(unsigned long);
+
+enum stack_type {
+	STACK_IS_UNKNOWN,
+	STACK_IS_NORMAL,
+	STACK_IS_EXCEPTION,
+	STACK_IS_IRQ,
+};
+
+static enum stack_type
+analyze_stack(int cpu, struct task_struct *task,
+	      unsigned long *stack, unsigned long **stack_end, char **id)
+{
+	unsigned long *irq_stack;
+	unsigned long addr;
+	unsigned used = 0;
+
+	addr = ((unsigned long)stack & (~(THREAD_SIZE - 1)));
+	if ((unsigned long)task_stack_page(task) == addr)
+		return STACK_IS_NORMAL;
+
+	*stack_end = in_exception_stack(cpu, (unsigned long)stack,
+					 &used, id);
+	if (*stack_end)
+		return STACK_IS_EXCEPTION;
+
+	*stack_end = (unsigned long *)per_cpu(irq_stack_ptr, cpu);
+	if (!*stack_end)
+		return STACK_IS_UNKNOWN;
+
+	irq_stack = *stack_end - irq_stack_size;
+
+	if (in_irq_stack(stack, irq_stack, *stack_end))
+		return STACK_IS_IRQ;
+
+	return STACK_IS_UNKNOWN;
+}
+
 /*
  * x86-64 can have up to three kernel stacks:
  * process stack
@@ -144,12 +183,11 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
 		const struct stacktrace_ops *ops, void *data)
 {
 	const unsigned cpu = get_cpu();
-	unsigned long *irq_stack_end =
-		(unsigned long *)per_cpu(irq_stack_ptr, cpu);
-	unsigned used = 0;
 	struct thread_info *tinfo;
-	int graph = 0;
+	unsigned long *irq_stack;
 	unsigned long dummy;
+	int graph = 0;
+	int done = 0;
 
 	if (!task)
 		task = current;
@@ -168,51 +206,62 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs,
 	 * exceptions
 	 */
 	tinfo = task_thread_info(task);
-	for (;;) {
+	while (!done) {
+		unsigned long *stack_end;
+		enum stack_type stype;
 		char *id;
-		unsigned long *estack_end;
-		estack_end = in_exception_stack(cpu, (unsigned long)stack,
-						&used, &id);
 
-		if (estack_end) {
+		stype = analyze_stack(cpu, task, stack, &stack_end, &id);
+
+		/* Default finish unless specified to continue */
+		done = 1;
+
+		switch (stype) {
+
+		/* Break out early if we are on the thread stack */
+		case STACK_IS_NORMAL:
+			break;
+
+		case STACK_IS_EXCEPTION:
+
 			if (ops->stack(data, id) < 0)
 				break;
 
 			bp = ops->walk_stack(tinfo, stack, bp, ops,
-					     data, estack_end, &graph);
+					     data, stack_end, &graph);
 			ops->stack(data, "<EOE>");
 			/*
 			 * We link to the next stack via the
 			 * second-to-last pointer (index -2 to end) in the
 			 * exception stack:
 			 */
-			stack = (unsigned long *) estack_end[-2];
-			continue;
-		}
-		if (irq_stack_end) {
-			unsigned long *irq_stack;
-			irq_stack = irq_stack_end -
-				(IRQ_STACK_SIZE - 64) / sizeof(*irq_stack);
-
-			if (in_irq_stack(stack, irq_stack, irq_stack_end)) {
-				if (ops->stack(data, "IRQ") < 0)
-					break;
-				bp = ops->walk_stack(tinfo, stack, bp,
-					ops, data, irq_stack_end, &graph);
-				/*
-				 * We link to the next stack (which would be
-				 * the process stack normally) the last
-				 * pointer (index -1 to end) in the IRQ stack:
-				 */
-				stack = (unsigned long *) (irq_stack_end[-1]);
-				bp = fixup_bp_irq_link(bp, stack, irq_stack,
-						       irq_stack_end);
-				irq_stack_end = NULL;
-				ops->stack(data, "EOI");
-				continue;
-			}
+			stack = (unsigned long *) stack_end[-2];
+			done = 0;
+			break;
+
+		case STACK_IS_IRQ:
+
+			if (ops->stack(data, "IRQ") < 0)
+				break;
+			bp = ops->walk_stack(tinfo, stack, bp,
+				     ops, data, stack_end, &graph);
+			/*
+			 * We link to the next stack (which would be
+			 * the process stack normally) the last
+			 * pointer (index -1 to end) in the IRQ stack:
+			 */
+			stack = (unsigned long *) (stack_end[-1]);
+			irq_stack = stack_end - irq_stack_size;
+			bp = fixup_bp_irq_link(bp, stack, irq_stack,
+					       stack_end);
+			ops->stack(data, "EOI");
+			done = 0;
+			break;
+
+		case STACK_IS_UNKNOWN:
+			ops->stack(data, "UNK");
+			break;
 		}
-		break;
 	}
 
 	/*
-- 
1.7.5.4



  parent reply	other threads:[~2011-08-06  1:24 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-06  1:11 [PATCH 0/4 v2] [GIT PULL][v3.2] x86: Consolidate 32 and 64 bit thread_info Steven Rostedt
2011-08-06  1:11 ` [PATCH 1/4 v2] x86: Nuke the supervisor_stack field in i386 thread_info Steven Rostedt
2011-08-06  1:11 ` [PATCH 2/4 v2] x86: Remove previous_esp from i386 thread_info structure Steven Rostedt
2011-08-06  1:11 ` [PATCH 3/4 v2] x86: Keep thread_info on thread stack in x86_32 Steven Rostedt
2011-08-06  1:11 ` Steven Rostedt [this message]
2011-08-06  2:04 ` [PATCH 0/4 v2] [GIT PULL][v3.2] x86: Consolidate 32 and 64 bit thread_info H. Peter Anvin
2011-08-08 21:23   ` Steven Rostedt
2011-08-19  2:01   ` Steven Rostedt
2011-08-19  5:17     ` H. Peter Anvin
2014-02-06 14:41 [PATCH 0/5] [GIT PULL] x86: Consolidate 32 and 64 bit thead_info (again) Steven Rostedt
2014-02-06 14:41 ` [PATCH 1/5] x86: Nuke the supervisor_stack field in i386 thread_info Steven Rostedt
2014-03-07  1:00   ` [tip:x86/threadinfo] " tip-bot for Steven Rostedt
2014-02-06 14:41 ` [PATCH 2/5] x86: Nuke GET_THREAD_INFO_WITH_ESP() macro for i386 Steven Rostedt
2014-03-07  1:00   ` [tip:x86/threadinfo] " tip-bot for Steven Rostedt (Red Hat)
2014-02-06 14:41 ` [PATCH 3/5] x86: Prepare removal of previous_esp from i386 thread_info structure Steven Rostedt
2014-03-07  1:01   ` [tip:x86/threadinfo] " tip-bot for Steven Rostedt
2014-02-06 14:41 ` [PATCH 4/5] x86: Keep thread_info on thread stack in x86_32 Steven Rostedt
2014-03-07  1:01   ` [tip:x86/threadinfo] " tip-bot for Steven Rostedt
2014-03-07  7:52   ` [PATCH] x86: redo "x86: Use inline assembler to get sp" Mathias Krause
2014-03-11  0:36     ` [tip:x86/threadinfo] x86, threadinfo: Redo " tip-bot for Mathias Krause
2014-02-06 14:41 ` [PATCH 5/5] x86: Clean up dumpstack_64.c code Steven Rostedt
2014-03-07  1:01   ` [tip:x86/threadinfo] " tip-bot for Steven Rostedt
2014-02-28 13:52 ` [PATCH 0/5] [GIT PULL] x86: Consolidate 32 and 64 bit thead_info (again) Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110806012354.684598995@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=brgerst@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.