Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Peter Zijlstra]

On Tue, May 26, 2020 at 02:28:26PM -0700, Song Liu wrote:
> It is useful to trace functions in kernel/event/core.c. Allow ftrace for
> them by removing $(CC_FLAGS_FTRACE) from Makefile.

Did you try using the ftrace event with perf with this on?

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Song Liu]

> On May 26, 2020, at 2:39 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> 
> On Tue, May 26, 2020 at 02:28:26PM -0700, Song Liu wrote:
>> It is useful to trace functions in kernel/event/core.c. Allow ftrace for
>> them by removing $(CC_FLAGS_FTRACE) from Makefile.
> 
> Did you try using the ftrace event with perf with this on?

I have tried a few things, like 

  perf stat -e probe:perf_read -I 1000
  perf record -e probe:__x64_sys_perf_event_open -aR

They all work fine. 

Do you have some tricky functions that we should double check?

Thanks,
Song

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Peter Zijlstra]

On Tue, May 26, 2020 at 09:46:29PM +0000, Song Liu wrote:
> 
> 
> > On May 26, 2020, at 2:39 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > On Tue, May 26, 2020 at 02:28:26PM -0700, Song Liu wrote:
> >> It is useful to trace functions in kernel/event/core.c. Allow ftrace for
> >> them by removing $(CC_FLAGS_FTRACE) from Makefile.
> > 
> > Did you try using the ftrace event with perf with this on?
> 
> I have tried a few things, like 
> 
>   perf stat -e probe:perf_read -I 1000
>   perf record -e probe:__x64_sys_perf_event_open -aR
> 
> They all work fine. 
> 
> Do you have some tricky functions that we should double check?

I've no idea what probe: does. iirc there's something like
ftrace:function that is like regular function tracing.

At some point using that made the kernel really sick due to recursion
between ftrace and perf. Quite possibly that's been fixed, dunno.

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Steven Rostedt]

On Tue, 26 May 2020 23:54:15 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, May 26, 2020 at 09:46:29PM +0000, Song Liu wrote:
> > 
> >   
> > > On May 26, 2020, at 2:39 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > > 
> > > On Tue, May 26, 2020 at 02:28:26PM -0700, Song Liu wrote:  
> > >> It is useful to trace functions in kernel/event/core.c. Allow ftrace for
> > >> them by removing $(CC_FLAGS_FTRACE) from Makefile.  
> > > 
> > > Did you try using the ftrace event with perf with this on?  
> > 
> > I have tried a few things, like 
> > 
> >   perf stat -e probe:perf_read -I 1000
> >   perf record -e probe:__x64_sys_perf_event_open -aR
> > 
> > They all work fine. 
> > 
> > Do you have some tricky functions that we should double check?  
> 
> I've no idea what probe: does. iirc there's something like
> ftrace:function that is like regular function tracing.
> 
> At some point using that made the kernel really sick due to recursion
> between ftrace and perf. Quite possibly that's been fixed, dunno.

In the early days there was a lot of issues with recursions, but I added a
lot of recursion protection since then. I'll give this patch a spin and see
if I can make it crash.

-- Steve

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Steven Rostedt]

On Tue, 26 May 2020 18:04:37 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> In the early days there was a lot of issues with recursions, but I added a
> lot of recursion protection since then. I'll give this patch a spin and see
> if I can make it crash.

I also have some patches that make both perf and the trace directory
visible by function tracing. I'll see if I can dig them out too.

-- Steve

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Song Liu]

> On May 26, 2020, at 2:54 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> 
> On Tue, May 26, 2020 at 09:46:29PM +0000, Song Liu wrote:
>> 
>> 
>>> On May 26, 2020, at 2:39 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>>> 
>>> On Tue, May 26, 2020 at 02:28:26PM -0700, Song Liu wrote:
>>>> It is useful to trace functions in kernel/event/core.c. Allow ftrace for
>>>> them by removing $(CC_FLAGS_FTRACE) from Makefile.
>>> 
>>> Did you try using the ftrace event with perf with this on?
>> 
>> I have tried a few things, like 
>> 
>>  perf stat -e probe:perf_read -I 1000
>>  perf record -e probe:__x64_sys_perf_event_open -aR
>> 
>> They all work fine. 
>> 
>> Do you have some tricky functions that we should double check?
> 
> I've no idea what probe: does. iirc there's something like
> ftrace:function that is like regular function tracing.

