Re: [PATCH v2 01/12] uprobes: update outdated comment

2024-07-10 Thread Andrii Nakryiko
On Wed, Jul 10, 2024 at 6:33 AM Oleg Nesterov wrote: > > On 07/03, Oleg Nesterov wrote: > > > > > /* > > > -* The NULL 'tsk' here ensures that any faults that occur here > > > -* will not be accounted to the task. 'mm' *is* current->mm, > > > -* but we treat this as a 'remote'

Re: [PATCH v2 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-07-09 Thread Andrii Nakryiko
On Tue, Jul 9, 2024 at 2:33 PM Oleg Nesterov wrote: > > On 07/09, Andrii Nakryiko wrote: > > > > On Tue, Jul 9, 2024 at 11:49 AM Oleg Nesterov wrote: > > > > > > > Yep, that would be unfortunate (just like SIGILL sent when uretprobe > > >

Re: [PATCH v2 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-07-09 Thread Andrii Nakryiko
On Tue, Jul 9, 2024 at 11:49 AM Oleg Nesterov wrote: > > On 07/08, Andrii Nakryiko wrote: > > > > On Sun, Jul 7, 2024 at 7:48 AM Oleg Nesterov wrote: > > > > > > And I forgot to mention... > > > > > > In any case __uprobe_unregister() can't

Re: [PATCH v2 05/12] uprobes: move offset and ref_ctr_offset into uprobe_consumer

2024-07-08 Thread Andrii Nakryiko
On Sun, Jul 7, 2024 at 5:50 AM Oleg Nesterov wrote: > > On 07/01, Andrii Nakryiko wrote: > > > > --- a/include/linux/uprobes.h > > +++ b/include/linux/uprobes.h > > @@ -42,6 +42,11 @@ struct uprobe_consumer { > >

Re: [PATCH v2 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-07-08 Thread Andrii Nakryiko
il to understand it. It looks > > obvioulsy wrong to me, see below. > > > > I tend to agree with the comments from Peter, but lets ignore them > > for the moment. > > > > On 07/01, Andrii Nakryiko wrote: > > > > > > static

Re: [PATCH v2 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-07-08 Thread Andrii Nakryiko
On Fri, Jul 5, 2024 at 8:38 AM Oleg Nesterov wrote: > > Tried to read this patch, but I fail to understand it. It looks > obvioulsy wrong to me, see below. > > I tend to agree with the comments from Peter, but lets ignore them > for the moment. > > On 07/01, Andrii Nakryik

Re: [PATCH v2 01/12] uprobes: update outdated comment

2024-07-03 Thread Andrii Nakryiko
So you still have time. I appreciate you looking at it, though! > On 07/01, Andrii Nakryiko wrote: > > > > There is no task_struct passed into get_user_pages_remote() anymore, > > drop the parts of comment mentioning NULL tsk, it's just confusing at > > this point. > >

Re: [PATCH v2 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-07-03 Thread Andrii Nakryiko
On Wed, Jul 3, 2024 at 6:36 AM Peter Zijlstra wrote: > > On Mon, Jul 01, 2024 at 03:39:27PM -0700, Andrii Nakryiko wrote: > > > One, attempted initially, way to solve this is through using > > atomic_inc_not_zero() approach, turning get_uprobe() in

Re: [PATCH v2 02/12] uprobes: correct mmap_sem locking assumptions in uprobe_write_opcode()

2024-07-03 Thread Andrii Nakryiko
On Wed, Jul 3, 2024 at 6:15 AM Masami Hiramatsu wrote: > > On Mon, 1 Jul 2024 15:39:25 -0700 > Andrii Nakryiko wrote: > > > It seems like uprobe_write_opcode() doesn't require writer locked > > mmap_sem, any lock (reader or writer) should be sufficient. This was > &g

Re: [PATCH v2 01/12] uprobes: update outdated comment

2024-07-03 Thread Andrii Nakryiko
On Wed, Jul 3, 2024 at 4:40 AM Oleg Nesterov wrote: > > Sorry for the late reply. I'll try to read this version/discussion > when I have time... yes, I have already promised this before, sorry :/ > > On 07/01, Andrii Nakryiko wrote: > > > > The

Re: [PATCH v2 05/12] uprobes: move offset and ref_ctr_offset into uprobe_consumer

2024-07-03 Thread Andrii Nakryiko
On Wed, Jul 3, 2024 at 3:13 AM Masami Hiramatsu wrote: > > On Wed, 3 Jul 2024 10:13:15 +0200 > Peter Zijlstra wrote: > > > On Mon, Jul 01, 2024 at 03:39:28PM -0700, Andrii Nakryiko wrote: > > > Simplify uprobe registration/unregistration interfaces by making offset &g

Re: [PATCH 06/12] uprobes: add batch uprobe register/unregister APIs

2024-07-02 Thread Andrii Nakryiko
On Tue, Jul 2, 2024 at 9:53 AM Steven Rostedt wrote: > > On Wed, 3 Jul 2024 00:19:05 +0900 > Masami Hiramatsu (Google) wrote: > > > > BTW, is this (batched register/unregister APIs) something you'd like > > > to use from the tracefs-based (or whatever it's called, I mean non-BPF > > > ones)

Re: [PATCH v2 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-07-02 Thread Andrii Nakryiko
On Tue, Jul 2, 2024 at 3:23 AM Peter Zijlstra wrote: > > On Mon, Jul 01, 2024 at 03:39:27PM -0700, Andrii Nakryiko wrote: > > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > > index 23449a8c5e7e..560cf1ca512a 100644 > > --- a/kernel/events/uprobe

Re: [PATCH 06/12] uprobes: add batch uprobe register/unregister APIs

2024-07-01 Thread Andrii Nakryiko
On Mon, Jul 1, 2024 at 6:01 PM Masami Hiramatsu wrote: > > On Mon, 1 Jul 2024 15:15:56 -0700 > Andrii Nakryiko wrote: > > > On Mon, Jul 1, 2024 at 10:55 AM Andrii Nakryiko > > wrote: > > > > > > On Sat, Jun 29, 2024 at 4:30 PM Masami Hiramatsu > &

[PATCH] perf,x86: avoid missing caller address in stack traces captured in uprobe

2024-07-01 Thread Andrii Nakryiko
n address is still pointed to by %rsp, so we fetch it and add to stack trace before proceeding to unwind the rest using frame pointer-based logic. Signed-off-by: Andrii Nakryiko --- arch/x86/events/core.c | 20 include/linux/uprobes.h | 2 ++ kernel/events/uprobes.c | 2 ++

[PATCH v2 12/12] uprobes: switch uprobes_treelock to per-CPU RW semaphore

2024-07-01 Thread Andrii Nakryiko
and this will hopefully be address in follow up patch set(s). Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index bb480a2400e1..1d76551e5e23

[PATCH v2 11/12] uprobes,bpf: switch to batch uprobe APIs for BPF multi-uprobes

2024-07-01 Thread Andrii Nakryiko
Switch internals of BPF multi-uprobes to batched version of uprobe registration and unregistration APIs. This also simplifies BPF clean up code a bit thanks to all-or-nothing guarantee of uprobes_register_batch(). Signed-off-by: Andrii Nakryiko --- kernel/trace/bpf_trace.c | 23

[PATCH v2 10/12] uprobes: improve lock batching for uprobe_unregister_batch

2024-07-01 Thread Andrii Nakryiko
() for all uprobe_consumers. That uprobe_consumer->uprobe field is really handy in helping with this. Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c in

[PATCH v2 09/12] uprobes: batch uprobes_treelock during registration

2024-07-01 Thread Andrii Nakryiko
lower-level __insert_uprobe() helper. Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 45 + 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 128677ffe662..ced85284bbf4 100644

[PATCH v2 08/12] uprobes: split uprobe allocation and uprobes_tree insertion steps

2024-07-01 Thread Andrii Nakryiko
to batch and optimize locking per each phase. Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 0f928a72a658..128677ffe662 100644 --- a/kernel

[PATCH v2 07/12] uprobes: inline alloc_uprobe() logic into __uprobe_register()

2024-07-01 Thread Andrii Nakryiko
To allow unbundling alloc-uprobe-and-insert step which is currently tightly coupled, inline alloc_uprobe() logic into uprobe_register_batch() loop. It's called from one place, so we don't really lose much in terms of maintainability. No functional changes. Signed-off-by: Andrii Nakryiko

[PATCH v2 06/12] uprobes: add batch uprobe register/unregister APIs

2024-07-01 Thread Andrii Nakryiko
on) uprobe instances, as necessary. And then the last step is actual uprobe registration for all affected VMAs. We take care to undo all the actions in the event of an error at any point in this lengthy process, so end result is all-or-nothing, as described above. Signed-off-by: Andrii Nakryiko

[PATCH v2 05/12] uprobes: move offset and ref_ctr_offset into uprobe_consumer

2024-07-01 Thread Andrii Nakryiko
also makes uprobe_register_refctr() unnecessary, so remove it and simplify consumers. No functional changes intended. Acked-by: Masami Hiramatsu (Google) Signed-off-by: Andrii Nakryiko --- include/linux/uprobes.h | 18 +++ kernel/events/uprobes.c | 19 ++- ke

[PATCH v2 03/12] uprobes: simplify error handling for alloc_uprobe()

2024-07-01 Thread Andrii Nakryiko
Return -ENOMEM instead of NULL, which makes caller's error handling just a touch simpler. Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index f87049c08ee9

[PATCH v2 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-07-01 Thread Andrii Nakryiko
istration and unregistration, so we remove the retry logic completely. Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 260 ++-- 1 file changed, 195 insertions(+), 65 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c i

[PATCH v2 02/12] uprobes: correct mmap_sem locking assumptions in uprobe_write_opcode()

2024-07-01 Thread Andrii Nakryiko
this clearly. [0] https://lore.kernel.org/linux-trace-kernel/20240625190748.gc14...@redhat.com/ Fixes: 29dedee0e693 ("uprobes: Add mem_cgroup_charge_anon() into uprobe_write_opcode()") Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 2 +- 1 file changed, 1 insertion(+),

[PATCH v2 01/12] uprobes: update outdated comment

2024-07-01 Thread Andrii Nakryiko
There is no task_struct passed into get_user_pages_remote() anymore, drop the parts of comment mentioning NULL tsk, it's just confusing at this point. Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel

[PATCH v2 00/12] uprobes: add batched register/unregister APIs and per-CPU RW semaphore

2024-07-01 Thread Andrii Nakryiko
ted UPROBE_REFCNT_* constants to be more meaningful (Oleg); - dropped the "fix" to switch to write-protected mmap_sem, adjusted invalid comment instead (Oleg). Andrii Nakryiko (12): uprobes: update outdated comment uprobes: correct mmap_sem locking assumptions in uprobe

Re: [PATCH 06/12] uprobes: add batch uprobe register/unregister APIs

2024-07-01 Thread Andrii Nakryiko
On Mon, Jul 1, 2024 at 10:55 AM Andrii Nakryiko wrote: > > On Sat, Jun 29, 2024 at 4:30 PM Masami Hiramatsu wrote: > > > > On Fri, 28 Jun 2024 09:34:26 -0700 > > Andrii Nakryiko wrote: > > > > > On Thu, Jun 27, 2024 at 11:28 PM Masami Hiramatsu > >

Re: [PATCH 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-07-01 Thread Andrii Nakryiko
On Thu, Jun 27, 2024 at 9:43 AM Andrii Nakryiko wrote: > > On Wed, Jun 26, 2024 at 7:30 PM Masami Hiramatsu wrote: > > > > On Mon, 24 Jun 2024 17:21:36 -0700 > > Andrii Nakryiko wrote: > > > > > Anyways, under exclusive writer lock, we double-check that re

Re: [PATCH 06/12] uprobes: add batch uprobe register/unregister APIs

2024-07-01 Thread Andrii Nakryiko
On Sat, Jun 29, 2024 at 4:30 PM Masami Hiramatsu wrote: > > On Fri, 28 Jun 2024 09:34:26 -0700 > Andrii Nakryiko wrote: > > > On Thu, Jun 27, 2024 at 11:28 PM Masami Hiramatsu > > wrote: > > > > > > On Thu, 27 Jun 2024 09:47:10 -0700 > > > A

Re: [PATCH 06/12] uprobes: add batch uprobe register/unregister APIs

2024-06-28 Thread Andrii Nakryiko
On Thu, Jun 27, 2024 at 11:28 PM Masami Hiramatsu wrote: > > On Thu, 27 Jun 2024 09:47:10 -0700 > Andrii Nakryiko wrote: > > > On Thu, Jun 27, 2024 at 6:04 AM Masami Hiramatsu > > wrote: > > > > > > On Mon, 24 Jun 2024 17:21:38 -0700 > > >

Re: [PATCH 06/12] uprobes: add batch uprobe register/unregister APIs

2024-06-27 Thread Andrii Nakryiko
On Thu, Jun 27, 2024 at 6:04 AM Masami Hiramatsu wrote: > > On Mon, 24 Jun 2024 17:21:38 -0700 > Andrii Nakryiko wrote: > > > -static int __uprobe_register(struct inode *inode, loff_t offset, > > - loff_t ref_ctr_offset, struct uprobe_con

Re: [PATCH 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-06-27 Thread Andrii Nakryiko
On Wed, Jun 26, 2024 at 7:30 PM Masami Hiramatsu wrote: > > On Mon, 24 Jun 2024 17:21:36 -0700 > Andrii Nakryiko wrote: > > > Anyways, under exclusive writer lock, we double-check that refcount > > didn't change and is still zero. If it is, we proceed with destruction, &

Re: [PATCH 06/12] uprobes: add batch uprobe register/unregister APIs

2024-06-26 Thread Andrii Nakryiko
On Wed, Jun 26, 2024 at 4:27 AM Jiri Olsa wrote: > > On Mon, Jun 24, 2024 at 05:21:38PM -0700, Andrii Nakryiko wrote: > > SNIP > > > + for (i = 0; i < cnt; i++) { > > + uc = get_uprobe_consumer(i, ctx); > > + > > + /* Each co

Re: [PATCH 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-06-26 Thread Andrii Nakryiko
t-patch#_base_tree_information] > > url: > https://github.com/intel-lab-lkp/linux/commits/Andrii-Nakryiko/uprobes-update-outdated-comment/20240626-001728 > base: next-20240624 > patch link: > https://lore.kernel.org/r/20240625002144.3485799-5-andrii%40kernel.org > pat

Re: [PATCH 02/12] uprobes: grab write mmap lock in unapply_uprobe()

2024-06-26 Thread Andrii Nakryiko
On Tue, Jun 25, 2024 at 12:09 PM Oleg Nesterov wrote: > > On 06/25, Andrii Nakryiko wrote: > > > > On Tue, Jun 25, 2024 at 7:51 AM Oleg Nesterov wrote: > > > > > > Why? > > > > > > So far I don't understand this change. Quite possibly I

Re: [PATCH 02/12] uprobes: grab write mmap lock in unapply_uprobe()

2024-06-25 Thread Andrii Nakryiko
On Tue, Jun 25, 2024 at 7:51 AM Oleg Nesterov wrote: > > On 06/25, Masami Hiramatsu wrote: > > > > On Mon, 24 Jun 2024 17:21:34 -0700 > > Andrii Nakryiko wrote: > > > > > Given unapply_uprobe() can call remove_breakpoint() which eventually > > &g

Re: [PATCH 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-06-25 Thread Andrii Nakryiko
untouched for many years now. But I'd really appreciate if you can give it a through review anyways! > > On 06/24, Andrii Nakryiko wrote: > > > > + * UPROBE_REFCNT_GET constant is chosen such that it will *increment both* > > + * epoch and refcnt parts atomically with one atomic

Re: [PATCH v2 4/4] selftests/bpf: add test validating uprobe/uretprobe stack traces

2024-06-24 Thread Andrii Nakryiko
On Mon, Jun 24, 2024 at 6:14 PM Masami Hiramatsu wrote: > > On Tue, 21 May 2024 18:38:45 -0700 > Andrii Nakryiko wrote: > > > Add a set of tests to validate that stack traces captured from or in the > > presence of active uprobes and uretprobes are valid and complete. >

Re: [PATCH v2 2/4] perf,uprobes: fix user stack traces in the presence of pending uretprobes

2024-06-24 Thread Andrii Nakryiko
On Mon, Jun 24, 2024 at 5:39 PM Masami Hiramatsu wrote: > > On Mon, 24 Jun 2024 13:32:35 -0700 > Andrii Nakryiko wrote: > > > On Mon, Jun 17, 2024 at 3:37 PM Andrii Nakryiko > > wrote: > > > > > > On Tue, Jun 4, 2024 at 10:16 AM Andrii Nakryiko > &g

[PATCH 11/12] uprobes,bpf: switch to batch uprobe APIs for BPF multi-uprobes

2024-06-24 Thread Andrii Nakryiko
Switch internals of BPF multi-uprobes to batched version of uprobe registration and unregistration APIs. This also simplifies BPF clean up code a bit thanks to all-or-nothing guarantee of uprobes_register_batch(). Signed-off-by: Andrii Nakryiko --- kernel/trace/bpf_trace.c | 23

[PATCH 12/12] uprobes: switch uprobes_treelock to per-CPU RW semaphore

2024-06-24 Thread Andrii Nakryiko
and this will hopefully be address in follow up patch set(s). Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 7e94671a672a..e24b81b0e149

[PATCH 09/12] uprobes: batch uprobes_treelock during registration

2024-06-24 Thread Andrii Nakryiko
lower-level __insert_uprobe() helper. Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 45 + 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 5e98e179d47d..416f408cbed9 100644

[PATCH 10/12] uprobes: improve lock batching for uprobe_unregister_batch

2024-06-24 Thread Andrii Nakryiko
() for all uprobe_consumers. That uprobe_consumer->uprobe field is really handy in helping with this. Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c in

[PATCH 08/12] uprobes: split uprobe allocation and uprobes_tree insertion steps

2024-06-24 Thread Andrii Nakryiko
to batch and optimize locking per each phase. Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index ebd8511b6eb2..5e98e179d47d 100644

[PATCH 06/12] uprobes: add batch uprobe register/unregister APIs

2024-06-24 Thread Andrii Nakryiko
on) uprobe instances, as necessary. And then the last step is actual uprobe registration for all affected VMAs. We take care to undo all the actions in the event of an error at any point in this lengthy process, so end result is all-or-nothing, as described above. Signed-off-by: Andrii Nakryiko

[PATCH 07/12] uprobes: inline alloc_uprobe() logic into __uprobe_register()

2024-06-24 Thread Andrii Nakryiko
To allow unbundling alloc-uprobe-and-insert step which is currently tightly coupled, inline alloc_uprobe() logic into uprobe_register_batch() loop. It's called from one place, so we don't really lose much in terms of maintainability. No functional changes. Signed-off-by: Andrii Nakryiko

[PATCH 05/12] uprobes: move offset and ref_ctr_offset into uprobe_consumer

2024-06-24 Thread Andrii Nakryiko
also makes uprobe_register_refctr() unnecessary, so remove it and simplify consumers. No functional changes intended. Signed-off-by: Andrii Nakryiko --- include/linux/uprobes.h | 18 +++ kernel/events/uprobes.c | 19 ++- kernel/trace/bpf_trace.c

[PATCH 03/12] uprobes: simplify error handling for alloc_uprobe()

2024-06-24 Thread Andrii Nakryiko
Return -ENOMEM instead of NULL, which makes caller's error handling just a touch simpler. Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index e896eeecb091

[PATCH 04/12] uprobes: revamp uprobe refcounting and lifetime management

2024-06-24 Thread Andrii Nakryiko
o by any other CPU). We also document why it's safe to do unconditional __get_uprobe() at all call sites, to make it clear that we maintain the above invariants. Note also, we now don't have a race between registration and unregistration, so we remove the retry logic completely. Signed-o

[PATCH 02/12] uprobes: grab write mmap lock in unapply_uprobe()

2024-06-24 Thread Andrii Nakryiko
aec ("uprobes: Teach handler_chain() to filter out the probed task") Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 197fbe4663b5..e896eeecb091 100644

[PATCH 01/12] uprobes: update outdated comment

2024-06-24 Thread Andrii Nakryiko
There is no task_struct passed into get_user_pages_remote() anymore, drop the parts of comment mentioning NULL tsk, it's just confusing at this point. Signed-off-by: Andrii Nakryiko --- kernel/events/uprobes.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel

[PATCH 00/12] uprobes: add batched register/unregister APIs and per-CPU RW semaphore

2024-06-24 Thread Andrii Nakryiko
uprobe APIs. As mentioned, a very straightforward patch #12 takes advantage of all the prep work and just switches uprobes_treelock to per-CPU RW semaphore. Andrii Nakryiko (12): uprobes: update outdated comment uprobes: grab write mmap lock in unapply_uprobe() uprobes: simplify error handling

Re: [PATCH v2 2/4] perf,uprobes: fix user stack traces in the presence of pending uretprobes

2024-06-24 Thread Andrii Nakryiko
On Mon, Jun 17, 2024 at 3:37 PM Andrii Nakryiko wrote: > > On Tue, Jun 4, 2024 at 10:16 AM Andrii Nakryiko > wrote: > > > > On Tue, Jun 4, 2024 at 7:13 AM Masami Hiramatsu wrote: > > > > > > On Tue, 21 May 2024 18:38:43 -0700 > > > Andrii Nakryi

Re: [PATCH v2 2/4] perf,uprobes: fix user stack traces in the presence of pending uretprobes

2024-06-17 Thread Andrii Nakryiko
On Tue, Jun 4, 2024 at 10:16 AM Andrii Nakryiko wrote: > > On Tue, Jun 4, 2024 at 7:13 AM Masami Hiramatsu wrote: > > > > On Tue, 21 May 2024 18:38:43 -0700 > > Andrii Nakryiko wrote: > > > > > When kernel has pending uretprobes installed, it hijacks orig

Re: [PATCH v2 2/4] perf,uprobes: fix user stack traces in the presence of pending uretprobes

2024-06-04 Thread Andrii Nakryiko
On Tue, Jun 4, 2024 at 7:13 AM Masami Hiramatsu wrote: > > On Tue, 21 May 2024 18:38:43 -0700 > Andrii Nakryiko wrote: > > > When kernel has pending uretprobes installed, it hijacks original user > > function return address on the stack with a uretprobe trampoline

Re: [PATCH v2 3/4] perf,x86: avoid missing caller address in stack traces captured in uprobe

2024-06-04 Thread Andrii Nakryiko
On Tue, Jun 4, 2024 at 7:06 AM Masami Hiramatsu wrote: > > On Tue, 21 May 2024 18:38:44 -0700 > Andrii Nakryiko wrote: > > > When tracing user functions with uprobe functionality, it's common to > > install the probe (e.g., a BPF program) at the first instruc

Re: [PATCH v2 0/4] Fix user stack traces captured from uprobes

2024-06-03 Thread Andrii Nakryiko
On Tue, May 21, 2024 at 6:38 PM Andrii Nakryiko wrote: > > This patch set reports two issues with captured stack traces. > > First issue, fixed in patch #2, deals with fixing up uretprobe trampoline > addresses in captured stack trace. This issue happens when there are pending

[PATCH v2 4/4] selftests/bpf: add test validating uprobe/uretprobe stack traces

2024-05-21 Thread Andrii Nakryiko
(in target_1) ENTRY #5: 0x75922c (in caller) ENTRY #6: 0x6f8f39 ENTRY #7: 0x6fa6f0 ENTRY #8: 0x7f986adc4cd0 Now there is a logical and complete sequence of function calls. Signed-off-by: Andrii Nakryiko --- .../bpf/prog_tests/uretprobe_stack.c | 186 ++ .../selftests/bpf/progs

[PATCH v2 3/4] perf,x86: avoid missing caller address in stack traces captured in uprobe

2024-05-21 Thread Andrii Nakryiko
ing frame pointer-based logic. Signed-off-by: Andrii Nakryiko --- arch/x86/events/core.c | 20 include/linux/uprobes.h | 2 ++ kernel/events/uprobes.c | 2 ++ 3 files changed, 24 insertions(+) diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index 5b0dd07b1ef1..82d

[PATCH v2 2/4] perf,uprobes: fix user stack traces in the presence of pending uretprobes

2024-05-21 Thread Andrii Nakryiko
im Signed-off-by: Andrii Nakryiko --- kernel/events/callchain.c | 43 ++- kernel/events/uprobes.c | 9 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/kernel/events/callchain.c b/kernel/events/callchain.c index 1273be84392c..b17e3323f

[PATCH v2 1/4] uprobes: rename get_trampoline_vaddr() and make it global

2024-05-21 Thread Andrii Nakryiko
This helper is needed in another file, so make it a bit more uniquely named and expose it internally. Signed-off-by: Andrii Nakryiko --- include/linux/uprobes.h | 1 + kernel/events/uprobes.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/linux/uprobes.h b

[PATCH v2 0/4] Fix user stack traces captured from uprobes

2024-05-21 Thread Andrii Nakryiko
selftests changes and can go in through non-BPF tree without the risk of merge conflicts. Patches are based on latest linux-trace/probes/for-next. v1->v2: - fixed GCC aggressively inlining test_uretprobe_stack() function (BPF CI); - fixed comments (Peter). Andrii Nakryiko (4): uprobes: ren

[PATCH] uprobes: prevent mutex_lock() under rcu_read_lock()

2024-05-20 Thread Andrii Nakryiko
prepare uprobe args buffer lazily") Reported-by: Breno Leitao Signed-off-by: Andrii Nakryiko --- kernel/trace/trace_uprobe.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index 8541fa1494ae..

Re: [PATCH 2/4] perf,uprobes: fix user stack traces in the presence of pending uretprobes

2024-05-20 Thread Andrii Nakryiko
On Mon, May 20, 2024 at 8:20 AM Jiri Olsa wrote: > > On Wed, May 15, 2024 at 08:32:30AM -0600, Andrii Nakryiko wrote: > > On Wed, May 15, 2024 at 3:30 AM Peter Zijlstra wrote: > > > > > > On Wed, May 08, 2024 at 02:26:03PM -0700, Andrii Nakryiko wro

Re: [PATCH 2/4] perf,uprobes: fix user stack traces in the presence of pending uretprobes

2024-05-15 Thread Andrii Nakryiko
On Wed, May 15, 2024 at 3:30 AM Peter Zijlstra wrote: > > On Wed, May 08, 2024 at 02:26:03PM -0700, Andrii Nakryiko wrote: > > > +static void fixup_uretprobe_trampoline_entries(struct perf_callchain_entry > > *entry, > > +

[PATCH 4/4] selftests/bpf: add test validating uprobe/uretprobe stack traces

2024-05-08 Thread Andrii Nakryiko
(in target_1) ENTRY #5: 0x75922c (in caller) ENTRY #6: 0x6f8f39 ENTRY #7: 0x6fa6f0 ENTRY #8: 0x7f986adc4cd0 Now there is a logical and complete sequence of function calls. Signed-off-by: Andrii Nakryiko --- .../bpf/prog_tests/uretprobe_stack.c | 185 ++ .../selftests/bpf/progs

[PATCH 3/4] perf,x86: avoid missing caller address in stack traces captured in uprobe

2024-05-08 Thread Andrii Nakryiko
ing frame pointer-based logic. Signed-off-by: Andrii Nakryiko --- arch/x86/events/core.c | 20 include/linux/uprobes.h | 2 ++ kernel/events/uprobes.c | 2 ++ 3 files changed, 24 insertions(+) diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index 5b0dd07b1ef1..82d

[PATCH 2/4] perf,uprobes: fix user stack traces in the presence of pending uretprobes

2024-05-08 Thread Andrii Nakryiko
ne address entries with correct original return address. This is done only if there are pending uretprobes for current task. Reported-by: Riham Selim Signed-off-by: Andrii Nakryiko --- kernel/events/callchain.c | 42 ++- kernel/events/uprobes.c | 9

[PATCH 1/4] uprobes: rename get_trampoline_vaddr() and make it global

2024-05-08 Thread Andrii Nakryiko
This helper is needed in another file, so make it a bit more uniquely named and expose it internally. Signed-off-by: Andrii Nakryiko --- include/linux/uprobes.h | 1 + kernel/events/uprobes.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/linux/uprobes.h b

[PATCH 0/4] Fix user stack traces captured from uprobes

2024-05-08 Thread Andrii Nakryiko
selftests, is isolated from any other BPF selftests changes and can go in through non-BPF tree without the risk of merge conflicts. Patches are based on latest linux-trace's probes/for-next branch. Andrii Nakryiko (4): uprobes: rename get_trampoline_vaddr() and make it global perf,uprobes: fix user

Re: [PATCH RFC] rethook: inline arch_rethook_trampoline_callback() in assembly code

2024-04-29 Thread Andrii Nakryiko
On Wed, Apr 24, 2024 at 5:02 PM Andrii Nakryiko wrote: > > At the lowest level, rethook-based kretprobes on x86-64 architecture go > through arch_rethoook_trampoline() function, manually written in > assembly, which calls into a simple arch_rethook_trampoline_callback() > functio

Re: [PATCH 0/2] Objpool performance improvements

2024-04-26 Thread Andrii Nakryiko
On Fri, Apr 26, 2024 at 7:25 AM Masami Hiramatsu wrote: > > Hi Andrii, > > On Wed, 24 Apr 2024 14:52:12 -0700 > Andrii Nakryiko wrote: > > > Improve objpool (used heavily in kretprobe hot path) performance with two > > improvements: > > - inlini

[PATCH RFC] rethook: inline arch_rethook_trampoline_callback() in assembly code

2024-04-24 Thread Andrii Nakryiko
change is acceptable and whether I should complete it with 32-bit "inlining" as well. Thanks! [0] https://nakryiko.com/posts/retsnoop-intro/#peering-deep-into-functions-with-lbr Signed-off-by: Andrii Nakryiko --- arch/x86/kernel/asm-offsets_64.c | 4 arch/x8

[PATCH 2/2] objpool: cache nr_possible_cpus() and avoid caching nr_cpu_ids

2024-04-24 Thread Andrii Nakryiko
-multi: 10.440 ± 0.108M/s AFTER = kretprobe : 10.106 ± 0.120M/s (+1.7%) kretprobe-multi: 10.515 ± 0.180M/s (+0.7%) Cc: Matt (Qiang) Wu Signed-off-by: Andrii Nakryiko --- include/linux/objpool.h | 6 +++--- lib/objpool.c | 12 ++-- 2 files changed, 9 insertions

[PATCH 1/2] objpool: enable inlining objpool_push() and objpool_pop() operations

2024-04-24 Thread Andrii Nakryiko
: 10.440 ± 0.108M/s (+2.2%) Cc: Matt (Qiang) Wu Signed-off-by: Andrii Nakryiko --- include/linux/objpool.h | 101 +++- lib/objpool.c | 100 --- 2 files changed, 99 insertions(+), 102 deletions(-) diff --git

[PATCH 0/2] Objpool performance improvements

2024-04-24 Thread Andrii Nakryiko
kprobes and kretprobes with BPF-based benchmarks. See individual patches for details and results. Andrii Nakryiko (2): objpool: enable inlining objpool_push() and objpool_pop() operations objpool: cache nr_possible_cpus() and avoid caching nr_cpu_ids include/linux/objpool.h | 105

Re: [PATCH v4 2/2] rethook: honor CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING in rethook_try_get()

2024-04-19 Thread Andrii Nakryiko
On Thu, Apr 18, 2024 at 6:00 PM Masami Hiramatsu wrote: > > On Thu, 18 Apr 2024 12:09:09 -0700 > Andrii Nakryiko wrote: > > > Take into account CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING when validating > > that RCU is watching when trying to setup rethooko on a function en

[PATCH v4 2/2] rethook: honor CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING in rethook_try_get()

2024-04-18 Thread Andrii Nakryiko
://lore.kernel.org/bpf/caef4bzauq2wkmjzdc9s0rbwa01bybgwhn6andxqshyia47p...@mail.gmail.com/ Signed-off-by: Andrii Nakryiko --- kernel/trace/rethook.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c index fa03094e9e69..a974605ad7a5 100644 --- a/kernel

[PATCH v4 1/2] ftrace: make extra rcu_is_watching() validation check optional

2024-04-18 Thread Andrii Nakryiko
/bpf/caef4bzauq2wkmjzdc9s0rbwa01bybgwhn6andxqshyia47p...@mail.gmail.com/ Cc: Steven Rostedt Cc: Masami Hiramatsu Cc: Paul E. McKenney Acked-by: Masami Hiramatsu (Google) Signed-off-by: Andrii Nakryiko --- include/linux/trace_recursion.h | 2 +- kernel/trace/Kconfig| 13

Re: [PATCH v3 2/2] rethook: honor CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING in rethook_try_get()

2024-04-18 Thread Andrii Nakryiko
On Tue, Apr 9, 2024 at 3:48 PM Masami Hiramatsu wrote: > > On Wed, 3 Apr 2024 15:03:28 -0700 > Andrii Nakryiko wrote: > > > Take into account CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING when validating > > that RCU is watching when trying to setup rethooko on a function ent

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-06 Thread Andrii Nakryiko
On Fri, Apr 5, 2024 at 8:41 PM Masami Hiramatsu wrote: > > On Tue, 2 Apr 2024 22:21:00 -0700 > Andrii Nakryiko wrote: > > > On Tue, Apr 2, 2024 at 9:00 PM Andrii Nakryiko > > wrote: > > > > > > On Tue, Apr 2, 2024 at 5:52 PM Steven Rostedt wrote: &

[PATCH v3 2/2] rethook: honor CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING in rethook_try_get()

2024-04-03 Thread Andrii Nakryiko
%, according to BPF benchmarks ([0]). [0] https://lore.kernel.org/bpf/caef4bzauq2wkmjzdc9s0rbwa01bybgwhn6andxqshyia47p...@mail.gmail.com/ Signed-off-by: Andrii Nakryiko --- kernel/trace/rethook.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c

[PATCH v3 1/2] ftrace: make extra rcu_is_watching() validation check optional

2024-04-03 Thread Andrii Nakryiko
/bpf/caef4bzauq2wkmjzdc9s0rbwa01bybgwhn6andxqshyia47p...@mail.gmail.com/ Cc: Steven Rostedt Cc: Masami Hiramatsu Cc: Paul E. McKenney Signed-off-by: Andrii Nakryiko --- include/linux/trace_recursion.h | 2 +- kernel/trace/Kconfig| 13 + 2 files changed, 14 insertions

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-02 Thread Andrii Nakryiko
On Tue, Apr 2, 2024 at 9:00 PM Andrii Nakryiko wrote: > > On Tue, Apr 2, 2024 at 5:52 PM Steven Rostedt wrote: > > > > On Wed, 3 Apr 2024 09:40:48 +0900 > > Masami Hiramatsu (Google) wrote: > > > > > OK, for me, this last sentence is preferred for

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-02 Thread Andrii Nakryiko
On Tue, Apr 2, 2024 at 5:52 PM Steven Rostedt wrote: > > On Wed, 3 Apr 2024 09:40:48 +0900 > Masami Hiramatsu (Google) wrote: > > > OK, for me, this last sentence is preferred for the help message. That > > explains > > what this is for. > > > > All callbacks that attach to the function

Re: [PATCH bpf-next] rethook: Remove warning messages printed for finding return address of a frame.

2024-04-02 Thread Andrii Nakryiko
_running(tsk)) > return 0; > This should probably go through Masami's tree, but the change makes sense to me, given this is an expected condition. Acked-by: Andrii Nakryiko > do { > -- > 2.34.1 > >

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-01 Thread Andrii Nakryiko
On Mon, Apr 1, 2024 at 5:38 PM Masami Hiramatsu wrote: > > On Mon, 1 Apr 2024 12:09:18 -0400 > Steven Rostedt wrote: > > > On Mon, 1 Apr 2024 20:25:52 +0900 > > Masami Hiramatsu (Google) wrote: > > > > > > Masami, > > > > > > > > Are you OK with just keeping it set to N. > > > > > > OK, if it

[PATCH v2] ftrace: make extra rcu_is_watching() validation check optional

2024-04-01 Thread Andrii Nakryiko
of ftrace subsystem. For most users it should probably be kept disabled to eliminate unnecessary runtime overhead. Cc: Steven Rostedt Cc: Masami Hiramatsu Cc: Paul E. McKenney Signed-off-by: Andrii Nakryiko --- include/linux/trace_recursion.h | 2 +- kernel/trace/Kconfig| 14

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-29 Thread Andrii Nakryiko
On Tue, Mar 26, 2024 at 11:58 AM Steven Rostedt wrote: > > On Tue, 26 Mar 2024 09:16:33 -0700 > Andrii Nakryiko wrote: > > > > It's no different than lockdep. Test boxes should have it enabled, but > > > there's no reason to have this enabled in a production system.

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-26 Thread Andrii Nakryiko
On Mon, Mar 25, 2024 at 3:11 PM Steven Rostedt wrote: > > On Mon, 25 Mar 2024 11:38:48 +0900 > Masami Hiramatsu (Google) wrote: > > > On Fri, 22 Mar 2024 09:03:23 -0700 > > Andrii Nakryiko wrote: > > > > > Introduce CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-25 Thread Andrii Nakryiko
On Sun, Mar 24, 2024 at 7:38 PM Masami Hiramatsu wrote: > > On Fri, 22 Mar 2024 09:03:23 -0700 > Andrii Nakryiko wrote: > > > Introduce CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING config option to > > control whether ftrace low-level code performs additional > > rcu_

[PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-22 Thread Andrii Nakryiko
with extra config to let users decide if they are willing to pay the price. Cc: Steven Rostedt Cc: Masami Hiramatsu Cc: Paul E. McKenney Signed-off-by: Andrii Nakryiko --- include/linux/trace_recursion.h | 2 +- kernel/trace/Kconfig| 13 + 2 files changed, 14 insertions(+), 1

[PATCH] tracing/kprobes: Fix symbol counting logic by looking at modules as well

2023-10-27 Thread Andrii Nakryiko
to kallsyms_on_each_match_symbol() to perform a proper counting. Cc: Francis Laniel Cc: sta...@vger.kernel.org Cc: Masami Hiramatsu Cc: Steven Rostedt Fixes: b022f0c7e404 ("tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols") Signed-off-by: Andrii Nakryiko --- kernel/trace/trace_kpr