All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Jiri Olsa <olsajiri@gmail.com>, Tianyi Liu <i.pear@outlook.com>,
	ajor@meta.com, albancrequy@linux.microsoft.com,
	andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
	flaniel@linux.microsoft.com, linux-trace-kernel@vger.kernel.org,
	linux@jordanrome.com, mathieu.desnoyers@efficios.com,
	mhiramat@kernel.org, rostedt@goodmis.org,
	Viktor Malik <vmalik@redhat.com>
Subject: Re: [PATCH v2] tracing/uprobe: Add missing PID filter for uretprobe
Date: Mon, 9 Sep 2024 12:41:44 +0200	[thread overview]
Message-ID: <Zt7Q6GVKtGTIdO1g@krava> (raw)
In-Reply-To: <20240906191814.GB17874@redhat.com>

On Fri, Sep 06, 2024 at 09:18:15PM +0200, Oleg Nesterov wrote:
> On 09/06, Jiri Olsa wrote:
> >
> > On Mon, Sep 02, 2024 at 03:22:25AM +0800, Tianyi Liu wrote:
> > >
> > > For now, please forget the original patch as we need a new solution ;)
> >
> > hi,
> > any chance we could go with your fix until we find better solution?
> 
> Well, as I said from the very beginning I won't really argue even if
> I obviously don't like this change very much. As long as the changelog /
> comments clearly explain this change. I understand that sometimes an
> ugly/incomplete/whatever workaround is better than nothing.
> 
> > it's simple and it fixes most of the cases for return uprobe pid filter
> > for events with bpf programs..
> 
> But to remind it doesn't even fixes all the filtering problems with uprobes,
> not uretprobes,
> 
> > I know during the discussion we found
> > that standard perf record path won't work if there's bpf program
> > attached on the same event,
> 
> Ah. Yes, this is another problem I tried to point out. But if we discuss
> the filtering we can forget about /usr/bin/perf.
> 
> Again, again, again, I know nothing about bpf. But it seems to me that
> perf_event_attach_bpf_prog() allows to attach up to BPF_TRACE_MAX_PROGS
> progs to event->tp_event->prog_array, and then bpf_prog_run_array_uprobe()
> should run them all. Right?
> 
> So I think that if you run 2 instances of run_prog from my last test-case
> with $PID1 and $PID2, the filtering will be broken again. Both instances
> will share the same trace_event_call and the same trace_uprobe_filter.
> 
> > and also it's not a common use case
> 
> OK.
> 
> And btw... Can bpftrace attach to the uprobe tp?
> 
> 	# perf probe -x ./test -a func
> 	Added new event:
> 	  probe_test:func      (on func in /root/TTT/test)
> 
> 	You can now use it in all perf tools, such as:
> 
> 		perf record -e probe_test:func -aR sleep 1
> 
> 	# bpftrace -e 'tracepoint:probe_test:func { printf("%d\n", pid); }'
> 	Attaching 1 probe...
> 	ioctl(PERF_EVENT_IOC_SET_BPF): Invalid argument
> 	ERROR: Error attaching probe: tracepoint:probe_test:func

the problem here is that bpftrace assumes BPF_PROG_TYPE_TRACEPOINT type
for bpf program, but that will fail in perf_event_set_bpf_prog where
perf event will be identified as uprobe and demands bpf program type
to be BPF_PROG_TYPE_KPROBE

there's same issue with kprobe as well:

	# perf probe -a ksys_read
	Added new event:
	  probe:ksys_read      (on ksys_read)

	You can now use it in all perf tools, such as:

		perf record -e probe:ksys_read -aR sleep 1


	# bpftrace -e 'tracepoint:probe:ksys_read { printf("%d\n", pid); }'
	Attaching 1 probe...
	ioctl(PERF_EVENT_IOC_SET_BPF): Invalid argument
	ERROR: Error attaching probe: tracepoint:probe:ksys_read

