Re: [PATCH] x86/dumpstack/64: Add guard pages to stack_info
flat view
From: Peter Zijlstra <peterz@infradead.org>
Date: 2021-09-16 08:00:40
Also in:
bpf, linux-perf-users, lkml
On Thu, Sep 16, 2021 at 11:47:49AM +0800, 王贇 wrote:
quoted hunk ↗ jump to hunk
I did some debug and found the issue, we are missing:@@ -122,7 +137,10 @@ static __always_inline bool in_exception_stack(unsigned long *stack, struct stac info->type = ep->type; info->begin = (unsigned long *)begin; info->end = (unsigned long *)end; - info->next_sp = (unsigned long *)regs->sp; + + if (!(ep->type & STACK_TYPE_GUARD)) + info->next_sp = (unsigned long *)regs->sp; + return true; }as the guard page are not working as real stack I guess?
Correct, but I thought I put if (type & GUARD) terminators in all paths that ended up caring about ->next_sp. Clearly I seem to have missed one :/ Let me try and figure out where that happens.
With that one things going on correctly, and some trivials below.
quoted
enum stack_type { - STACK_TYPE_UNKNOWN, + STACK_TYPE_UNKNOWN = 0,Is this necessary?
No, but it makes it more explicit we care about the value.
quoted
STACK_TYPE_TASK, STACK_TYPE_IRQ, STACK_TYPE_SOFTIRQ, STACK_TYPE_ENTRY, STACK_TYPE_EXCEPTION, STACK_TYPE_EXCEPTION_LAST = STACK_TYPE_EXCEPTION + N_EXCEPTION_STACKS-1, + STACK_TYPE_GUARD = 0x80,
Note that this is a flag.
quoted
}; struct stack_info {--- a/arch/x86/kernel/dumpstack_64.c +++ b/arch/x86/kernel/dumpstack_64.c@@ -32,9 +32,15 @@ const char *stack_type_name(enum stack_t { BUILD_BUG_ON(N_EXCEPTION_STACKS != 6); + if (type == STACK_TYPE_TASK) + return "TASK"; + if (type == STACK_TYPE_IRQ) return "IRQ"; + if (type == STACK_TYPE_SOFTIRQ) + return "SOFTIRQ"; +Do we need one for GUARD too?
No, GUARD is not a single type but a flag. The caller can trivially do something like: "%s %s", stack_type_name(type & ~GUARD), (type & GUARD) ? "GUARD" : ""
quoted
if (type == STACK_TYPE_ENTRY) { /* * On 64-bit, we have a generic entry stack that we
quoted
@@ -111,10 +122,11 @@ static __always_inline bool in_exception k = (stk - begin) >> PAGE_SHIFT; /* Lookup the page descriptor */ ep = &estack_pages[k]; - /* Guard page? */ + /* unknown entry */ if (!ep->size) return false; +Extra line?
Gone now, thanks!