Re: [PATCH] libtracefs: Have tracefs_dynevent_get_all() find kprobes and uprobes properly

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 16 Oct 2024 09:33:00 +0100
Metin Kaya <metin.kaya@arm.com> wrote:


> > 
> > I could successfully apply the patch on 4cbebed79b1f ("libtracefs: 
> > Documentation: Add missing documentation to meson.build").
> > The unit tests at [1] passes with this patch.
> > 
> > Just for the records, there are some unit test failures in libtracefs 
> > (they were there even before the patch):
> > 
> > $ sudo ./utest/trace-utest
> >       CUnit - A unit testing framework for C - Version 2.1-3
> >       http://cunit.sourceforge.net/
> > 
> > Memory mapped buffers not supported
> > 
> > Suite: tracefs library
> >    Test: Test tracefs/debugfs mounting ...FAILED
> >      1. tracefs-utest.c:1806  - ret == 0

Note, if you have anything in the /sys/kernel/tracing directory (including
gdb instances of trace-cmd), it will fail to unmount. I tripped over this
too.

> >    Test: trace cpu read ...passed
> >    Test: trace cpu read_buf_percent ...passed
> >    Test: trace cpu pipe ...passed
> >    Test: trace pid events filter ...passed
> >    Test: trace pid function filter ...passed
> >    Test: trace sql ...passed
> >    Test: trace sql trace onmax ...passed
> >    Test: trace sql trace onchange ...passed
> >    Test: trace sql snapshot onmax ...passed
> >    Test: trace sql snapshot onchange ...passed
> >    Test: trace sql save onmax ...passed
> >    Test: trace sql save onchange ...passed
> >    Test: trace sql trace and snapshot onmax ...passed
> >    Test: trace sql trace and snapshot onchange ...passed
> >    Test: tracing file / directory APIs ...passed
> >    Test: instance file / directory APIs ...passed
> >    Test: instance file descriptor ...passed
> >    Test: instance reset ...passed
> >    Test: systems and events APIs ...passed
> >    Test: tracefs_iterate_snapshot_events API ...passed
> >    Test: tracefs_iterate_raw_events API ...FAILED
> >      1. tracefs-utest.c:235  - ret == sizeof(struct test_sample)
> >      2. tracefs-utest.c:235  - ret == sizeof(struct test_sample)

Did you do a trace-cmd reset before running the tests?

> >    Test: Follow events ...passed
> >    Test: Follow events clear ...passed
> >    Test: tracefs_tracers API ...passed
> >    Test: tracefs_local events API ...passed
> >    Test: tracefs_instances_walk API ...passed
> >    Test: tracefs_get_clock API ...passed
> >    Test: tracing on / off ...passed
> >    Test: tracing options ...passed
> >    Test: custom system directory ...passed
> >    Test: ftrace marker ...passed
> >    Test: kprobes ...passed
> >    Test: synthetic events ...passed
> >    Test: eprobes ...passed
> >    Test: uprobes ...FAILED
> >      1. tracefs-utest.c:2222  - ret == 0
> >      2. tracefs-utest.c:2222  - ret == 0

Did you have left over uprobes?

> > 
> > Run Summary:    Type    Total      Ran   Passed Failed Inactive
> >                suites        1        1      n/a      0        0
> >                 tests       36       36       33      3        0
> >               asserts 30341119 30341119 30341114      5      n/a
> > 
> > Elapsed time =  124.937 seconds
> > 
> > With that,
> > 
> > Reviewed-by: Metin Kaya <metin.kaya@arm.com>
> > 
> > 
> > Thanks a lot for fixing it promptly!
> > 
> > [1] 
> > https://lore.kernel.org/all/20241015140840.4183007-1-metin.kaya@arm.com/
> > 
> >   
> 
> Sorry, I need to revoke my Reviewed-by. Because, "trace-cmd reset" 
> cannot destroy uretprobes after this patch.
> 
> # cd /sys/kernel/tracing/
> /sys/kernel/tracing # cat uprobe_events
> /sys/kernel/tracing # echo 'r /bin/bash:0x4245c0' > uprobe_events
> /sys/kernel/tracing # cat uprobe_events
> r:uprobes/p_bash_0x4245c0 /bin/bash:0x00000000004245c0
> /sys/kernel/tracing # trace-cmd reset
> /sys/kernel/tracing # cat uprobe_events
> r:uprobes/p_bash_0x4245c0 /bin/bash:0x00000000004245c0
> 
> OTOH, "trace-cmd reset" is able to destroy uretprobes if there is also a 
> kprobe in addition to a uretprobe:

Oops, I know why. I had a bug in the code that collects all the probes and
looks at different files. It assumed that if the return of
tracefs_instance_file_read() returns NULL from kprobe_events, it is an
error. But that function also returns NULL if the file is empty. I changed
the code from:

	content = tracefs_instance_file_read(NULL, desc->file, NULL);
	if (!content)
		return -1;

to:

	if (!tracefs_file_exists(NULL, desc->file))
		return -1;

	content = tracefs_instance_file_read(NULL, desc->file, NULL);
	/* File exists, but may be empty */
	if (!content)
		return 0;

I'll send out a v2. Thanks for testing.

I'll also add more tests to the libtracefs utest to check for this too.

-- Steve


> 
> /sys/kernel/tracing # cat dynamic_events
> /sys/kernel/tracing # cat uprobe_events
> /sys/kernel/tracing # echo 'p do_sys_open' > kprobe_events
> /sys/kernel/tracing # echo 'r /bin/bash:0x4245c0' > uprobe_events
> /sys/kernel/tracing # cat dynamic_events
> p:kprobes/p_do_sys_open_0 do_sys_open
> r:uprobes/p_bash_0x4245c0 /bin/bash:0x00000000004245c0
> /sys/kernel/tracing # cat uprobe_events
> r:uprobes/p_bash_0x4245c0 /bin/bash:0x00000000004245c0
> /sys/kernel/tracing # trace-cmd reset
> /sys/kernel/tracing # cat dynamic_events
> /sys/kernel/tracing # cat uprobe_events
> /sys/kernel/tracing #