On Fri, Apr 19, 2019 at 11:07:17AM +0200, Peter Zijlstra wrote: > On Fri, Apr 19, 2019 at 10:32:30AM +0200, Thomas Gleixner wrote: > > On Fri, 19 Apr 2019, Peter Zijlstra wrote: > > > On Thu, Apr 18, 2019 at 10:41:47AM +0200, Thomas Gleixner wrote: > > > > > > > +typedef bool (*stack_trace_consume_fn)(void *cookie, unsigned long addr, > > > > + bool reliable); > > > > > > > +void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie, > > > > + struct task_struct *task, struct pt_regs *regs); > > > > +int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry, void *cookie, > > > > + struct task_struct *task); > > > > > > This bugs me a little; ideally the _reliable() thing would not exists. > > > > > > Thomas said that the existing __save_stack_trace_reliable() is different > > > enough for the unification to be non-trivial, but maybe Josh can help > > > out? > > > > > > >From what I can see the biggest significant differences are: > > > > > > - it looks at the regs sets on the stack and for FP bails early > > > - bails for khreads and idle (after it does all the hard work!?!) That's done for a reason, see the "Success path" comments. > > > The first (FP checking for exceptions) should probably be reflected in > > > consume_fn(.reliable) anyway -- although that would mean a lot of extra > > > '?' entries where there are none today. > > > > > > And the second (KTHREAD/IDLE) is something that the generic code can > > > easily do before calling into the arch unwinder. > > > > And looking at the powerpc version of it, that has even more interesting > > extra checks in that function. > > Right, but not fundamentally different from determining @reliable I > think. > > Anyway, it would be good if someone knowledgable could have a look at > this. Yeah, we could probably do that. The flow would need to be changed a bit -- some of the errors are soft errors which most users don't care about because they just want a best effort. The soft errors can be remembered without breaking out of the loop, and just returned at the end. Most users could just ignore the return code. The only thing I'd be worried about is performance for the non-livepatch users, but I guess those checks don't look very expensive. And the x86 unwinders are already pretty slow anyway... -- Josh
