From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D3308C433F5 for ; Mon, 7 Feb 2022 15:28:11 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4Jsqlt2htMz3cPh for ; Tue, 8 Feb 2022 02:28:10 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=kernel.org (client-ip=139.178.84.217; helo=dfw.source.kernel.org; envelope-from=srs0=xgpx=sw=goodmis.org=rostedt@kernel.org; receiver=) Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4JsqhC6VMvz3Wtp for ; Tue, 8 Feb 2022 02:24:59 +1100 (AEDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 81594614A3; Mon, 7 Feb 2022 15:24:57 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E78B2C340F0; Mon, 7 Feb 2022 15:24:55 +0000 (UTC) Date: Mon, 7 Feb 2022 10:24:54 -0500 From: Steven Rostedt To: "Naveen N. Rao" Subject: Re: [RFC PATCH 2/3] powerpc/ftrace: Override ftrace_location_lookup() for MPROFILE_KERNEL Message-ID: <20220207102454.41b1d6b5@gandalf.local.home> In-Reply-To: References: X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Borkmann , Yauheni Kaliuta , Jiri Olsa , Jordan Niethe , bpf@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Alexei Starovoitov , Hari Bathini Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Mon, 7 Feb 2022 12:37:21 +0530 "Naveen N. Rao" wrote: > --- a/arch/powerpc/kernel/trace/ftrace.c > +++ b/arch/powerpc/kernel/trace/ftrace.c > @@ -1137,3 +1137,14 @@ char *arch_ftrace_match_adjust(char *str, const char *search) > return str; > } > #endif /* PPC64_ELF_ABI_v1 */ > + > +#ifdef CONFIG_MPROFILE_KERNEL > +unsigned long ftrace_location_lookup(unsigned long ip) > +{ > + /* > + * Per livepatch.h, ftrace location is always within the first > + * 16 bytes of a function on powerpc with -mprofile-kernel. > + */ > + return ftrace_location_range(ip, ip + 16); I think this is the wrong approach for the implementation and error prone. > +} > +#endif > -- What I believe is a safer approach is to use the record address and add the range to it. That is, you know that the ftrace modification site is a range (multiple instructions), where in the ftrace infrastructure, only one ip represents that range. What you want is if you pass in an ip, and that ip is within that range, you return the ip that represents that range to ftrace. It looks like we need to change the compare function in the bsearch. Perhaps add a new macro to define the size of the range to be searched, instead of just using MCOUNT_INSN_SIZE? Then we may not even need this new lookup function? static int ftrace_cmp_recs(const void *a, const void *b) { const struct dyn_ftrace *key = a; const struct dyn_ftrace *rec = b; if (key->flags < rec->ip) return -1; if (key->ip >= rec->ip + ARCH_IP_SIZE) return 1; return 0; } Where ARCH_IP_SIZE is defined to MCOUNT_INSN_SIZE by default, but an arch could define it to something else, like 16. Would that work for you, or am I missing something? -- Steve From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 238C0C433F5 for ; Mon, 7 Feb 2022 15:38:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242216AbiBGPhq (ORCPT ); Mon, 7 Feb 2022 10:37:46 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45132 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1357145AbiBGPY6 (ORCPT ); Mon, 7 Feb 2022 10:24:58 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DF067C0401C9 for ; Mon, 7 Feb 2022 07:24:57 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 7C8AB6077B for ; Mon, 7 Feb 2022 15:24:57 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E78B2C340F0; Mon, 7 Feb 2022 15:24:55 +0000 (UTC) Date: Mon, 7 Feb 2022 10:24:54 -0500 From: Steven Rostedt To: "Naveen N. Rao" Cc: Daniel Borkmann , Alexei Starovoitov , Michael Ellerman , , , Yauheni Kaliuta , Hari Bathini , Jordan Niethe , Jiri Olsa Subject: Re: [RFC PATCH 2/3] powerpc/ftrace: Override ftrace_location_lookup() for MPROFILE_KERNEL Message-ID: <20220207102454.41b1d6b5@gandalf.local.home> In-Reply-To: References: X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On Mon, 7 Feb 2022 12:37:21 +0530 "Naveen N. Rao" wrote: > --- a/arch/powerpc/kernel/trace/ftrace.c > +++ b/arch/powerpc/kernel/trace/ftrace.c > @@ -1137,3 +1137,14 @@ char *arch_ftrace_match_adjust(char *str, const char *search) > return str; > } > #endif /* PPC64_ELF_ABI_v1 */ > + > +#ifdef CONFIG_MPROFILE_KERNEL > +unsigned long ftrace_location_lookup(unsigned long ip) > +{ > + /* > + * Per livepatch.h, ftrace location is always within the first > + * 16 bytes of a function on powerpc with -mprofile-kernel. > + */ > + return ftrace_location_range(ip, ip + 16); I think this is the wrong approach for the implementation and error prone. > +} > +#endif > -- What I believe is a safer approach is to use the record address and add the range to it. That is, you know that the ftrace modification site is a range (multiple instructions), where in the ftrace infrastructure, only one ip represents that range. What you want is if you pass in an ip, and that ip is within that range, you return the ip that represents that range to ftrace. It looks like we need to change the compare function in the bsearch. Perhaps add a new macro to define the size of the range to be searched, instead of just using MCOUNT_INSN_SIZE? Then we may not even need this new lookup function? static int ftrace_cmp_recs(const void *a, const void *b) { const struct dyn_ftrace *key = a; const struct dyn_ftrace *rec = b; if (key->flags < rec->ip) return -1; if (key->ip >= rec->ip + ARCH_IP_SIZE) return 1; return 0; } Where ARCH_IP_SIZE is defined to MCOUNT_INSN_SIZE by default, but an arch could define it to something else, like 16. Would that work for you, or am I missing something? -- Steve