1. Patchew
  2. linux
  3. View series

[PATCH] selftests/ftrace: Let fprobe test consider already enabled functions

Heiko Carstens posted 1 patch 9 months, 3 weeks ago 
.../test.d/dynevent/add_remove_fprobe.tc       | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
[PATCH] selftests/ftrace: Let fprobe test consider already enabled functions
Posted by Heiko Carstens 9 months, 3 weeks ago 
The fprobe test fails on Fedora 41 since the fprobe test assumption that
the number of enabled_functions is zero before the test starts is not
necessarily true. Some user space tools, like systemd, add BPF programs
that attach to functions. Those will show up in the enabled_functions table
and must be taken into account by the fprobe test.

Therefore count the number of lines of enabled_functions before tests
start, and use that as base when comparing expected results.

Fixes: e85c5e9792b9 ("selftests/ftrace: Update fprobe test to check enabled_functions file")
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 .../test.d/dynevent/add_remove_fprobe.tc       | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

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 449f9d8be746..73f6c6fcecab 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
@@ -10,12 +10,16 @@ PLACE=$FUNCTION_FORK
 PLACE2="kmem_cache_free"
 PLACE3="schedule_timeout"
 
+# Some functions may have BPF programs attached, therefore
+# count already enabled_functions before tests start
+ocnt=`cat enabled_functions | wc -l`
+
 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
+if [ $cnt -ne $((ocnt + 1)) ]; then
 	exit_fail
 fi
 
@@ -23,7 +27,7 @@ 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
+if [ $cnt -ne $((ocnt + 1)) ]; then
 	exit_fail
 fi
 
@@ -32,7 +36,7 @@ echo "f:myevent3 $PLACE2" >> dynamic_events
 
 grep -q $PLACE2 enabled_functions
 cnt=`cat enabled_functions | wc -l`
-if [ $cnt -ne 2 ]; then
+if [ $cnt -ne $((ocnt + 2)) ]; then
 	exit_fail
 fi
 
@@ -49,7 +53,7 @@ grep -q myevent1 dynamic_events
 
 # should still have 2 left
 cnt=`cat enabled_functions | wc -l`
-if [ $cnt -ne 2 ]; then
+if [ $cnt -ne $((ocnt + 2)) ]; then
 	exit_fail
 fi
 
@@ -57,7 +61,7 @@ echo > dynamic_events
 
 # Should have none left
 cnt=`cat enabled_functions | wc -l`
-if [ $cnt -ne 0 ]; then
+if [ $cnt -ne $ocnt ]; then
 	exit_fail
 fi
 
@@ -65,7 +69,7 @@ echo "f:myevent4 $PLACE" >> dynamic_events
 
 # Should only have one enabled
 cnt=`cat enabled_functions | wc -l`
-if [ $cnt -ne 1 ]; then
+if [ $cnt -ne $((ocnt + 1)) ]; then
 	exit_fail
 fi
 
@@ -73,7 +77,7 @@ echo > dynamic_events
 
 # Should have none left
 cnt=`cat enabled_functions | wc -l`
-if [ $cnt -ne 0 ]; then
+if [ $cnt -ne $ocnt ]; then
 	exit_fail
 fi
 
-- 
2.45.2
Re: [PATCH] selftests/ftrace: Let fprobe test consider already enabled functions
Posted by Masami Hiramatsu (Google) 9 months, 3 weeks ago 
On Wed, 26 Feb 2025 15:27:03 +0100
Heiko Carstens <hca@linux.ibm.com> wrote:

> The fprobe test fails on Fedora 41 since the fprobe test assumption that
> the number of enabled_functions is zero before the test starts is not
> necessarily true. Some user space tools, like systemd, add BPF programs
> that attach to functions. Those will show up in the enabled_functions table
> and must be taken into account by the fprobe test.

Hmm, this ftrace selftests has been expected to be run without 
any BPF programs... Is there any other issue on other test cases?

Anyway, this looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you!


