On Thu, Sep 16, 2021 at 11:47:49AM +0800, 王贇 wrote:
> 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.
> > 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.
> > 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.
> > };
> >
> > 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" : ""
> > if (type == STACK_TYPE_ENTRY) {
> > /*
> > * On 64-bit, we have a generic entry stack that we
> > @@ -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!
