From: "Nicholas Piggin" <npiggin@gmail.com>
To: "Naveen N Rao" <naveen@kernel.org>,
<linuxppc-dev@lists.ozlabs.org>,
<linux-trace-kernel@vger.kernel.org>, <bpf@vger.kernel.org>
Cc: "Michael Ellerman" <mpe@ellerman.id.au>,
"Steven Rostedt" <rostedt@goodmis.org>,
"Masami Hiramatsu" <mhiramat@kernel.org>,
"Christophe Leroy" <christophe.leroy@csgroup.eu>,
"Masahiro Yamada" <masahiroy@kernel.org>,
"Mark Rutland" <mark.rutland@arm.com>,
"Alexei Starovoitov" <ast@kernel.org>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"John Fastabend" <john.fastabend@gmail.com>,
"Andrii Nakryiko" <andrii@kernel.org>,
"Song Liu" <song@kernel.org>, "Jiri Olsa" <jolsa@kernel.org>
Subject: Re: [RFC PATCH v3 01/11] powerpc/kprobes: Use ftrace to determine if a probe is at function entry
Date: Mon, 01 Jul 2024 18:40:50 +1000 [thread overview]
Message-ID: <D2E2GLXWB7TH.1L7TFQZO3149Y@gmail.com> (raw)
In-Reply-To: <2cd04be69e90adc34bcf98d405ab6b21f268cb6a.1718908016.git.naveen@kernel.org>
On Fri Jun 21, 2024 at 4:54 AM AEST, Naveen N Rao wrote:
> Rather than hard-coding the offset into a function to be used to
> determine if a kprobe is at function entry, use ftrace_location() to
> determine the ftrace location within the function and categorize all
> instructions till that offset to be function entry.
>
> For functions that cannot be traced, we fall back to using a fixed
> offset of 8 (two instructions) to categorize a probe as being at
> function entry for 64-bit elfv2, unless we are using pcrel.
>
> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Signed-off-by: Naveen N Rao <naveen@kernel.org>
> ---
> arch/powerpc/kernel/kprobes.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 14c5ddec3056..ca204f4f21c1 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -105,24 +105,22 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
> return addr;
> }
>
> -static bool arch_kprobe_on_func_entry(unsigned long offset)
> +static bool arch_kprobe_on_func_entry(unsigned long addr, unsigned long offset)
> {
> -#ifdef CONFIG_PPC64_ELF_ABI_V2
> -#ifdef CONFIG_KPROBES_ON_FTRACE
> - return offset <= 16;
> -#else
> - return offset <= 8;
> -#endif
> -#else
> + unsigned long ip = ftrace_location(addr);
> +
> + if (ip)
> + return offset <= (ip - addr);
> + if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V2) && !IS_ENABLED(CONFIG_PPC_KERNEL_PCREL))
> + return offset <= 8;
If it is PCREL, why not offset == 0 as well?
Thanks,
Nick
WARNING: multiple messages have this Message-ID (diff)
From: "Nicholas Piggin" <npiggin@gmail.com>
To: "Naveen N Rao" <naveen@kernel.org>,
<linuxppc-dev@lists.ozlabs.org>,
<linux-trace-kernel@vger.kernel.org>, <bpf@vger.kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Masahiro Yamada <masahiroy@kernel.org>,
John Fastabend <john.fastabend@gmail.com>,
Alexei Starovoitov <ast@kernel.org>,
Christophe Leroy <christophe.leroy@csgroup.eu>,
Andrii Nakryiko <andrii@kernel.org>, Song Liu <song@kernel.org>,
Steven Rostedt <rostedt@goodmis.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Jiri Olsa <jolsa@kernel.org>
Subject: Re: [RFC PATCH v3 01/11] powerpc/kprobes: Use ftrace to determine if a probe is at function entry
Date: Mon, 01 Jul 2024 18:40:50 +1000 [thread overview]
Message-ID: <D2E2GLXWB7TH.1L7TFQZO3149Y@gmail.com> (raw)
In-Reply-To: <2cd04be69e90adc34bcf98d405ab6b21f268cb6a.1718908016.git.naveen@kernel.org>
On Fri Jun 21, 2024 at 4:54 AM AEST, Naveen N Rao wrote:
> Rather than hard-coding the offset into a function to be used to
> determine if a kprobe is at function entry, use ftrace_location() to
> determine the ftrace location within the function and categorize all
> instructions till that offset to be function entry.
>
> For functions that cannot be traced, we fall back to using a fixed
> offset of 8 (two instructions) to categorize a probe as being at
> function entry for 64-bit elfv2, unless we are using pcrel.
>
> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Signed-off-by: Naveen N Rao <naveen@kernel.org>
> ---
> arch/powerpc/kernel/kprobes.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 14c5ddec3056..ca204f4f21c1 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -105,24 +105,22 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
> return addr;
> }
>
> -static bool arch_kprobe_on_func_entry(unsigned long offset)
> +static bool arch_kprobe_on_func_entry(unsigned long addr, unsigned long offset)
> {
> -#ifdef CONFIG_PPC64_ELF_ABI_V2
> -#ifdef CONFIG_KPROBES_ON_FTRACE
> - return offset <= 16;
> -#else
> - return offset <= 8;
> -#endif
> -#else
> + unsigned long ip = ftrace_location(addr);
> +
> + if (ip)
> + return offset <= (ip - addr);
> + if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V2) && !IS_ENABLED(CONFIG_PPC_KERNEL_PCREL))
> + return offset <= 8;
If it is PCREL, why not offset == 0 as well?
Thanks,
Nick
next prev parent reply other threads:[~2024-07-01 8:40 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-20 18:54 [RFC PATCH v3 00/11] powerpc: Add support for ftrace direct and BPF trampolines Naveen N Rao
2024-06-20 18:54 ` Naveen N Rao
2024-06-20 18:54 ` [RFC PATCH v3 01/11] powerpc/kprobes: Use ftrace to determine if a probe is at function entry Naveen N Rao
2024-06-20 18:54 ` Naveen N Rao
2024-07-01 8:40 ` Nicholas Piggin [this message]
2024-07-01 8:40 ` Nicholas Piggin
2024-07-01 18:18 ` Naveen N Rao
2024-07-01 18:18 ` Naveen N Rao
2024-06-20 18:54 ` [RFC PATCH v3 02/11] powerpc/ftrace: Unify 32-bit and 64-bit ftrace entry code Naveen N Rao
2024-06-20 18:54 ` Naveen N Rao
2024-07-01 8:57 ` Nicholas Piggin
2024-07-01 8:57 ` Nicholas Piggin
2024-07-01 18:34 ` Naveen N Rao
2024-07-01 18:34 ` Naveen N Rao
2024-06-20 18:54 ` [RFC PATCH v3 03/11] powerpc/module_64: Convert #ifdef to IS_ENABLED() Naveen N Rao
2024-06-20 18:54 ` Naveen N Rao
2024-07-01 9:01 ` Nicholas Piggin
2024-07-01 9:01 ` Nicholas Piggin
2024-06-20 18:54 ` [RFC PATCH v3 04/11] powerpc/ftrace: Remove pointer to struct module from dyn_arch_ftrace Naveen N Rao
2024-06-20 18:54 ` Naveen N Rao
2024-07-01 9:27 ` Nicholas Piggin
2024-07-01 9:27 ` Nicholas Piggin
2024-07-01 18:51 ` Naveen N Rao
2024-07-01 18:51 ` Naveen N Rao
2024-06-20 18:54 ` [RFC PATCH v3 05/11] kbuild: Add generic hook for architectures to use before the final vmlinux link Naveen N Rao
2024-06-20 18:54 ` Naveen N Rao
2024-07-01 9:30 ` Nicholas Piggin
2024-07-01 9:30 ` Nicholas Piggin
2024-06-20 18:54 ` [RFC PATCH v3 06/11] powerpc64/ftrace: Move ftrace sequence out of line Naveen N Rao
2024-06-20 18:54 ` Naveen N Rao
2024-07-01 10:39 ` Nicholas Piggin
2024-07-01 10:39 ` Nicholas Piggin
2024-07-01 19:44 ` Naveen N Rao
2024-07-01 19:44 ` Naveen N Rao
2024-06-20 18:54 ` [RFC PATCH v3 07/11] powerpc/ftrace: Add support for DYNAMIC_FTRACE_WITH_CALL_OPS Naveen N Rao
2024-06-20 18:54 ` Naveen N Rao
2024-06-20 18:54 ` [RFC PATCH v3 08/11] powerpc/ftrace: Add support for DYNAMIC_FTRACE_WITH_DIRECT_CALLS Naveen N Rao
2024-06-20 18:54 ` Naveen N Rao
2024-06-20 18:54 ` [RFC PATCH v3 09/11] samples/ftrace: Add support for ftrace direct samples on powerpc Naveen N Rao
2024-06-20 18:54 ` Naveen N Rao
2024-06-20 19:09 ` [RFC PATCH v3 10/11] powerpc64/bpf: Fold bpf_jit_emit_func_call_hlp() into bpf_jit_emit_func_call_rel() Naveen N Rao
2024-06-20 19:09 ` Naveen N Rao
2024-06-20 19:09 ` [RFC PATCH v3 11/11] powerpc64/bpf: Add support for bpf trampolines Naveen N Rao
2024-06-20 19:09 ` Naveen N Rao
2024-07-01 11:03 ` Nicholas Piggin
2024-07-01 11:03 ` Nicholas Piggin
2024-07-01 19:58 ` Naveen N Rao
2024-07-01 19:58 ` Naveen N Rao
2024-06-24 11:59 ` [RFC PATCH v3 00/11] powerpc: Add support for ftrace direct and BPF trampolines Vishal Chourasia
2024-06-24 11:59 ` Vishal Chourasia
2024-07-03 11:11 ` Vishal Chourasia
2024-07-03 11:11 ` Vishal Chourasia
2024-07-14 7:52 ` Naveen N Rao
2024-07-14 7:52 ` Naveen N Rao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D2E2GLXWB7TH.1L7TFQZO3149Y@gmail.com \
--to=npiggin@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=christophe.leroy@csgroup.eu \
--cc=daniel@iogearbox.net \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mark.rutland@arm.com \
--cc=masahiroy@kernel.org \
--cc=mhiramat@kernel.org \
--cc=mpe@ellerman.id.au \
--cc=naveen@kernel.org \
--cc=rostedt@goodmis.org \
--cc=song@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.