From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757590AbcHCCuo (ORCPT ); Tue, 2 Aug 2016 22:50:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59686 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755572AbcHCCua (ORCPT ); Tue, 2 Aug 2016 22:50:30 -0400 Date: Tue, 2 Aug 2016 21:50:12 -0500 From: Josh Poimboeuf To: Steven Rostedt Cc: Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, Andy Lutomirski , Linus Torvalds , Brian Gerst , Kees Cook , Peter Zijlstra , Frederic Weisbecker , Byungchul Park Subject: Re: [PATCH 05/19] x86/dumpstack: fix function graph tracing stack dump reliability issues Message-ID: <20160803025012.bmte6hjhcd6lotod@treble> References: <20160729222036.340b51ce@grimm.local.home> <20160730135125.qqw5qz24szlc2crz@treble> <20160801102821.1d6261a9@gandalf.local.home> <20160801153633.c7sa2rclkqwbdagd@treble> <20160802210011.k6li4mnyvky5jtev@treble> <20160802171610.09156c90@gandalf.local.home> <20160802221359.tkb3mkjzkq3petrq@treble> <20160802191622.466f53d2@gandalf.local.home> <20160803015656.j6fxrgucmghh5gfn@treble> <20160802223011.6209e28c@grimm.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160802223011.6209e28c@grimm.local.home> User-Agent: Mutt/1.6.0.1 (2016-04-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Wed, 03 Aug 2016 02:50:15 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Aug 02, 2016 at 10:30:11PM -0400, Steven Rostedt wrote: > On Tue, 2 Aug 2016 20:56:56 -0500 > Josh Poimboeuf wrote: > > > It's not specific to NMIs. The problem is that dump_trace() is starting > > from the frame pointed to by a pt_regs, rather than the current frame. > > Instead of starting with the current frame, the first 10 functions on > > the stack are skipped by the unwinder, but they're *not* skipped on the > > ret_stack. So it starts out out-of-sync. > > OK, I see what you mean. If we do a dumpstack from interrupt passing in > the pt_regs of the kernel thread that was interrupted, even though > functions up to the interrupt was called and traced, which will show up > in the dump stack that shouldn't. > > OK, you convinced me. Add the extra pointer, then we will have 4 longs > and 2 long longs in ftrace_ret_stack. That's not that bad. Hm, since 'fp' is only used for mcount, I guess we could avoid allocating it for fentry? That would save a long when a modern compiler is used. Like: diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index 1e814ae..fc508a7 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -795,7 +795,9 @@ struct ftrace_ret_stack { unsigned long func; unsigned long long calltime; unsigned long long subtime; +#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY) unsigned long fp; +#endif }; /* diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c index 9caa9b2..86b2719 100644 --- a/kernel/trace/trace_functions_graph.c +++ b/kernel/trace/trace_functions_graph.c @@ -171,7 +171,9 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func, int *depth, current->ret_stack[index].func = func; current->ret_stack[index].calltime = calltime; current->ret_stack[index].subtime = 0; +#if defined(CONFIG_HAVE_FUNCTION_GRAPH_FP_TEST) && !defined(CC_USING_FENTRY) current->ret_stack[index].fp = frame_pointer; +#endif *depth = current->curr_ret_stack; return 0;