I'm not sure there's an easy way to filter these events from bpftrce -l
output (or change the program type accordingly), because I don't think
there's a way to find out the tracepoint subtype (kprobe/uprobe) from
the tracefs record

jirka

  reply	other threads:[~2024-09-09 10:41 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-23 13:53 [PATCH v2] tracing/uprobe: Add missing PID filter for uretprobe Tianyi Liu
2024-08-23 17:44 ` Masami Hiramatsu
2024-08-23 19:07   ` Andrii Nakryiko
2024-08-24  5:49     ` Tianyi Liu
2024-08-24 17:27       ` Masami Hiramatsu
2024-08-25 17:14       ` Oleg Nesterov
2024-08-25 18:43         ` Oleg Nesterov
2024-08-25 22:40         ` Oleg Nesterov
2024-08-26 10:05           ` Jiri Olsa
2024-08-26 11:57             ` Oleg Nesterov
2024-08-26 12:24               ` Oleg Nesterov
2024-08-26 13:48               ` Jiri Olsa
2024-08-26 18:56                 ` Oleg Nesterov
2024-08-26 21:25                 ` Oleg Nesterov
2024-08-26 22:01                   ` Jiri Olsa
2024-08-26 22:08                     ` Andrii Nakryiko
2024-08-26 22:29                     ` Oleg Nesterov
2024-08-27 13:07                       ` Jiri Olsa
2024-08-27 13:45                         ` Jiri Olsa
2024-08-27 16:45                         ` Oleg Nesterov
2024-08-28 11:40                           ` Jiri Olsa
2024-08-27 20:19                         ` Oleg Nesterov
2024-08-28 11:46                           ` Jiri Olsa
2024-08-29 15:20                             ` Oleg Nesterov
2024-08-29 19:46                               ` Jiri Olsa
2024-08-29 21:12                                 ` Oleg Nesterov
2024-08-29 23:22                                   ` Jiri Olsa
2024-08-27  6:27                   ` Tianyi Liu
2024-08-27 10:08               ` Jiri Olsa
2024-08-27 10:20                 ` Jiri Olsa
2024-08-27 10:54                   ` Oleg Nesterov
2024-08-27 10:40                 ` Oleg Nesterov
2024-08-27 13:32                   ` Jiri Olsa
2024-08-27 14:26                     ` Oleg Nesterov
2024-08-27 14:41                       ` Jiri Olsa
2024-08-26 14:52           ` Tianyi Liu
2024-08-25 17:00     ` Oleg Nesterov
2024-08-30 10:12 ` Oleg Nesterov
2024-08-30 12:23   ` Oleg Nesterov
2024-08-30 13:34   ` Jiri Olsa
2024-08-30 15:51     ` Andrii Nakryiko
2024-09-02  9:11       ` Jiri Olsa
2024-09-03 18:09         ` Andrii Nakryiko
2024-09-03 18:11           ` Andrii Nakryiko
2024-09-03 19:15             ` Jiri Olsa
2024-09-01 19:22   ` Tianyi Liu
2024-09-01 23:26     ` Oleg Nesterov
2024-09-02 17:17       ` Oleg Nesterov
2024-09-03 14:33         ` Jiri Olsa
2024-09-06 10:43     ` Jiri Olsa
2024-09-06 19:18       ` Oleg Nesterov
2024-09-09 10:41         ` Jiri Olsa [this message]
2024-09-09 18:34           ` Oleg Nesterov
2024-09-10  8:45             ` Jiri Olsa
2024-09-07 19:19       ` Tianyi Liu
2024-09-08 13:15         ` Oleg Nesterov
2024-09-09  1:16           ` Andrii Nakryiko

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=Zt7Q6GVKtGTIdO1g@krava \
    --to=olsajiri@gmail.com \
    --cc=ajor@meta.com \
    --cc=albancrequy@linux.microsoft.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=flaniel@linux.microsoft.com \
    --cc=i.pear@outlook.com \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=linux@jordanrome.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=oleg@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=vmalik@redhat.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.