Messages in this thread | | | From | Mark-PK Tsai <> | Subject | Re: [PATCH] tracing: make tracer_init_tracefs initcall asynchronous | Date | Wed, 16 Mar 2022 23:58:16 +0800 |
| |
> > 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! >
| |