| Message ID | 20200909023050.1996918-1-oohall@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | stack: only print stack usage backtraces when we hit a new watermark | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (abe4c4799ffee4be12674ad59fc0bc521b0724f3) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
diff --git a/core/stack.c b/core/stack.c index 2df960b3e6cb..05506f6b364b 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; }
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. --- core/stack.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)