[PATCH] selftests/ftrace: Disable multiple_kprobes test on powerpc

[PATCH] selftests/ftrace: Disable multiple_kprobes test on powerpc

[Naveen N. Rao]

Not all symbols are blacklisted on powerpc. Disable multiple_kprobes
test until that is sorted, so that rest of ftrace and kprobe selftests
can be run.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 .../testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
index 312d237800969e..41503c32f53eed 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
@@ -7,7 +7,7 @@
 # Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc64le
 case `uname -m` in
   x86_64|i[3456]86) OFFS=5;;
-  ppc64le) OFFS=8;;
+  ppc*) exit_unsupported;;
   *) OFFS=0;;
 esac
 

base-commit: 36bbbd0e234d817938bdc52121a0f5473b3e58f5
-- 
2.25.4

Re: [PATCH] selftests/ftrace: Disable multiple_kprobes test on powerpc

[Masami Hiramatsu]

On Tue,  5 Jan 2021 12:27:30 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Not all symbols are blacklisted on powerpc. Disable multiple_kprobes
> test until that is sorted, so that rest of ftrace and kprobe selftests
> can be run.

This looks good to me, but could you try to find the functions
which should be blocked from kprobes?
(Usually, the function which are involved in the sw-breakpoint
 handling, including locks etc.)

Thank you,

> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  .../testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc  | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
> index 312d237800969e..41503c32f53eed 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
> @@ -7,7 +7,7 @@
>  # Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc64le
>  case `uname -m` in
>    x86_64|i[3456]86) OFFS=5;;
> -  ppc64le) OFFS=8;;
> +  ppc*) exit_unsupported;;
>    *) OFFS=0;;
>  esac
>  
> 
> base-commit: 36bbbd0e234d817938bdc52121a0f5473b3e58f5
> -- 
> 2.25.4
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH] selftests/ftrace: Disable multiple_kprobes test on powerpc

[Naveen N. Rao]

Masami Hiramatsu wrote:
> On Tue,  5 Jan 2021 12:27:30 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
>> Not all symbols are blacklisted on powerpc. Disable multiple_kprobes
>> test until that is sorted, so that rest of ftrace and kprobe selftests
>> can be run.
> 
> This looks good to me, but could you try to find the functions
> which should be blocked from kprobes?
> (Usually, the function which are involved in the sw-breakpoint
>  handling, including locks etc.)

Yes, we did add several blacklists some time back, but there has been 
quite a bit of churn in our entry code. I've been meaning to audit it 
for a while now, but this has been blocking tests. It would be nice to 
skip this test for now until I am able to spend some time on this.

Thanks,
Naveen

Re: [PATCH] selftests/ftrace: Disable multiple_kprobes test on powerpc

[Masami Hiramatsu]

On Tue, 05 Jan 2021 15:42:44 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Masami Hiramatsu wrote:
> > On Tue,  5 Jan 2021 12:27:30 +0530
> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> > 
> >> Not all symbols are blacklisted on powerpc. Disable multiple_kprobes
> >> test until that is sorted, so that rest of ftrace and kprobe selftests
> >> can be run.
> > 
> > This looks good to me, but could you try to find the functions
> > which should be blocked from kprobes?
> > (Usually, the function which are involved in the sw-breakpoint
> >  handling, including locks etc.)
> 
> Yes, we did add several blacklists some time back, but there has been 
> quite a bit of churn in our entry code. I've been meaning to audit it 
> for a while now, but this has been blocking tests. It would be nice to 
> skip this test for now until I am able to spend some time on this.

OK, But for revisiting this testcase in the future, I think it should
return UNTESTED instead of UNSUPPORTED.

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH] selftests/ftrace: Disable multiple_kprobes test on powerpc

[Masami Hiramatsu]

On Tue, 5 Jan 2021 19:01:56 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Tue,  5 Jan 2021 12:27:30 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
> > Not all symbols are blacklisted on powerpc. Disable multiple_kprobes
> > test until that is sorted, so that rest of ftrace and kprobe selftests
> > can be run.
> 
> This looks good to me, but could you try to find the functions
> which should be blocked from kprobes?
> (Usually, the function which are involved in the sw-breakpoint
>  handling, including locks etc.)

Ah, OK. 
I wonder why CONFIG_KPROBE_EVENTS_ON_NOTRACE=n doesn't help, it
was ignored if the arch doesn't support CONFIG_KPROBES_ON_FTRACE.

Naveen, could you try to run this test case with following patch
on powerpc?

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index b911e9f6d9f5..241a55313476 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -433,7 +433,7 @@ static int disable_trace_kprobe(struct trace_event_call *call,
        return 0;
 }
 
