Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753155AbdLDIFS (ORCPT ); Mon, 4 Dec 2017 03:05:18 -0500 Received: from smtprelay0154.hostedemail.com ([216.40.44.154]:45445 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750812AbdLDIFQ (ORCPT ); Mon, 4 Dec 2017 03:05:16 -0500 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::::::::,RULES_HIT:41:355:379:541:599:800:960:968:973:988:989:1260:1277:1311:1313:1314:1345:1359:1437:1515:1516:1518:1535:1544:1593:1594:1605:1711:1730:1747:1777:1792:2194:2199:2393:2553:2559:2562:2691:2693:2893:3138:3139:3140:3141:3142:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:4250:4398:5007:6261:7514:7875:7903:7904:9108:10004:10128:10848:10967:11026:11232:11473:11658:11914:12043:12050:12296:12438:12663:12740:12760:12895:13141:13161:13229:13230:13439:14096:14097:14181:14659:14721:21080:21434:21451:21627:30003:30012:30054:30090:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:2,LUA_SUMMARY:none X-HE-Tag: wound35_67d96685a2c42 X-Filterd-Recvd-Size: 5736 Date: Mon, 4 Dec 2017 03:05:09 -0500 From: Steven Rostedt To: Alan Kao Cc: Palmer Dabbelt , Albert Ou , patches@groups.riscv.org, Ingo Molnar , linux-kernel@vger.kernel.org, Alan Kao , Greentime Hu Subject: Re: [PATCH] riscv/ftrace: Add basic support Message-ID: <20171204030509.1369baeb@vmware.local.home> In-Reply-To: <20171204055228.GC30057@d0b76efcf64c> References: <20171130085335.7949-1-alankao@andestech.com> <20171130152421.773fe6ec@gandalf.local.home> <20171204055228.GC30057@d0b76efcf64c> X-Mailer: Claws Mail 3.15.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4580 Lines: 130 On Mon, 4 Dec 2017 13:52:30 +0800 Alan Kao wrote: > > > Note that the functions in both ftrace.c and setup.c should not be > > > hooked with the compiler's -pg option: to prevent infinite self- > > > referencing for the former, and to ignore early setup stuff for the latter. > > > > I'm curious to what is in setup.c that ftrace uses. > > In the scenario for some embedded systems, the __init prefix does not give > us the notrace feature without the MODULE config. Therefore, all functions > would have been hooked with the _mcount trampoline if the -pg flag was not > specifically disabled. But is there functions you may want to trace. There's an effort going on to allow function tracing to start in early boot up. > > And a terrible result would have happened after function setup_vm called > _mcount. As _mcount compared the value of ftrace_trace_function and > the position of ftrace_stub, it crashed the kernel because one of them > was a physical address while the other was a virtual address but > actually they pointed to the same. > > Adding notrace to setup_vm can solve the described issue, but it might be > redundant once the MODULE config becomes stable and default on most > platforms. To be honest, nobody really needs those init procedures to be > ftrace-able. Um no, because MODULE init code can now be traced. It use to be that we didn't trace any __init, but I worked on having both inits be traced. The module code was a little bit trickier because it can be loaded multiple times and we needed to figure out the best way to handle init functions in the buffer that went stale and is replaced by other module init functions. > > > +config TRACE_IRQFLAGS_SUPPORT > > > + def_bool y > > > + > > > +config LOCKDEP_SUPPORT > > > + def_bool y > > > > Hmm, not sure the above is needed for function tracing. > > > > FTRACE depends on TRACING_SUPPORT, and TRACING_SUPPORT depends on > TRACE_IRQFLAGS_SUPPORT. But LOCKDEP_SUPPORT is not actually needed > for any of the ftrace features implemented in this patch. Hmm, I think that's stale. Thanks for bringing that to my attention, and don't believe that dependency still exists. > > The LOCKDEP_SUPPORT will be removed in the next version. > I should have also asked, is lockdep really supported on this arch, and is IRQSFLAGS really supported too? I vaguely remember making ftrace depend on IRQFLAGS because we wanted archs to support TRACE_IRQFLAGS before they supported ftrace. Maybe I'll keep that dependency. > > > +ENTRY(_mcount) > > > + la t4, ftrace_stub > > > +#ifdef CONFIG_FUNCTION_GRAPH_TRACER > > > + la t0, ftrace_graph_return > > > + ld t1, 0(t0) > > > + bne t1, t4, do_ftrace_graph_caller > > > > If function graph is enabled, you jump straight to the graph tracer, > > but never return back to here? > > > > Because prepare_ftrace_return function can return to the caller of > _mcount directly without messing up the stack. Yes, is that required? > > > > + > > > + la t3, ftrace_graph_entry > > > + ld t2, 0(t3) > > > + la t6, ftrace_graph_entry_stub > > > + bne t2, t6, do_ftrace_graph_caller > > > +#endif > > > + la t3, ftrace_trace_function > > > + ld t5, 0(t3) > > > + bne t5, t4, do_trace > > > + ret > > > + > > > +#ifdef CONFIG_FUNCTION_GRAPH_TRACER > > > +/* > > > + * A pseudo representation for the function graph tracer: > > > + * prepare_to_return(&ra_to_caller_of_caller, ra_to_caller) > > > + */ > > > +do_ftrace_graph_caller: > > > + addi a0, s0, -8 > > > + mv a1, ra > > > +#ifdef HAVE_FUNCTION_GRAPH_FP_TEST > > > + ld a2, -16(s0) > > > +#endif > > > + SAVE_ABI_STATE > > > + la t0, prepare_ftrace_return > > > + jalr t0 > > > + STORE_ABI_STATE > > > > I'm guessing you don't support function tracer and function graph > > tracer running at the same time? > > > > -- Steve > > > > This code section implements similar logic as those for arm, arm64, > blackfin, and others. Also, according to Documentation/trace/ftrace.txt, > the current_tracer is introduced as singular. > > Is it necessary to support simultaneous tracers? Well, you can do things like have multiple buffers today (different tracers recording in different buffers). We can have function tracing happening at the same time as the graph tracer. Is this a requirement? No. Just letting you know. While you only support static ftrace, and not dynamic (code modifying) ftrace, this isn't yet an issue. I'm just trying to let you know of some of the current features that are supported in other archs, in case you extend this. -- Steve