Those are kprobes. I have CONFIG_HAVE_KPROBES_ON_FTRACE=y in .config. 

> 
> At some point using that made the kernel really sick due to recursion
> between ftrace and perf. Quite possibly that's been fixed, dunno.

I also tried on ctx_sched_out() and event_sched_out(). They also work
as expected. 

Thanks,
Song

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Song Liu]

> On May 26, 2020, at 3:04 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Tue, 26 May 2020 23:54:15 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
>> On Tue, May 26, 2020 at 09:46:29PM +0000, Song Liu wrote:
>>> 
>>> 
>>>> On May 26, 2020, at 2:39 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>>>> 
>>>> On Tue, May 26, 2020 at 02:28:26PM -0700, Song Liu wrote:  
>>>>> It is useful to trace functions in kernel/event/core.c. Allow ftrace for
>>>>> them by removing $(CC_FLAGS_FTRACE) from Makefile.  
>>>> 
>>>> Did you try using the ftrace event with perf with this on?  
>>> 
>>> I have tried a few things, like 
>>> 
>>>  perf stat -e probe:perf_read -I 1000
>>>  perf record -e probe:__x64_sys_perf_event_open -aR
>>> 
>>> They all work fine. 
>>> 
>>> Do you have some tricky functions that we should double check?  
>> 
>> I've no idea what probe: does. iirc there's something like
>> ftrace:function that is like regular function tracing.
>> 
>> At some point using that made the kernel really sick due to recursion
>> between ftrace and perf. Quite possibly that's been fixed, dunno.
> 
> In the early days there was a lot of issues with recursions, but I added a
> lot of recursion protection since then. I'll give this patch a spin and see
> if I can make it crash.

Thanks! 

Song

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Masami Hiramatsu]

On Tue, 26 May 2020 21:46:29 +0000
Song Liu <songliubraving@fb.com> wrote:

> 
> 
> > On May 26, 2020, at 2:39 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > On Tue, May 26, 2020 at 02:28:26PM -0700, Song Liu wrote:
> >> It is useful to trace functions in kernel/event/core.c. Allow ftrace for
> >> them by removing $(CC_FLAGS_FTRACE) from Makefile.
> > 
> > Did you try using the ftrace event with perf with this on?
> 
> I have tried a few things, like 
> 
>   perf stat -e probe:perf_read -I 1000
>   perf record -e probe:__x64_sys_perf_event_open -aR
> 
> They all work fine. 

Did you try using perf with function-tracer or function-graph tracer?
If you just want to trace those functions with kprobes, you can 
build your kernel with CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, which allows
you to probe perf_read etc.

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Song Liu]

> On May 26, 2020, at 10:40 PM, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> On Tue, 26 May 2020 21:46:29 +0000
> Song Liu <songliubraving@fb.com> wrote:
> 
>> 
>> 
>>> On May 26, 2020, at 2:39 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>>> 
>>> On Tue, May 26, 2020 at 02:28:26PM -0700, Song Liu wrote:
>>>> It is useful to trace functions in kernel/event/core.c. Allow ftrace for
>>>> them by removing $(CC_FLAGS_FTRACE) from Makefile.
>>> 
>>> Did you try using the ftrace event with perf with this on?
>> 
>> I have tried a few things, like 
>> 
>>  perf stat -e probe:perf_read -I 1000
>>  perf record -e probe:__x64_sys_perf_event_open -aR
>> 
>> They all work fine. 
> 
> Did you try using perf with function-tracer or function-graph tracer?
> If you just want to trace those functions with kprobes, you can 
> build your kernel with CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, which allows
> you to probe perf_read etc.

Thanks for the hint. 

kprobe does work with CONFIG_KPROBE_EVENTS_ON_NOTRACE, but BPF trampoline
doesn't work. If it is safe, removing $(CC_FLAGS_FTRACE) is preferable.

Thanks,
Song

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Song Liu]

Hi Steven,

