| Message ID | 20250218193126.956559192@goodmis.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ftrace: Fix fprobe with function graph accounting | expand |
On Tue, 18 Feb 2025 14:30:38 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > A few bugs were found in the fprobe accounting logic along with it using > the function graph infrastructure. Update the fprobe selftest to catch > those bugs in case they or something similar shows up in the future. > > The test now checks the enabled_functions file which shows all the > functions attached to ftrace or fgraph. When enabling a fprobe, make sure > that its corresponding function is also added to that file. Also add two > more fprobes to enable to make sure that the fprobe logic works properly > with multiple probes. This looks good to me. Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> BTW, would I pick the last 3 patches via probes/fixes branch? Thanks, > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > .../test.d/dynevent/add_remove_fprobe.tc | 54 +++++++++++++++++++ > 1 file changed, 54 insertions(+) > > diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc > index dc25bcf4f9e2..449f9d8be746 100644 > --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc > +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc > @@ -7,12 +7,38 @@ echo 0 > events/enable > echo > dynamic_events > > PLACE=$FUNCTION_FORK > +PLACE2="kmem_cache_free" > +PLACE3="schedule_timeout" > > echo "f:myevent1 $PLACE" >> dynamic_events > + > +# Make sure the event is attached and is the only one > +grep -q $PLACE enabled_functions > +cnt=`cat enabled_functions | wc -l` > +if [ $cnt -ne 1 ]; then > + exit_fail > +fi > + > echo "f:myevent2 $PLACE%return" >> dynamic_events > > +# It should till be the only attached function > +cnt=`cat enabled_functions | wc -l` > +if [ $cnt -ne 1 ]; then > + exit_fail > +fi > + > +# add another event > +echo "f:myevent3 $PLACE2" >> dynamic_events > + > +grep -q $PLACE2 enabled_functions > +cnt=`cat enabled_functions | wc -l` > +if [ $cnt -ne 2 ]; then > + exit_fail > +fi > + > grep -q myevent1 dynamic_events > grep -q myevent2 dynamic_events > +grep -q myevent3 dynamic_events > test -d events/fprobes/myevent1 > test -d events/fprobes/myevent2 > > @@ -21,6 +47,34 @@ echo "-:myevent2" >> dynamic_events > grep -q myevent1 dynamic_events > ! grep -q myevent2 dynamic_events > > +# should still have 2 left > +cnt=`cat enabled_functions | wc -l` > +if [ $cnt -ne 2 ]; then > + exit_fail > +fi > + > echo > dynamic_events > > +# Should have none left > +cnt=`cat enabled_functions | wc -l` > +if [ $cnt -ne 0 ]; then > + exit_fail > +fi > + > +echo "f:myevent4 $PLACE" >> dynamic_events > + > +# Should only have one enabled > +cnt=`cat enabled_functions | wc -l` > +if [ $cnt -ne 1 ]; then > + exit_fail > +fi > + > +echo > dynamic_events > + > +# Should have none left > +cnt=`cat enabled_functions | wc -l` > +if [ $cnt -ne 0 ]; then > + exit_fail > +fi > + > clear_trace > -- > 2.47.2 > >
On Wed, 19 Feb 2025 10:01:11 +0900 Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > This looks good to me. > > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > BTW, would I pick the last 3 patches via probes/fixes branch? Only if you think it's necessary. Or I can push the entire series out to Linus this week, as I plan on basing all the new work on rc4 (which would be released this Sunday, hopefully with these fixes). Or if you want, you can take the entire series if you want to push it to Linus this week. As I'll be basing off of rc4, it doesn't matter who send it to Linus as long as it's there before Sunday's release. -- Steve
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc index dc25bcf4f9e2..449f9d8be746 100644 --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe.tc @@ -7,12 +7,38 @@ echo 0 > events/enable echo > dynamic_events PLACE=$FUNCTION_FORK +PLACE2="kmem_cache_free" +PLACE3="schedule_timeout" echo "f:myevent1 $PLACE" >> dynamic_events + +# Make sure the event is attached and is the only one +grep -q $PLACE enabled_functions +cnt=`cat enabled_functions | wc -l` +if [ $cnt -ne 1 ]; then + exit_fail +fi + echo "f:myevent2 $PLACE%return" >> dynamic_events +# It should till be the only attached function +cnt=`cat enabled_functions | wc -l` +if [ $cnt -ne 1 ]; then + exit_fail +fi + +# add another event +echo "f:myevent3 $PLACE2" >> dynamic_events + +grep -q $PLACE2 enabled_functions +cnt=`cat enabled_functions | wc -l` +if [ $cnt -ne 2 ]; then + exit_fail +fi + grep -q myevent1 dynamic_events grep -q myevent2 dynamic_events +grep -q myevent3 dynamic_events test -d events/fprobes/myevent1 test -d events/fprobes/myevent2 @@ -21,6 +47,34 @@ echo "-:myevent2" >> dynamic_events grep -q myevent1 dynamic_events ! grep -q myevent2 dynamic_events +# should still have 2 left +cnt=`cat enabled_functions | wc -l` +if [ $cnt -ne 2 ]; then + exit_fail +fi + echo > dynamic_events +# Should have none left +cnt=`cat enabled_functions | wc -l` +if [ $cnt -ne 0 ]; then + exit_fail +fi + +echo "f:myevent4 $PLACE" >> dynamic_events + +# Should only have one enabled +cnt=`cat enabled_functions | wc -l` +if [ $cnt -ne 1 ]; then + exit_fail +fi + +echo > dynamic_events + +# Should have none left +cnt=`cat enabled_functions | wc -l` +if [ $cnt -ne 0 ]; then + exit_fail +fi + clear_trace