> 
> Therefore count the number of lines of enabled_functions before tests
> start, and use that as base when comparing expected results.
> 
> Fixes: e85c5e9792b9 ("selftests/ftrace: Update fprobe test to check enabled_functions file")
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  .../test.d/dynevent/add_remove_fprobe.tc       | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> 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 449f9d8be746..73f6c6fcecab 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
> @@ -10,12 +10,16 @@ PLACE=$FUNCTION_FORK
>  PLACE2="kmem_cache_free"
>  PLACE3="schedule_timeout"
>  
> +# Some functions may have BPF programs attached, therefore
> +# count already enabled_functions before tests start
> +ocnt=`cat enabled_functions | wc -l`
> +
>  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
> +if [ $cnt -ne $((ocnt + 1)) ]; then
>  	exit_fail
>  fi
>  
> @@ -23,7 +27,7 @@ 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
> +if [ $cnt -ne $((ocnt + 1)) ]; then
>  	exit_fail
>  fi
>  
> @@ -32,7 +36,7 @@ echo "f:myevent3 $PLACE2" >> dynamic_events
>  
>  grep -q $PLACE2 enabled_functions
>  cnt=`cat enabled_functions | wc -l`
> -if [ $cnt -ne 2 ]; then
> +if [ $cnt -ne $((ocnt + 2)) ]; then
>  	exit_fail
>  fi
>  
> @@ -49,7 +53,7 @@ grep -q myevent1 dynamic_events
>  
>  # should still have 2 left
>  cnt=`cat enabled_functions | wc -l`
> -if [ $cnt -ne 2 ]; then
> +if [ $cnt -ne $((ocnt + 2)) ]; then
>  	exit_fail
>  fi
>  
> @@ -57,7 +61,7 @@ echo > dynamic_events
>  
>  # Should have none left
>  cnt=`cat enabled_functions | wc -l`
> -if [ $cnt -ne 0 ]; then
> +if [ $cnt -ne $ocnt ]; then
>  	exit_fail
>  fi
>  
> @@ -65,7 +69,7 @@ echo "f:myevent4 $PLACE" >> dynamic_events
>  
>  # Should only have one enabled
>  cnt=`cat enabled_functions | wc -l`
> -if [ $cnt -ne 1 ]; then
> +if [ $cnt -ne $((ocnt + 1)) ]; then
>  	exit_fail
>  fi
>  
> @@ -73,7 +77,7 @@ echo > dynamic_events
>  
>  # Should have none left
>  cnt=`cat enabled_functions | wc -l`
> -if [ $cnt -ne 0 ]; then
> +if [ $cnt -ne $ocnt ]; then
>  	exit_fail
>  fi
>  
> -- 
> 2.45.2
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>
Re: [PATCH] selftests/ftrace: Let fprobe test consider already enabled functions
Posted by Steven Rostedt 9 months, 3 weeks ago 
On Wed, 26 Feb 2025 23:54:47 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> Hmm, this ftrace selftests has been expected to be run without 
> any BPF programs... Is there any other issue on other test cases?

Unfortunately, systemd is starting to add BPF programs :-p

I noticed that my laptop has users.

-- Steve
Re: [PATCH] selftests/ftrace: Let fprobe test consider already enabled functions
Posted by Masami Hiramatsu (Google) 9 months, 3 weeks ago 
On Wed, 26 Feb 2025 11:13:00 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 26 Feb 2025 23:54:47 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > Hmm, this ftrace selftests has been expected to be run without 
> > any BPF programs... Is there any other issue on other test cases?
> 
> Unfortunately, systemd is starting to add BPF programs :-p
> 
> I noticed that my laptop has users.

Ah, I might need systemd-busybox to build minimum test environment :-(

Thanks,

> 
> -- Steve


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>
Re: [PATCH] selftests/ftrace: Let fprobe test consider already enabled functions
Posted by Heiko Carstens 9 months, 3 weeks ago 
On Wed, Feb 26, 2025 at 11:54:47PM +0900, Masami Hiramatsu wrote:
> On Wed, 26 Feb 2025 15:27:03 +0100
> Heiko Carstens <hca@linux.ibm.com> wrote:
> 
> > The fprobe test fails on Fedora 41 since the fprobe test assumption that
> > the number of enabled_functions is zero before the test starts is not
> > necessarily true. Some user space tools, like systemd, add BPF programs
> > that attach to functions. Those will show up in the enabled_functions table
> > and must be taken into account by the fprobe test.
> 
> Hmm, this ftrace selftests has been expected to be run without 
> any BPF programs... Is there any other issue on other test cases?

At least on s390 all tests pass now.

That is of course except for the "add/remove/test uprobe events" test
where I sent a patch. Not sure what will happen with that one:

https://lore.kernel.org/all/20250220130102.2079179-1-hca@linux.ibm.com/
Re: [PATCH] selftests/ftrace: Let fprobe test consider already enabled functions
Posted by Steven Rostedt 9 months, 3 weeks ago 
On Wed, 26 Feb 2025 16:28:35 +0100
Heiko Carstens <hca@linux.ibm.com> wrote:

> That is of course except for the "add/remove/test uprobe events" test
> where I sent a patch. Not sure what will happen with that one:
> 
> https://lore.kernel.org/all/20250220130102.2079179-1-hca@linux.ibm.com/

Shuah's kernel.org emails seem to be bouncing. I'll try her Linux
Foundation one for that.

-- Steve

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.