From: "Björn Töpel" <bjorn@kernel.org>
To: Guo Ren <guoren@kernel.org>, Mark Rutland <mark.rutland@arm.com>
Cc: "liaochang (A)" <liaochang1@huawei.com>,
palmer@dabbelt.com, paul.walmsley@sifive.com,
mhiramat@kernel.org, conor.dooley@microchip.com,
penberg@kernel.org, linux-riscv@lists.infradead.org,
linux-kernel@vger.kernel.org, Guo Ren <guoren@linux.alibaba.com>
Subject: Re: [PATCH] riscv: kprobe: Optimize kprobe with accurate atomicity
Date: Tue, 31 Jan 2023 08:12:22 +0100 [thread overview]
Message-ID: <87a61z2n55.fsf@all.your.base.are.belong.to.us> (raw)
In-Reply-To: <CAJF2gTRgze_owuWvJjnrPpBNs8+GY-km7wvHU4EuJzarQc+BPQ@mail.gmail.com>
Guo Ren <guoren@kernel.org> writes:
> On Mon, Jan 30, 2023 at 11:49 PM Mark Rutland <mark.rutland@arm.com> wrote:
>>
>> Hi Bjorn,
>>
>> On Mon, Jan 30, 2023 at 04:28:15PM +0100, Björn Töpel wrote:
>> > Guo Ren <guoren@kernel.org> writes:
>> >
>> > >> In the serie of RISCV OPTPROBES [1], it patches a long-jump instructions pair
>> > >> AUIPC/JALR in kernel text, so in order to ensure other CPUs does not execute
>> > >> in the instructions that will be modified, it is still need to stop other CPUs
>> > >> via patch_text API, or you have any better solution to achieve the purpose?
>> > > - The stop_machine is an expensive way all architectures should
>> > > avoid, and you could keep that in your OPTPROBES implementation files
>> > > with static functions.
>> > > - The stop_machine couldn't work with PREEMPTION, so your
>> > > implementation needs to work with !PREEMPTION.
>> >
>> > ...and stop_machine() with !PREEMPTION is broken as well, when you're
>> > replacing multiple instructions (see Mark's post at [1]). The
>> > stop_machine() dance might work when you're replacing *one* instruction,
>> > not multiple as in the RISC-V case. I'll expand on this in a comment in
>> > the OPTPROBES v6 series.
>>
>> Just to clarify, my comments in [1] were assuming that stop_machine() was not
>> used, in which case there is a problem with or without PREEMPTION.
>>
>> I believe that when using stop_machine(), the !PREEMPTION case is fine, since
>> stop_machine() schedules work rather than running work in IRQ context on the
>> back of an IPI, so no CPUs should be mid-sequnce during the patching, and it's
>> not possible for there to be threads which are preempted mid-sequence.
>>
>> That all said, IIUC optprobes is going to disappear once fprobe is ready
>> everywhere, so that might be moot.
> The optprobes could be in the middle of a function, but fprobe must be
> the entry of a function, right?
>
> Does your fprobe here mean: ?
>
> The Linux kernel configuration item CONFIG_FPROBE:
>
> prompt: Kernel Function Probe (fprobe)
> type: bool
> depends on: ( CONFIG_FUNCTION_TRACER ) && (
> CONFIG_DYNAMIC_FTRACE_WITH_REGS ) && ( CONFIG_HAVE_RETHOOK )
> defined in kernel/trace/Kconfig
See the cover of [1]. It's about direct calls for BPF tracing (and more)
on Arm, and you're completly right, that it's *not* related to optprobes
at all.
[1] https://lore.kernel.org/all/20221108220651.24492-1-revest@chromium.org/
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2023-01-31 7:12 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-26 16:15 [PATCH] riscv: kprobe: Optimize kprobe with accurate atomicity guoren
2023-01-28 3:52 ` liaochang (A)
2023-01-28 4:45 ` Guo Ren
2023-01-30 15:28 ` Björn Töpel
2023-01-30 15:49 ` Mark Rutland
2023-01-30 16:56 ` Björn Töpel
2023-01-31 1:48 ` Guo Ren
2023-01-31 7:12 ` Björn Töpel [this message]
2023-01-31 8:30 ` Guo Ren
2023-01-31 10:33 ` Mark Rutland
2023-02-16 15:23 ` Masami Hiramatsu
2023-02-20 10:35 ` Mark Rutland
2023-02-21 1:30 ` Guo Ren
2023-01-31 1:01 ` Guo Ren
2023-01-31 1:09 ` Guo Ren
2023-01-31 7:03 ` Björn Töpel
2023-01-31 8:27 ` Guo Ren
2023-01-31 6:40 ` Björn Töpel
2023-01-31 8:15 ` Guo Ren
2023-01-31 10:56 ` Andrea Parri
2023-01-31 13:23 ` Guo Ren
2023-02-16 7:54 ` Björn Töpel
2023-02-17 2:28 ` Guo Ren
2023-02-17 7:32 ` Björn Töpel
2023-02-21 1:56 ` Guo Ren
2023-02-16 15:42 ` Masami Hiramatsu
2023-02-21 0:57 ` Guo Ren
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=87a61z2n55.fsf@all.your.base.are.belong.to.us \
--to=bjorn@kernel.org \
--cc=conor.dooley@microchip.com \
--cc=guoren@kernel.org \
--cc=guoren@linux.alibaba.com \
--cc=liaochang1@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=mhiramat@kernel.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=penberg@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).