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 <[email protected]>
Cc: Christian Brauner <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
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, Oct 23, 2020 at 09:35:23AM -0400, Steven Rostedt wrote:
> On Fri, 23 Oct 2020 10:52:03 +0200
> Alexander Gordeev <[email protected]> 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 10:52:03 +0200
Alexander Gordeev <[email protected]> wrote:
> The _do_fork() is not completely removed from selftests
> in favor of the kernel_clone().
>
Looks good to me.
Acked-by: Masami Hiramatsu <[email protected]>
Thank you!
> Fixes: eea11285dab3 ("tracing: switch to kernel_clone()")
> Cc: Steven Rostedt <[email protected]>
> Cc: Christian Brauner <[email protected]>
> Cc: Masami Hiramatsu <[email protected]>
> Signed-off-by: Alexander Gordeev <[email protected]>
> ---
> 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
>
--
Masami Hiramatsu <[email protected]>
On Fri, 23 Oct 2020 10:52:03 +0200
Alexander Gordeev <[email protected]> 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, 23 Oct 2020 17:12:44 +0200
Alexander Gordeev <[email protected]> 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 <[email protected]> wrote:
> On Fri, 23 Oct 2020 17:12:44 +0200
> Alexander Gordeev <[email protected]> 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 <[email protected]> wrote:
> On Fri, 23 Oct 2020 10:52:03 +0200
> Alexander Gordeev <[email protected]> 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,
--
Masami Hiramatsu <[email protected]>
On Sat, 24 Oct 2020 10:31:12 +0900
Masami Hiramatsu <[email protected]> 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 <[email protected]> wrote:
>
>> On Fri, 23 Oct 2020 17:12:44 +0200
>> Alexander Gordeev <[email protected]> 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/[email protected]/
Just making sure.
thanks,
-- Shuah
On Tue, 27 Oct 2020 22:46:57 -0400
Steven Rostedt <[email protected]> wrote:
> Yep, that was the result of this thread.
And if it's not clear. That thread supersedes this one.
-- Steve
On Tue, 27 Oct 2020 15:55:32 -0600
Shuah Khan <[email protected]> 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/[email protected]/
>
> Just making sure.
Yep, that was the result of this thread.
Thanks Shuah,
-- Steve