| Message ID | 1603443123-17457-1-git-send-email-agordeev@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/ftrace: remove _do_fork() leftovers | expand |
On Fri, 23 Oct 2020 10:52:03 +0200 Alexander Gordeev <agordeev@linux.ibm.com> wrote: > The _do_fork() is not completely removed from selftests > in favor of the kernel_clone(). > Looks good to me. Acked-by: Masami Hiramatsu <mhiramat@kernel.org> Thank you! > Fixes: eea11285dab3 ("tracing: switch to kernel_clone()") > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Christian Brauner <christian.brauner@ubuntu.com> > Cc: Masami Hiramatsu <mhiramat@kernel.org> > Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com> > --- > tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc | 2 +- > tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > index acb17ce..0ddb948 100644 > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > @@ -39,7 +39,7 @@ do_test() { > disable_tracing > > echo do_execve* > set_ftrace_filter > - echo *do_fork >> set_ftrace_filter > + echo kernel_clone >> set_ftrace_filter > > echo $PID > set_ftrace_notrace_pid > echo function > current_tracer > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > index 9f0a968..71319b3 100644 > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > @@ -39,7 +39,7 @@ do_test() { > disable_tracing > > echo do_execve* > set_ftrace_filter > - echo *do_fork >> set_ftrace_filter > + echo kernel_clone >> set_ftrace_filter > > echo $PID > set_ftrace_pid > echo function > current_tracer > -- > 1.8.3.1 >
On Fri, 23 Oct 2020 10:52:03 +0200 Alexander Gordeev <agordeev@linux.ibm.com> wrote: > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > index acb17ce..0ddb948 100644 > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > @@ -39,7 +39,7 @@ do_test() { > disable_tracing > > echo do_execve* > set_ftrace_filter > - echo *do_fork >> set_ftrace_filter > + echo kernel_clone >> set_ftrace_filter > > echo $PID > set_ftrace_notrace_pid > echo function > current_tracer > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > index 9f0a968..71319b3 100644 > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > @@ -39,7 +39,7 @@ do_test() { > disable_tracing > > echo do_execve* > set_ftrace_filter > - echo *do_fork >> set_ftrace_filter > + echo kernel_clone >> set_ftrace_filter > > echo $PID > set_ftrace_pid > echo function > current_tracer The issue I have with this, is that I run these tests on older kernels too, and tests that use to work on older kernels should still work. In fact, this fails on the kernel I'm currently adding new changes to! Perhaps we should have: # older kernels have do_fork, but newer kernels have kernel_clone echo kernel_clone >> set_ftrace_filter || echo *do_fork >> set_ftrace_filter The above still seems to work for me. -- Steve
On Fri, Oct 23, 2020 at 09:35:23AM -0400, Steven Rostedt wrote: > On Fri, 23 Oct 2020 10:52:03 +0200 > Alexander Gordeev <agordeev@linux.ibm.com> wrote: > > > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > > index acb17ce..0ddb948 100644 > > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > > @@ -39,7 +39,7 @@ do_test() { > > disable_tracing > > > > echo do_execve* > set_ftrace_filter > > - echo *do_fork >> set_ftrace_filter > > + echo kernel_clone >> set_ftrace_filter > > > > echo $PID > set_ftrace_notrace_pid > > echo function > current_tracer > > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > > index 9f0a968..71319b3 100644 > > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > > @@ -39,7 +39,7 @@ do_test() { > > disable_tracing > > > > echo do_execve* > set_ftrace_filter > > - echo *do_fork >> set_ftrace_filter > > + echo kernel_clone >> set_ftrace_filter > > > > echo $PID > set_ftrace_pid > > echo function > current_tracer > > The issue I have with this, is that I run these tests on older kernels too, > and tests that use to work on older kernels should still work. In fact, > this fails on the kernel I'm currently adding new changes to! > > Perhaps we should have: > > # older kernels have do_fork, but newer kernels have kernel_clone > echo kernel_clone >> set_ftrace_filter || echo *do_fork >> set_ftrace_filter Would you suggest to do the same with all occurences in eea11285dab3 ("tracing: switch to kernel_clone()")? Otherwise it does not really make sense to just fix couple of tests out of dozens. > The above still seems to work for me. > > -- Steve
On Fri, 23 Oct 2020 17:12:44 +0200 Alexander Gordeev <agordeev@linux.ibm.com> wrote: > > Perhaps we should have: > > > > # older kernels have do_fork, but newer kernels have kernel_clone > > echo kernel_clone >> set_ftrace_filter || echo *do_fork >> set_ftrace_filter > > Would you suggest to do the same with all occurences in > eea11285dab3 ("tracing: switch to kernel_clone()")? > Otherwise it does not really make sense to just fix couple > of tests out of dozens. Yes. I haven't pulled in the updated tests, so I haven't hit the errors yet (nor have I merged my work with the switch to the new name yet). So those will most definitely break my tests. But because it's a more generic issue, we should have a way to find what to use. Perhaps add to the test.d/functions, something like: FUNCTION_FORK=`(if grep '\bkernel_clone\b' /proc/kallsyms > /dev/null; then echo kernel_clone; else echo '_do_fork'; fi)` and use $FUNCTION_FORK everywhere that references it. -- Steve
On Fri, 23 Oct 2020 11:49:48 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 23 Oct 2020 17:12:44 +0200 > Alexander Gordeev <agordeev@linux.ibm.com> wrote: > > > > Perhaps we should have: > > > > > > # older kernels have do_fork, but newer kernels have kernel_clone > > > echo kernel_clone >> set_ftrace_filter || echo *do_fork >> set_ftrace_filter > > > > Would you suggest to do the same with all occurences in > > eea11285dab3 ("tracing: switch to kernel_clone()")? > > Otherwise it does not really make sense to just fix couple > > of tests out of dozens. > > Yes. I haven't pulled in the updated tests, so I haven't hit the errors yet > (nor have I merged my work with the switch to the new name yet). So those > will most definitely break my tests. > > But because it's a more generic issue, we should have a way to find what to > use. Perhaps add to the test.d/functions, something like: > > FUNCTION_FORK=`(if grep '\bkernel_clone\b' /proc/kallsyms > /dev/null; then > echo kernel_clone; else echo '_do_fork'; fi)` > > and use $FUNCTION_FORK everywhere that references it. > > Let me pull in the latest changes, and whip up a patch that works on both the older kernels as well as the newer ones. -- Steve
On Fri, 23 Oct 2020 09:35:23 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 23 Oct 2020 10:52:03 +0200 > Alexander Gordeev <agordeev@linux.ibm.com> wrote: > > > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > > index acb17ce..0ddb948 100644 > > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc > > @@ -39,7 +39,7 @@ do_test() { > > disable_tracing > > > > echo do_execve* > set_ftrace_filter > > - echo *do_fork >> set_ftrace_filter > > + echo kernel_clone >> set_ftrace_filter > > > > echo $PID > set_ftrace_notrace_pid > > echo function > current_tracer > > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > > index 9f0a968..71319b3 100644 > > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc > > @@ -39,7 +39,7 @@ do_test() { > > disable_tracing > > > > echo do_execve* > set_ftrace_filter > > - echo *do_fork >> set_ftrace_filter > > + echo kernel_clone >> set_ftrace_filter > > > > echo $PID > set_ftrace_pid > > echo function > current_tracer > > The issue I have with this, is that I run these tests on older kernels too, > and tests that use to work on older kernels should still work. In fact, > this fails on the kernel I'm currently adding new changes to! > > Perhaps we should have: > > # older kernels have do_fork, but newer kernels have kernel_clone > echo kernel_clone >> set_ftrace_filter || echo *do_fork >> set_ftrace_filter Good catch. BTW, can we check the filter-bility by grep the pattern from set_ftrace_filter? Thank you,
On Sat, 24 Oct 2020 10:31:12 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > > Perhaps we should have: > > > > # older kernels have do_fork, but newer kernels have kernel_clone > > echo kernel_clone >> set_ftrace_filter || echo *do_fork >> set_ftrace_filter > > Good catch. BTW, can we check the filter-bility by grep the pattern from set_ftrace_filter? I think we need to use /proc/kallsyms, as the kprobe code should still work if function tracing is disabled, and the function filter files will not exist. -- Steve
On 10/23/20 9:51 AM, Steven Rostedt wrote: > On Fri, 23 Oct 2020 11:49:48 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > >> On Fri, 23 Oct 2020 17:12:44 +0200 >> Alexander Gordeev <agordeev@linux.ibm.com> wrote: >> >>>> Perhaps we should have: >>>> >>>> # older kernels have do_fork, but newer kernels have kernel_clone >>>> echo kernel_clone >> set_ftrace_filter || echo *do_fork >> set_ftrace_filter >>> >>> Would you suggest to do the same with all occurences in >>> eea11285dab3 ("tracing: switch to kernel_clone()")? >>> Otherwise it does not really make sense to just fix couple >>> of tests out of dozens. >> >> Yes. I haven't pulled in the updated tests, so I haven't hit the errors yet >> (nor have I merged my work with the switch to the new name yet). So those >> will most definitely break my tests. >> >> But because it's a more generic issue, we should have a way to find what to >> use. Perhaps add to the test.d/functions, something like: >> >> FUNCTION_FORK=`(if grep '\bkernel_clone\b' /proc/kallsyms > /dev/null; then >> echo kernel_clone; else echo '_do_fork'; fi)` >> >> and use $FUNCTION_FORK everywhere that references it. >> >> > > Let me pull in the latest changes, and whip up a patch that works on both > the older kernels as well as the newer ones. > > -- Steve > Assume this is handled by selftests/ftrace: Use $FUNCTION_FORK to reference kernel fork function https://patchwork.kernel.org/project/linux-kselftest/patch/20201026162032.124c728d@gandalf.local.home/ Just making sure. thanks, -- Shuah
On Tue, 27 Oct 2020 15:55:32 -0600 Shuah Khan <skhan@linuxfoundation.org> wrote: > > Let me pull in the latest changes, and whip up a patch that works on both > > the older kernels as well as the newer ones. > > > > -- Steve > > > > Assume this is handled by > > selftests/ftrace: Use $FUNCTION_FORK to reference kernel fork function > https://patchwork.kernel.org/project/linux-kselftest/patch/20201026162032.124c728d@gandalf.local.home/ > > Just making sure. Yep, that was the result of this thread. Thanks Shuah, -- Steve
On Tue, 27 Oct 2020 22:46:57 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> Yep, that was the result of this thread.
And if it's not clear. That thread supersedes this one.
-- Steve
diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc index acb17ce..0ddb948 100644 --- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc @@ -39,7 +39,7 @@ do_test() { disable_tracing echo do_execve* > set_ftrace_filter - echo *do_fork >> set_ftrace_filter + echo kernel_clone >> set_ftrace_filter echo $PID > set_ftrace_notrace_pid echo function > current_tracer diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc index 9f0a968..71319b3 100644 --- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc @@ -39,7 +39,7 @@ do_test() { disable_tracing echo do_execve* > set_ftrace_filter - echo *do_fork >> set_ftrace_filter + echo kernel_clone >> set_ftrace_filter echo $PID > set_ftrace_pid echo function > current_tracer
The _do_fork() is not completely removed from selftests in favor of the kernel_clone(). Fixes: eea11285dab3 ("tracing: switch to kernel_clone()") Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Christian Brauner <christian.brauner@ubuntu.com> Cc: Masami Hiramatsu <mhiramat@kernel.org> Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com> --- tools/testing/selftests/ftrace/test.d/ftrace/func-filter-notrace-pid.tc | 2 +- tools/testing/selftests/ftrace/test.d/ftrace/func-filter-pid.tc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)