From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Jinjie Ruan <ruanjinjie@huawei.com>
Cc: <paul.walmsley@sifive.com>, <palmer@dabbelt.com>,
<aou@eecs.berkeley.edu>, <rostedt@goodmis.org>,
<mark.rutland@arm.com>, <linux-riscv@lists.infradead.org>,
<linux-kernel@vger.kernel.org>,
<linux-trace-kernel@vger.kernel.org>
Subject: Re: [PATCH] trace: riscv: Remove deprecated kprobe on ftrace support
Date: Wed, 19 Jun 2024 23:07:20 +0900 [thread overview]
Message-ID: <20240619230720.14fbd3a7e03139cbc0157407@kernel.org> (raw)
In-Reply-To: <20240613111347.1745379-1-ruanjinjie@huawei.com>
On Thu, 13 Jun 2024 19:13:47 +0800
Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> Since commit 7caa9765465f60 ("ftrace: riscv: move from REGS to ARGS"),
> kprobe on ftrace is not supported by riscv, because riscv's support for
> FTRACE_WITH_REGS has been replaced with support for FTRACE_WITH_ARGS, and
> KPROBES_ON_FTRACE will be supplanted by FPROBES. So remove the deprecated
> kprobe on ftrace support, which is misunderstood.
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
No problem, Now we have fprobe instead.
Anyway, this looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thank you,
> ---
> arch/riscv/Kconfig | 1 -
> arch/riscv/kernel/probes/Makefile | 1 -
> arch/riscv/kernel/probes/ftrace.c | 65 -------------------------------
> 3 files changed, 67 deletions(-)
> delete mode 100644 arch/riscv/kernel/probes/ftrace.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 0525ee2d63c7..a1f2d604c459 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -149,7 +149,6 @@ config RISCV
> select HAVE_KERNEL_UNCOMPRESSED if !XIP_KERNEL && !EFI_ZBOOT
> select HAVE_KERNEL_ZSTD if !XIP_KERNEL && !EFI_ZBOOT
> select HAVE_KPROBES if !XIP_KERNEL
> - select HAVE_KPROBES_ON_FTRACE if !XIP_KERNEL
> select HAVE_KRETPROBES if !XIP_KERNEL
> # https://github.com/ClangBuiltLinux/linux/issues/1881
> select HAVE_LD_DEAD_CODE_DATA_ELIMINATION if !LD_IS_LLD
> diff --git a/arch/riscv/kernel/probes/Makefile b/arch/riscv/kernel/probes/Makefile
> index 8265ff497977..d2129f2c61b8 100644
> --- a/arch/riscv/kernel/probes/Makefile
> +++ b/arch/riscv/kernel/probes/Makefile
> @@ -1,7 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o simulate-insn.o
> obj-$(CONFIG_RETHOOK) += rethook.o rethook_trampoline.o
> -obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
> obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o simulate-insn.o
> CFLAGS_REMOVE_simulate-insn.o = $(CC_FLAGS_FTRACE)
> CFLAGS_REMOVE_rethook.o = $(CC_FLAGS_FTRACE)
> diff --git a/arch/riscv/kernel/probes/ftrace.c b/arch/riscv/kernel/probes/ftrace.c
> deleted file mode 100644
> index a69dfa610aa8..000000000000
> --- a/arch/riscv/kernel/probes/ftrace.c
> +++ /dev/null
> @@ -1,65 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -
> -#include <linux/kprobes.h>
> -
> -/* Ftrace callback handler for kprobes -- called under preepmt disabled */
> -void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *ops, struct ftrace_regs *fregs)
> -{
> - struct kprobe *p;
> - struct pt_regs *regs;
> - struct kprobe_ctlblk *kcb;
> - int bit;
> -
> - if (unlikely(kprobe_ftrace_disabled))
> - return;
> -
> - bit = ftrace_test_recursion_trylock(ip, parent_ip);
> - if (bit < 0)
> - return;
> -
> - p = get_kprobe((kprobe_opcode_t *)ip);
> - if (unlikely(!p) || kprobe_disabled(p))
> - goto out;
> -
> - regs = ftrace_get_regs(fregs);
> - kcb = get_kprobe_ctlblk();
> - if (kprobe_running()) {
> - kprobes_inc_nmissed_count(p);
> - } else {
> - unsigned long orig_ip = instruction_pointer(regs);
> -
> - instruction_pointer_set(regs, ip);
> -
> - __this_cpu_write(current_kprobe, p);
> - kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> - if (!p->pre_handler || !p->pre_handler(p, regs)) {
> - /*
> - * Emulate singlestep (and also recover regs->pc)
> - * as if there is a nop
> - */
> - instruction_pointer_set(regs,
> - (unsigned long)p->addr + MCOUNT_INSN_SIZE);
> - if (unlikely(p->post_handler)) {
> - kcb->kprobe_status = KPROBE_HIT_SSDONE;
> - p->post_handler(p, regs, 0);
> - }
> - instruction_pointer_set(regs, orig_ip);
> - }
> -
> - /*
> - * If pre_handler returns !0, it changes regs->pc. We have to
> - * skip emulating post_handler.
> - */
> - __this_cpu_write(current_kprobe, NULL);
> - }
> -out:
> - ftrace_test_recursion_unlock(bit);
> -}
> -NOKPROBE_SYMBOL(kprobe_ftrace_handler);
> -
> -int arch_prepare_kprobe_ftrace(struct kprobe *p)
> -{
> - p->ainsn.api.insn = NULL;
> - return 0;
> -}
> --
> 2.34.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
WARNING: multiple messages have this Message-ID (diff)
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Jinjie Ruan <ruanjinjie@huawei.com>
Cc: <paul.walmsley@sifive.com>, <palmer@dabbelt.com>,
<aou@eecs.berkeley.edu>, <rostedt@goodmis.org>,
<mark.rutland@arm.com>, <linux-riscv@lists.infradead.org>,
<linux-kernel@vger.kernel.org>,
<linux-trace-kernel@vger.kernel.org>
Subject: Re: [PATCH] trace: riscv: Remove deprecated kprobe on ftrace support
Date: Wed, 19 Jun 2024 23:07:20 +0900 [thread overview]
Message-ID: <20240619230720.14fbd3a7e03139cbc0157407@kernel.org> (raw)
In-Reply-To: <20240613111347.1745379-1-ruanjinjie@huawei.com>
On Thu, 13 Jun 2024 19:13:47 +0800
Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> Since commit 7caa9765465f60 ("ftrace: riscv: move from REGS to ARGS"),
> kprobe on ftrace is not supported by riscv, because riscv's support for
> FTRACE_WITH_REGS has been replaced with support for FTRACE_WITH_ARGS, and
> KPROBES_ON_FTRACE will be supplanted by FPROBES. So remove the deprecated
> kprobe on ftrace support, which is misunderstood.
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
No problem, Now we have fprobe instead.
Anyway, this looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thank you,
> ---
> arch/riscv/Kconfig | 1 -
> arch/riscv/kernel/probes/Makefile | 1 -
> arch/riscv/kernel/probes/ftrace.c | 65 -------------------------------
> 3 files changed, 67 deletions(-)
> delete mode 100644 arch/riscv/kernel/probes/ftrace.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 0525ee2d63c7..a1f2d604c459 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -149,7 +149,6 @@ config RISCV
> select HAVE_KERNEL_UNCOMPRESSED if !XIP_KERNEL && !EFI_ZBOOT
> select HAVE_KERNEL_ZSTD if !XIP_KERNEL && !EFI_ZBOOT
> select HAVE_KPROBES if !XIP_KERNEL
> - select HAVE_KPROBES_ON_FTRACE if !XIP_KERNEL
> select HAVE_KRETPROBES if !XIP_KERNEL
> # https://github.com/ClangBuiltLinux/linux/issues/1881
> select HAVE_LD_DEAD_CODE_DATA_ELIMINATION if !LD_IS_LLD
> diff --git a/arch/riscv/kernel/probes/Makefile b/arch/riscv/kernel/probes/Makefile
> index 8265ff497977..d2129f2c61b8 100644
> --- a/arch/riscv/kernel/probes/Makefile
> +++ b/arch/riscv/kernel/probes/Makefile
> @@ -1,7 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o simulate-insn.o
> obj-$(CONFIG_RETHOOK) += rethook.o rethook_trampoline.o
> -obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
> obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o simulate-insn.o
> CFLAGS_REMOVE_simulate-insn.o = $(CC_FLAGS_FTRACE)
> CFLAGS_REMOVE_rethook.o = $(CC_FLAGS_FTRACE)
> diff --git a/arch/riscv/kernel/probes/ftrace.c b/arch/riscv/kernel/probes/ftrace.c
> deleted file mode 100644
> index a69dfa610aa8..000000000000
> --- a/arch/riscv/kernel/probes/ftrace.c
> +++ /dev/null
> @@ -1,65 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -
> -#include <linux/kprobes.h>
> -
> -/* Ftrace callback handler for kprobes -- called under preepmt disabled */
> -void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *ops, struct ftrace_regs *fregs)
> -{
> - struct kprobe *p;
> - struct pt_regs *regs;
> - struct kprobe_ctlblk *kcb;
> - int bit;
> -
> - if (unlikely(kprobe_ftrace_disabled))
> - return;
> -
> - bit = ftrace_test_recursion_trylock(ip, parent_ip);
> - if (bit < 0)
> - return;
> -
> - p = get_kprobe((kprobe_opcode_t *)ip);
> - if (unlikely(!p) || kprobe_disabled(p))
> - goto out;
> -
> - regs = ftrace_get_regs(fregs);
> - kcb = get_kprobe_ctlblk();
> - if (kprobe_running()) {
> - kprobes_inc_nmissed_count(p);
> - } else {
> - unsigned long orig_ip = instruction_pointer(regs);
> -
> - instruction_pointer_set(regs, ip);
> -
> - __this_cpu_write(current_kprobe, p);
> - kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> - if (!p->pre_handler || !p->pre_handler(p, regs)) {
> - /*
> - * Emulate singlestep (and also recover regs->pc)
> - * as if there is a nop
> - */
> - instruction_pointer_set(regs,
> - (unsigned long)p->addr + MCOUNT_INSN_SIZE);
> - if (unlikely(p->post_handler)) {
> - kcb->kprobe_status = KPROBE_HIT_SSDONE;
> - p->post_handler(p, regs, 0);
> - }
> - instruction_pointer_set(regs, orig_ip);
> - }
> -
> - /*
> - * If pre_handler returns !0, it changes regs->pc. We have to
> - * skip emulating post_handler.
> - */
> - __this_cpu_write(current_kprobe, NULL);
> - }
> -out:
> - ftrace_test_recursion_unlock(bit);
> -}
> -NOKPROBE_SYMBOL(kprobe_ftrace_handler);
> -
> -int arch_prepare_kprobe_ftrace(struct kprobe *p)
> -{
> - p->ainsn.api.insn = NULL;
> - return 0;
> -}
> --
> 2.34.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
next prev parent reply other threads:[~2024-06-19 14:07 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-13 11:13 [PATCH] trace: riscv: Remove deprecated kprobe on ftrace support Jinjie Ruan
2024-06-13 11:13 ` Jinjie Ruan
2024-06-19 14:07 ` Masami Hiramatsu [this message]
2024-06-19 14:07 ` Masami Hiramatsu
2024-07-25 13:20 ` patchwork-bot+linux-riscv
2024-07-25 13:20 ` patchwork-bot+linux-riscv
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=20240619230720.14fbd3a7e03139cbc0157407@kernel.org \
--to=mhiramat@kernel.org \
--cc=aou@eecs.berkeley.edu \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=rostedt@goodmis.org \
--cc=ruanjinjie@huawei.com \
/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.