-#if defined(CONFIG_KPROBES_ON_FTRACE) && \
+#if defined(CONFIG_FUNCTION_TRACER) && \
        !defined(CONFIG_KPROBE_EVENTS_ON_NOTRACE)
 static bool __within_notrace_func(unsigned long addr)
 {

Of course you need to enable CONFIG_FUNCTION_TRACER=y, but it may
pass the test.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH] selftests/ftrace: Disable multiple_kprobes test on powerpc

[Naveen N. Rao]

Masami Hiramatsu wrote:
> On Tue, 5 Jan 2021 19:01:56 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
>> On Tue,  5 Jan 2021 12:27:30 +0530
>> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>> 
>> > Not all symbols are blacklisted on powerpc. Disable multiple_kprobes
>> > test until that is sorted, so that rest of ftrace and kprobe selftests
>> > can be run.
>> 
>> This looks good to me, but could you try to find the functions
>> which should be blocked from kprobes?
>> (Usually, the function which are involved in the sw-breakpoint
>>  handling, including locks etc.)
> 
> Ah, OK. 
> I wonder why CONFIG_KPROBE_EVENTS_ON_NOTRACE=n doesn't help, it
> was ignored if the arch doesn't support CONFIG_KPROBES_ON_FTRACE.

Good point, though we do support CONFIG_KPROBES_ON_FTRACE on powerpc so 
the below patch is unlikely to help. However, since entry code is 
unlikely to be the source of the issue due to 
CONFIG_KPROBE_EVENTS_ON_NOTRACE, I will take another look to see where 
the problem lies.

> 
> Naveen, could you try to run this test case with following patch
> on powerpc?
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index b911e9f6d9f5..241a55313476 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -433,7 +433,7 @@ static int disable_trace_kprobe(struct trace_event_call *call,
>         return 0;
>  }
> 
> -#if defined(CONFIG_KPROBES_ON_FTRACE) && \
> +#if defined(CONFIG_FUNCTION_TRACER) && \
>         !defined(CONFIG_KPROBE_EVENTS_ON_NOTRACE)
>  static bool __within_notrace_func(unsigned long addr)
>  {
> 

This looks like a good change regardless, so if you intend to post this 
separately:
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Thanks,
Naveen

Re: [PATCH] selftests/ftrace: Disable multiple_kprobes test on powerpc

[Masami Hiramatsu]

On Tue, 05 Jan 2021 16:51:50 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Masami Hiramatsu wrote:
> > On Tue, 5 Jan 2021 19:01:56 +0900
> > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > 
> >> On Tue,  5 Jan 2021 12:27:30 +0530
> >> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> >> 
> >> > Not all symbols are blacklisted on powerpc. Disable multiple_kprobes
> >> > test until that is sorted, so that rest of ftrace and kprobe selftests
> >> > can be run.
> >> 
> >> This looks good to me, but could you try to find the functions
> >> which should be blocked from kprobes?
> >> (Usually, the function which are involved in the sw-breakpoint
> >>  handling, including locks etc.)
> > 
> > Ah, OK. 
> > I wonder why CONFIG_KPROBE_EVENTS_ON_NOTRACE=n doesn't help, it
> > was ignored if the arch doesn't support CONFIG_KPROBES_ON_FTRACE.
> 
> Good point, though we do support CONFIG_KPROBES_ON_FTRACE on powerpc so 
> the below patch is unlikely to help. However, since entry code is 
> unlikely to be the source of the issue due to 
> CONFIG_KPROBE_EVENTS_ON_NOTRACE, I will take another look to see where 
> the problem lies.

Yeah, if any issue happens even if the notrace function check is done,
there might be some other issues. (of course this depends on whether the
critical functions correctly annotated by notrace...)

> > Naveen, could you try to run this test case with following patch
> > on powerpc?
> > 
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index b911e9f6d9f5..241a55313476 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -433,7 +433,7 @@ static int disable_trace_kprobe(struct trace_event_call *call,
> >         return 0;
> >  }
> > 
> > -#if defined(CONFIG_KPROBES_ON_FTRACE) && \
> > +#if defined(CONFIG_FUNCTION_TRACER) && \
> >         !defined(CONFIG_KPROBE_EVENTS_ON_NOTRACE)
> >  static bool __within_notrace_func(unsigned long addr)
> >  {
> > 
> 
> This looks like a good change regardless, so if you intend to post this 
> separately:
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Thank you!


-- 
Masami Hiramatsu <mhiramat@kernel.org>