stack: only print stack usage backtraces when we hit a new watermark

With DEBUG=1 builds we use the mcount hook to instrument how much stack
space we're using. If we detect that a function call has come within
2KB of the bottom of the stack we currently print a backtrace. This can
result in a huge amount of console IO in DEBUG=1 builds which can cause
op-test timeouts, etc.

Printing a backtrace on each function call isn't terribly useful, and it
ends up crowding out the backtrace that's printed when we hit a new
stack usage watermark. The watermark should provide enough information
to find and fix excessive stack usage issues so drop the per-function
backtrace printing and move the warning into the high-watermark check.

This change is largely necessary because of DEBUG=1 expands adds a
backtrace save area to struct lock which expands the size of it to nearly
2KB. struct cpu_thread (which lives at the bottom of the per-thread stacks)
contains three locks and an additional backtrace save area which is
enabled when DEBUG=1. The extra space requirements result in cpu_thread
ballooning from ~420 bytes to nearly 8KB.

Any growth in cpu_thread also results in less stack space being
available for the thread, so when DEBUG=1 is enabled we go from having
a 16KB stack to an 8KB stack. Although this seems large, skiboot does
have some fairly deep call chains (UART console flushing, TPM drivers,
both combined) which can cause the thread to come within 2KB of the stack
use warning zone.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
Maybe we should swap locations of the normal and emergency stacks so
cpu_thread takes space from the emergency stack rather than the normal one.
The e-stack should only be used at runtime where the call chains should be
smaller.

1 files changed, 4 insertions, 4 deletions

diff --git a/core/stack.c b/core/stack.c
index 2df960b..05506f6 100644
--- a/core/stack.c
+++ b/core/stack.c
@@ -206,16 +206,16 @@ void __nomcount __mcount_stack_check(uint64_t sp, uint64_t lr)
 		backtrace_create(c->stack_bot_bt, CPU_BACKTRACE_SIZE,
 				 &c->stack_bot_bt_metadata);
 		unlock(&stack_check_lock);
-	}
 
-	/* Stack is within bounds ? check for warning and bail */
-	if (sp >= (bot + STACK_SAFETY_GAP) && sp < top) {
 		if (mark < STACK_WARNING_GAP) {
 			prlog(PR_EMERG, "CPU %04x Stack usage danger !"
 			      " pc=%08llx sp=%08llx (gap=%lld) token=%lld\n",
 			      c->pir, lr, sp, mark, c->current_token);
-			backtrace();
 		}
+	}
+
+	/* Stack is within bounds? */
+	if (sp >= (bot + STACK_SAFETY_GAP) && sp < top) {
 		c->in_mcount = false;
 		return;
 	}