On Tue, Apr 02, 2019 at 10:43:29AM -0500, Josh Poimboeuf wrote:
> On Tue, Apr 02, 2019 at 12:19:46PM +0200, Thomas Gleixner wrote:
> > static bool in_exception_stack(unsigned long *stack, struct stack_info
> > *info)
> > {
> > - unsigned long estacks, begin, end, stk = (unsigned long)stack;
> > + unsigned long begin, end, stk = (unsigned long)stack;
> > + const struct estack_pages *ep;
> > struct pt_regs *regs;
> > unsigned int k;
> >
> > BUILD_BUG_ON(N_EXCEPTION_STACKS != 4);
> >
> > - estacks = (unsigned long)__this_cpu_read(cea_exception_stacks);
> > -
> > - for (k = 0; k < N_EXCEPTION_STACKS; k++) {
> > - begin = estacks + layout[k].begin;
> > - end = estacks + layout[k].end;
> > - regs = (struct pt_regs *)end - 1;
> > -
> > - if (stk <= begin || stk >= end)
> > - continue;
> > -
> > - info->type = STACK_TYPE_EXCEPTION + k;
> > - info->begin = (unsigned long *)begin;
> > - info->end = (unsigned long *)end;
> > - info->next_sp = (unsigned long *)regs->sp;
> > -
> > - return true;
> > - }
> > -
> > - return false;
> > + begin = (unsigned long)__this_cpu_read(cea_exception_stacks);
> > + end = begin + sizeof(struct cea_exception_stacks);
> > + /* Bail if @stack is outside the exception stack area. */
> > + if (stk <= begin || stk >= end)
> > + return false;
>
> This check is the most important piece. Exception stack dumps are quite
> rare, so this ensures an early exit in most cases regardless of whether
> there's a loop below.
>
> > +
> > + /* Calc page offset from start of exception stacks */
> > + k = (stk - begin) >> PAGE_SHIFT;
> > + /* Lookup the page descriptor */
> > + ep = &estack_pages[k];
> > + /* Guard page? */
> > + if (unlikely(!ep->size))
> > + return false;
> > +
> > + begin += (unsigned long)ep->offs;
> > + end = begin + (unsigned long)ep->size;
> > + regs = (struct pt_regs *)end - 1;
> > +
> > + info->type = ep->type;
> > + info->begin = (unsigned long *)begin;
> > + info->end = (unsigned long *)end;
> > + info->next_sp = (unsigned long *)regs->sp;
> > + return true;
>
> With the above "(stk <= begin || stk >= end)" check, removing the loop
> becomes not all that important since exception stack dumps are quite
> rare and not performance sensitive. With all the macros this code
> becomes a little more obtuse, so I'm not sure whether removal of the
> loop is a net positive.
Are you sure; perf does a lot of stack dumps from NMI context.
