Messages in this thread | | | Date | Tue, 2 Aug 2016 17:13:59 -0500 | From | Josh Poimboeuf <> | Subject | Re: [PATCH 05/19] x86/dumpstack: fix function graph tracing stack dump reliability issues |
| |
On Tue, Aug 02, 2016 at 05:16:10PM -0400, Steven Rostedt wrote: > On Tue, 2 Aug 2016 16:00:11 -0500 > Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > [<ffffffff81061d8b>] nmi_raise_cpu_backtrace+0x1b/0x20 > > > > The ret_stack is out of sync with the stack dump because the stack dump > > was started with the regs from the NMI, instead of being started from > > the current frame. > > > > So I guess there are a couple of ways to fix it: > > > > a) keep track of the return address pointer like we discussed above; > > > > or > > > > b) have the unwinder count the # of skipped frames which refer to > > 'return_to_handler', and pass that as the initial index value to > > ftrace_graph_ret_addr(). > > > > Option a) would be much cleaner. But to fix it for both mcount and > > fentry, we couldn't override 'fp' so I guess we'd need to add a new > > field to ftrace_ret_stack. > > Actually, what about calling ftrace_graph_ret_addr() to figure out the > next stack conversion only if reliable or CONFIG_FRAME_POINTER is not > enabled? > > unsigned long real_addr = addr; > > [...] > > if (!IS_ENABLED(CONFIG_FRAME_POINTER) || reliable) > real_addr = ftrace_graph_ret_addr(task, graph, addr); > if (addr != real_addr) > ops->address(data, addr, 0); > ops->address(data, real_addr, reliable); > > Then we only need the fp use case when FRAME_POINTER is not set. As > mcount forces FRAME_POINTER, we only need to worry about the fentry > case.
Hm, I'm confused. First, I don't see where mcount forces FRAME_POINTER.
Second, I don't see why that even matters. If mcount and frame pointers are enabled, then the 'fp' field of ftrace_ret_stack is needed for the gcc sanity check, right? So we couldn't override 'fp', and the old "stateful index" version of ftrace_graph_ret_addr() would have to be used in the code above for reliable addresses, and we'd still have the same out-of-sync bug.
Or am I missing something?
-- Josh
| |