| Message ID | 1a3567d5-e558-351a-c45d-73b2e5a8788c@loongson.cn (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Question about config UPROBES and UPROBE_EVENTS | expand |
On Wed, 11 Sep 2024 14:40:56 +0800 Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > Hi Masami, > > I am a little confused about config UPROBES and UPROBE_EVENTS. > Uprobes is the user-space counterpart to kprobes, I want to do > some small changes: > > (1) since config KPROBES can be selectable, just make config UPROBES > selectable too. > > (2) since config KPROBE_EVENTS depends on KPROBES rather than select > KPROBES, just make config UPROBE_EVENTS depends on UPROBES rather > than select UPROBES. > > Could you please let me know are you OK with the following changes? > If yes, I will send formal patches later. The difference between uprobes and kprobes is that kprobes can be enabled inside the kernel outside of kprobe events. Where as, uprobes is only enabled by uprobe events, so why have the separate option? That is, is there a reason to enable uprobes without enabling uprobe events? If you make uprobe events depend on uprobes, that may confuse people about how to enable uprobe events. Especially since it may break existing configs. kprobes have been around much longer than kprobe events. That's not the same with uprobes and uprobe events. They are much more coupled. -- Steve
On Wed, 11 Sep 2024 14:40:56 +0800 Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > Hi Masami, > > I am a little confused about config UPROBES and UPROBE_EVENTS. > Uprobes is the user-space counterpart to kprobes, I want to do > some small changes: > > (1) since config KPROBES can be selectable, just make config UPROBES > selectable too. > > (2) since config KPROBE_EVENTS depends on KPROBES rather than select > KPROBES, just make config UPROBE_EVENTS depends on UPROBES rather > than select UPROBES. > > Could you please let me know are you OK with the following changes? > If yes, I will send formal patches later. Hm, I don't completely reject this idea, but I'm not sure about the benefits to users and keeping backward compatibility. Especially, the latter one may hide uprobe_events by default. As you can see, the CONFIG_KPROBES is enabled by default, thus it does not hide the CONFIG_KPROBE_EVENTS. But the CONFIG_UPROBES is disabled by default and make CONFIG_UPROBE_EVENTS depending on it, the uprobe_events menu is hidden. I don't like this. Thank you, > > -- >8 -- > diff --git a/arch/Kconfig b/arch/Kconfig > index 975dd22a2dbd..5de2187d3440 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -124,7 +124,8 @@ config KPROBES_ON_FTRACE > optimize on top of function tracing. > > config UPROBES > - def_bool n > + bool "Uprobes" > + default n > depends on ARCH_SUPPORTS_UPROBES > help > Uprobes is the user-space counterpart to kprobes: they > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > index 721c3b221048..7db0462a5d11 100644 > --- a/kernel/trace/Kconfig > +++ b/kernel/trace/Kconfig > @@ -732,10 +732,9 @@ config KPROBE_EVENTS_ON_NOTRACE > > config UPROBE_EVENTS > bool "Enable uprobes-based dynamic events" > - depends on ARCH_SUPPORTS_UPROBES > + depends on UPROBES > depends on MMU > depends on PERF_EVENTS > - select UPROBES > select PROBE_EVENTS > select DYNAMIC_EVENTS > select TRACING > > Thanks, > Tiezhu > >
On 09/30/2024 07:15 AM, Masami Hiramatsu (Google) wrote: > On Wed, 11 Sep 2024 14:40:56 +0800 > Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > >> Hi Masami, >> >> I am a little confused about config UPROBES and UPROBE_EVENTS. >> Uprobes is the user-space counterpart to kprobes, I want to do >> some small changes: >> >> (1) since config KPROBES can be selectable, just make config UPROBES >> selectable too. >> >> (2) since config KPROBE_EVENTS depends on KPROBES rather than select >> KPROBES, just make config UPROBE_EVENTS depends on UPROBES rather >> than select UPROBES. >> >> Could you please let me know are you OK with the following changes? >> If yes, I will send formal patches later. > > Hm, I don't completely reject this idea, Thanks for your reply. I have almost dropped this idea due to my thoughtless after receiving the reply of Steven Rostedt [1]. > but I'm not sure about the benefits > to users and keeping backward compatibility. Yes, I think so too. > Especially, the latter one > may hide uprobe_events by default. Yes. > As you can see, the CONFIG_KPROBES is > enabled by default, thus it does not hide the CONFIG_KPROBE_EVENTS.But Maybe I missed something, AFAICT, the CONFIG_KPROBES is disabled by default, it needs to enable manually by the users, and also we can not see the CONFIG_KPROBE_EVENTS menu if CONFIG_KPROBES is not set because CONFIG_KPROBE_EVENTS depends on CONFIG_KPROBES. > the CONFIG_UPROBES is disabled by default and make CONFIG_UPROBE_EVENTS > depending on it, the uprobe_events menu is hidden. I don't like this. This is somehow like the current status of CONFIG_KPROBES and CONFIG_KPROBE_EVENTS. [1] https://lore.kernel.org/all/20240911094317.4a28fc3b@gandalf.local.home/ Thanks, Tiezhu
On Mon, 30 Sep 2024 09:33:42 +0800 Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > > the CONFIG_UPROBES is disabled by default and make CONFIG_UPROBE_EVENTS > > depending on it, the uprobe_events menu is hidden. I don't like this. > > This is somehow like the current status of CONFIG_KPROBES and > CONFIG_KPROBE_EVENTS. The question is, can uprobes be used without uprobe_events? With the current BPF work that I haven't been following, it may be possible now. If uprobes can be used without uprobe events, like kprobes can be used without kprobe events, then I can see having uprobes as a separate config menu option. If not, then no, it shouldn't be. -- Steve
On Mon, 30 Sep 2024 10:06:30 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Mon, 30 Sep 2024 09:33:42 +0800 > Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > > > > the CONFIG_UPROBES is disabled by default and make CONFIG_UPROBE_EVENTS > > > depending on it, the uprobe_events menu is hidden. I don't like this. > > > > This is somehow like the current status of CONFIG_KPROBES and > > CONFIG_KPROBE_EVENTS. > > The question is, can uprobes be used without uprobe_events? With the > current BPF work that I haven't been following, it may be possible now. uprobe_register/unregister APIs are exposed to the kernel modules, since systemtap had been introduced this feature. Thank you, > > If uprobes can be used without uprobe events, like kprobes can be used > without kprobe events, then I can see having uprobes as a separate config > menu option. If not, then no, it shouldn't be. > > -- Steve
On Tue, 1 Oct 2024 00:28:13 +0900 Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > On Mon, 30 Sep 2024 10:06:30 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > On Mon, 30 Sep 2024 09:33:42 +0800 > > Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > > > > > > the CONFIG_UPROBES is disabled by default and make CONFIG_UPROBE_EVENTS > > > > depending on it, the uprobe_events menu is hidden. I don't like this. > > > > > > This is somehow like the current status of CONFIG_KPROBES and > > > CONFIG_KPROBE_EVENTS. > > > > The question is, can uprobes be used without uprobe_events? With the > > current BPF work that I haven't been following, it may be possible now. > > uprobe_register/unregister APIs are exposed to the kernel modules, > since systemtap had been introduced this feature. > OK, but since they have always been visible, I would just make CONFIG_UPROBES a normal option and CONFIG_UPROBE_EVENTS select it if it gets selected, and not depend on it. -- Steve
On Mon, 30 Sep 2024 11:32:31 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Tue, 1 Oct 2024 00:28:13 +0900 > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > > On Mon, 30 Sep 2024 10:06:30 -0400 > > Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > On Mon, 30 Sep 2024 09:33:42 +0800 > > > Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > > > > > > > > the CONFIG_UPROBES is disabled by default and make CONFIG_UPROBE_EVENTS > > > > > depending on it, the uprobe_events menu is hidden. I don't like this. > > > > > > > > This is somehow like the current status of CONFIG_KPROBES and > > > > CONFIG_KPROBE_EVENTS. > > > > > > The question is, can uprobes be used without uprobe_events? With the > > > current BPF work that I haven't been following, it may be possible now. > > > > uprobe_register/unregister APIs are exposed to the kernel modules, > > since systemtap had been introduced this feature. > > > > OK, but since they have always been visible, I would just make > CONFIG_UPROBES a normal option and CONFIG_UPROBE_EVENTS select it if it > gets selected, and not depend on it. Agreed. Thanks,
On 9/30/24 23:43, Masami Hiramatsu (Google) wrote: > On Mon, 30 Sep 2024 11:32:31 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > >> On Tue, 1 Oct 2024 00:28:13 +0900 >> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: >> >>> On Mon, 30 Sep 2024 10:06:30 -0400 >>> Steven Rostedt <rostedt@goodmis.org> wrote: >>> >>>> On Mon, 30 Sep 2024 09:33:42 +0800 >>>> Tiezhu Yang <yangtiezhu@loongson.cn> wrote: >>>> >>>>>> the CONFIG_UPROBES is disabled by default and make CONFIG_UPROBE_EVENTS >>>>>> depending on it, the uprobe_events menu is hidden. I don't like this. >>>>> >>>>> This is somehow like the current status of CONFIG_KPROBES and >>>>> CONFIG_KPROBE_EVENTS. >>>> >>>> The question is, can uprobes be used without uprobe_events? With the >>>> current BPF work that I haven't been following, it may be possible now. >>> >>> uprobe_register/unregister APIs are exposed to the kernel modules, >>> since systemtap had been introduced this feature. >>> >> >> OK, but since they have always been visible, I would just make >> CONFIG_UPROBES a normal option and CONFIG_UPROBE_EVENTS select it if it >> gets selected, and not depend on it. > > Agreed. Thanks very much for your discussions. I agree with you. Then, CONFIG_KPROBE_EVENTS should depend on or select CONFIG_KPROBES? In the current code, CONFIG_KPROBE_EVENTS depend on CONFIG_KPROBES, the CONFIG_KPROBE_EVENTS menu is hidden if CONFIG_KPROBES is not set. Thanks, Tiezhu
On Tue, 1 Oct 2024 14:30:33 +0800 Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > Then, CONFIG_KPROBE_EVENTS should depend on or select CONFIG_KPROBES? > In the current code, CONFIG_KPROBE_EVENTS depend on CONFIG_KPROBES, > the CONFIG_KPROBE_EVENTS menu is hidden if CONFIG_KPROBES is not set. We could just for consistency. KPROBE_EVENTS would then need to depend on HAVE_KPROBES as well. It does add some duplication. -- Steve
On Mon, 30 Sep 2024 11:32:31 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > uprobe_register/unregister APIs are exposed to the kernel modules, > > since systemtap had been introduced this feature. > > > > OK, but since they have always been visible, I would just make > CONFIG_UPROBES a normal option and CONFIG_UPROBE_EVENTS select it if it > gets selected, and not depend on it. Thinking about this more, since systemtap is out of tree, and if that's the only user of uprobes, I don't think it should be exposed as a prompt. If you want uprobes for systemtap, you must also have uprobe events. -- Steve
On Tue, 1 Oct 2024 08:30:42 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Tue, 1 Oct 2024 14:30:33 +0800 > Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > > > Then, CONFIG_KPROBE_EVENTS should depend on or select CONFIG_KPROBES? > > In the current code, CONFIG_KPROBE_EVENTS depend on CONFIG_KPROBES, > > the CONFIG_KPROBE_EVENTS menu is hidden if CONFIG_KPROBES is not set. > > We could just for consistency. KPROBE_EVENTS would then need to depend on > HAVE_KPROBES as well. It does add some duplication. > I take this back. I don't think there's any reason to have a UPROBES prompt. If you want UPROBES, you should have UPROBE_EVENTS. They are completely different than kprobes. -- Steve
diff --git a/arch/Kconfig b/arch/Kconfig index 975dd22a2dbd..5de2187d3440 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -124,7 +124,8 @@ config KPROBES_ON_FTRACE optimize on top of function tracing. config UPROBES - def_bool n + bool "Uprobes" + default n depends on ARCH_SUPPORTS_UPROBES help Uprobes is the user-space counterpart to kprobes: they diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig index 721c3b221048..7db0462a5d11 100644 --- a/kernel/trace/Kconfig +++ b/kernel/trace/Kconfig @@ -732,10 +732,9 @@ config KPROBE_EVENTS_ON_NOTRACE config UPROBE_EVENTS bool "Enable uprobes-based dynamic events" - depends on ARCH_SUPPORTS_UPROBES + depends on UPROBES depends on MMU depends on PERF_EVENTS - select UPROBES select PROBE_EVENTS select DYNAMIC_EVENTS select TRACING
Hi Masami, I am a little confused about config UPROBES and UPROBE_EVENTS. Uprobes is the user-space counterpart to kprobes, I want to do some small changes: (1) since config KPROBES can be selectable, just make config UPROBES selectable too. (2) since config KPROBE_EVENTS depends on KPROBES rather than select KPROBES, just make config UPROBE_EVENTS depends on UPROBES rather than select UPROBES. Could you please let me know are you OK with the following changes? If yes, I will send formal patches later. -- >8 -- Thanks, Tiezhu