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

[Masami Hiramatsu <mhiramat@kernel.org>]

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>