lkml.org 
[lkml]   [2022]   [Mar]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] tracing: make tracer_init_tracefs initcall asynchronous
Date
> > On Fri, 11 Mar 2022 19:26:56 +0800
> > Mark-PK Tsai <mark-pk.tsai@mediatek.com> wrote:
> >
> > > tracer_init_tracefs() is slow especially when there are
> > > lots of trace events.
> > > Create a kthread to do tracer_init_tracefs() asynchronously
> >
> > When making comments like this, please provide the benchmarks you used,
> > with the numbers before and after.
>
> I've retest it with kernel 5.17-rc7 on my arm64 board, and I found that
> the critical path is trace_eval_sync which spend about 430 ms.
> It's almost half of the time the do_initcalls spends.
> Below is the test result.
>
> before after
> tracer_init_tracefs 29872 us 66 us
> trace_eval_sync 429695 us 459370 us
> do_initcalls 797818 us 799890 us
>
> I locally skip trace_eval_sync and got below result.
>
> before after diff
> do_initcalls 359252 us 341725 us -17527 us
>
> So beside this patch, could we add a kernel parameter or a
> option to skip it when it doesn't used right after kernel boot?
>

Please ignore this.
I do more tests and found that ftrace_eval_maps is in INIT_DATA.
So the eval_wq may crash if it doesn't finish before init mem
is freed.
And ftrace_eval_maps is big when there are lots of trace event
(About 6KB with arch/arm64/defconfig).
So it seems not a good idea to remove all the needed funcs from init.

So please review v2 [1] I've just push which queue the tracer_init_tracefs()
to eval_map_wq.

[1]: https://lore.kernel.org/lkml/20220316151639.9216-1-mark-pk.tsai@mediatek.com/

> >
> > > to speed up the initialization of kernel and move the
> > > related functions and variables out of init section.
> >
> > Thus we sacrifice memory for boot time. I'd like to also see how much
> > memory is freed from init before and after this patch.
> >
>
> Below is the INIT_TEXT and INIT_DATA diff:
>
> before after diff
> INIT_TEXT 7F290 7EDAC -0x4e4 bytes
> INIT_DATA 116FE8 116FE8 0 bytes
>
> And the init section is 64K aligned on arm64 so that when I test
> on my platform, the actual memory freed by initmem_free() have no
> diffrence after apply this patch.
>
> #define SEGMENT_ALIGN SZ_64K
>
> > >
> > > static int trace_panic_handler(struct notifier_block *this,
> > > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> > > index 3147614c1812..fe055bef1e8f 100644
> > > --- a/kernel/trace/trace_events.c
> > > +++ b/kernel/trace/trace_events.c
> > > @@ -3687,7 +3687,7 @@ static __init int event_trace_init_fields(void)
> > > return 0;
> > > }
> > >
> > > -__init int event_trace_init(void)
> > > +int event_trace_init(void)
> > > {
> > > struct trace_array *tr;
> > > struct dentry *entry;
> >
> > Hmm, this calls early_event_tracer() which is also in __init. Looks like
> > there's going to be a ripple effect due to this change.
> >
> > If we want to go this route, then first a change must be made to remove the
> > needed functions from init, and then see if we can consolidate it. As there
> > are some init functions that are duplicated for init purposes.
>
> Got it!
>


\
 
 \ /
  Last update: 2022-09-17 16:17    [W:0.049 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site