From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754650AbcDNK6U (ORCPT ); Thu, 14 Apr 2016 06:58:20 -0400 Received: from smtprelay0009.hostedemail.com ([216.40.44.9]:34833 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754602AbcDNK6O (ORCPT ); Thu, 14 Apr 2016 06:58:14 -0400 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::,RULES_HIT:41:305:355:379:541:599:800:960:966:973:988:989:1260:1277:1311:1313:1314:1345:1359:1437:1515:1516:1518:1534:1543:1593:1594:1605:1711:1730:1747:1777:1792:1981:2194:2196:2199:2200:2393:2553:2559:2562:2693:2736:3138:3139:3140:3141:3142:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:4321:4385:5007:6119:6261:7875:7903:8545:10004:10400:10450:10455:10848:10967:11026:11232:11473:11657:11658:11914:12043:12050:12296:12438:12517:12519:12555:12740:13180:13229:13255:13439:14096:14097:14181:14659:14721:19904:19999:21080:21212:21220:21324:30022:30029:30054:30090:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:1,LUA_SUMMARY:none X-HE-Tag: balls56_479c912425d2c X-Filterd-Recvd-Size: 4673 Date: Thu, 14 Apr 2016 06:58:05 -0400 From: Steven Rostedt To: Michael Ellerman Cc: Thiago Jung Bauermann , linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Ingo Molnar Subject: Re: [PATCH] ftrace: filter: Match dot symbols when searching functions on ppc64. Message-ID: <20160414065805.102dfd90@grimm.local.home> In-Reply-To: <1460617895.2754.8.camel@ellerman.id.au> References: <1459480972-20238-1-git-send-email-bauerman@linux.vnet.ibm.com> <1460617895.2754.8.camel@ellerman.id.au> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; 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 On Thu, 14 Apr 2016 17:11:35 +1000 Michael Ellerman wrote: > > > > diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h > > index 50ca7585abe2..68f1858796c6 100644 > > --- a/arch/powerpc/include/asm/ftrace.h > > +++ b/arch/powerpc/include/asm/ftrace.h > > @@ -58,6 +58,15 @@ struct dyn_arch_ftrace { > > struct module *mod; > > }; > > #endif /* CONFIG_DYNAMIC_FTRACE */ > > + > > +#if CONFIG_PPC64 && (!defined(_CALL_ELF) || _CALL_ELF != 2) > > +#define ARCH_HAS_FTRACE_MATCH_ADJUST > > I *think* the consenus these days is that we don't add ARCH_HAS #defines in > headers. Instead you should either: > - add a CONFIG_HAVE_ARCH_FOO and use that > - use the #define foo foo trick > > The latter being that you do: > > static inline void arch_ftrace_match_adjust(char **str, char *search) > { > ... > } > #define arch_ftrace_match_adjust arch_ftrace_match_adjust > > And in ftrace.c: > > #ifndef arch_ftrace_match_adjust > static inline void arch_ftrace_match_adjust(char **str, char *search) {} > #endif > > > Presumably Steve will have a preference for which style you use. Actually, what I usually do is simply make a "weak" stub function and let the arch override it. > > > +static inline void arch_ftrace_match_adjust(char **str, char *search) > > +{ > > + if ((*str)[0] == '.' && search[0] != '.') > > + (*str)++; > > +} > > +#endif /* CONFIG_PPC64 && (!defined(_CALL_ELF) || _CALL_ELF != 2) */ > > #endif /* __ASSEMBLY__ */ > > It seems unfortunate that we need to introduce yet another mechanism to deal > with dot symbols. But I guess none of the existing mechanisms work, eg. > kprobe_lookup_name(). What about making a generic conversion to function name, like: arch_sane_function_name(str); Where all sane archs simply return the string name and powerpc can strip the '.' prefix. ;-) Of course that would require: sane_str = arch_sane_function(str); sane_search = arch_sane_function(g->search); and then compare sane_str with sane_search. > > > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > > index b1870fbd2b67..e806c2a3b7a8 100644 > > --- a/kernel/trace/ftrace.c > > +++ b/kernel/trace/ftrace.c > > @@ -3444,11 +3444,24 @@ struct ftrace_glob { > > int type; > > }; > > > > +#ifndef ARCH_HAS_FTRACE_MATCH_ADJUST > > +/* > > + * If symbols in an architecture don't correspond exactly to the user-visible > > + * name of what they represent, it is possible to define this function to > > + * perform the necessary adjustments. > > +*/ > > +static inline void arch_ftrace_match_adjust(char **str, char *search) > > +{ > > +} > > +#endif > > + > > static int ftrace_match(char *str, struct ftrace_glob *g) > > { > > int matched = 0; > > int slen; > > > > + arch_ftrace_match_adjust(&str, g->search); > > I think this would less magical if it didn't modify str directly, instead doing: > > str = arch_ftrace_match_adjust(str, g->search); > > And arch_ftrace_match_adjust() would return the adjusted char *. > > That would mean the generic version would need to return str rather than being > empty. I agree, because if we need to free the string passed it, the function would modify the pointer and we wouldn't be able to free it. In those cases we could do: tmp_str = arch_ftrace_match_adjust(str, search); /* use tmp_str and then ignore */ kfree(str); ** Disclaimer ** Note, I just took the red-eye (2 hours of sleep on the plane) and waiting for my next flight. My focus may be off in this email. -- Steve