> On May 26, 2020, at 3:04 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Tue, 26 May 2020 23:54:15 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
>> On Tue, May 26, 2020 at 09:46:29PM +0000, Song Liu wrote:
>>> 
>>> 
>>>> On May 26, 2020, at 2:39 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>>>> 
>>>> On Tue, May 26, 2020 at 02:28:26PM -0700, Song Liu wrote:  
>>>>> It is useful to trace functions in kernel/event/core.c. Allow ftrace for
>>>>> them by removing $(CC_FLAGS_FTRACE) from Makefile.  
>>>> 
>>>> Did you try using the ftrace event with perf with this on?  
>>> 
>>> I have tried a few things, like 
>>> 
>>>  perf stat -e probe:perf_read -I 1000
>>>  perf record -e probe:__x64_sys_perf_event_open -aR
>>> 
>>> They all work fine. 
>>> 
>>> Do you have some tricky functions that we should double check?  
>> 
>> I've no idea what probe: does. iirc there's something like
>> ftrace:function that is like regular function tracing.
>> 
>> At some point using that made the kernel really sick due to recursion
>> between ftrace and perf. Quite possibly that's been fixed, dunno.
> 
> In the early days there was a lot of issues with recursions, but I added a
> lot of recursion protection since then. I'll give this patch a spin and see
> if I can make it crash.

How does this work in your tests? 

Thanks,
Song

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Steven Rostedt]

On Fri, 5 Jun 2020 21:58:48 +0000
Song Liu <songliubraving@fb.com> wrote:

> 
> How does this work in your tests? 

I started it, but got distracted by other work. It did not crash with
the little testing I did do. I wanted to also look at my patch that
adds tracing to the ftrace directory too.

I'll try to remember to address this some more next week. Feel free to
ping me again then.

-- Steve

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Song Liu]

Hi Steven,

> On Jun 5, 2020, at 3:02 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Fri, 5 Jun 2020 21:58:48 +0000
> Song Liu <songliubraving@fb.com> wrote:
> 
>> 
>> How does this work in your tests? 
> 
> I started it, but got distracted by other work. It did not crash with
> the little testing I did do. I wanted to also look at my patch that
> adds tracing to the ftrace directory too.
> 
> I'll try to remember to address this some more next week. Feel free to
> ping me again then.

Have you got chance to run these tests?

Thanks,
Song

Re: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

[Song Liu]

> On Jun 5, 2020, at 3:02 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Fri, 5 Jun 2020 21:58:48 +0000
> Song Liu <songliubraving@fb.com> wrote:
> 
>> 
>> How does this work in your tests? 
> 
> I started it, but got distracted by other work. It did not crash with
> the little testing I did do. I wanted to also look at my patch that
> adds tracing to the ftrace directory too.
> 
> I'll try to remember to address this some more next week. Feel free to
> ping me again then.

It looks like we dropped the ball more than a year ago. 

Adding Andrii and KP, who both hit this limitation in their recent work. 
I also hit this again recently. I was trying to use kernel live patch to 
fix a perf issue, but realized the whole kernel/events directory is not
ftrace-able. :(


Hi Steven, 

I am not sure whether you have got chance to test it more thoroughly. 
Optimistically assuming it didn't crash easily :-) , what would you 
suggest for our next steps? 

Thanks,
Song



Attaching the original RFC/PATCH below. 

From: Song Liu <songliubraving@fb.com>
Subject: [RFC] perf/core: allow ftrace for functions in kernel/event/core.c

It is useful to trace functions in kernel/event/core.c. Allow ftrace for
them by removing $(CC_FLAGS_FTRACE) from Makefile.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
 kernel/events/Makefile | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/kernel/events/Makefile b/kernel/events/Makefile
index 3c022e33c109..8591c180b52b 100644
--- a/kernel/events/Makefile
+++ b/kernel/events/Makefile
@@ -1,10 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-ifdef CONFIG_FUNCTION_TRACER
-CFLAGS_REMOVE_core.o =3D $(CC_FLAGS_FTRACE)
-endif
-
 obj-y :=3D core.o ring_buffer.o callchain.o
=20
 obj-$(CONFIG_HAVE_HW_BREAKPOINT) +=3D hw_breakpoint.o
 obj-$(CONFIG_UPROBES) +=3D uprobes.o
-
--=20
2.24.1