Re: [PATCH] init/main.c: log initcall level when initcall_debug is used

Hi Tim,

On Tuesday, 1 April 2025 at 19:57:22 Bird, Tim <[email protected]> wrote:
> > -----Original Message-----
> > From: Francesco Valla <[email protected]>
> > When initcall_debug is specified on the command line, the start and
> > return point for each initcall is printed. However, no information on
> > the initcall level is reported.
> > 
> > Add to the initcall_debug infrastructure an additional print that
> > informs when a new initcall level is entered. This is particularly
> > useful when debugging dependency chains and/or working on boot time
> > reduction.
> > 
> > Signed-off-by: Francesco Valla <[email protected]>
> > ---
> >  init/main.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/init/main.c b/init/main.c
> > index 2a1757826397..80a07563036d 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -1214,6 +1214,12 @@ trace_initcall_finish_cb(void *data, initcall_t fn, int ret)
> >  		 fn, ret, (unsigned long long)ktime_us_delta(rettime, *calltime));
> >  }
> > 
> > +static __init_or_module void
> > +trace_initcall_level_cb(void *data, const char *level)
> > +{
> > +	printk(KERN_DEBUG "entering initcall level: %s\n", level);
> > +}
> > +
> >  static ktime_t initcall_calltime;
> > 
> >  #ifdef TRACEPOINTS_ENABLED
> > @@ -1225,10 +1231,12 @@ static void __init initcall_debug_enable(void)
> >  					    &initcall_calltime);
> >  	ret |= register_trace_initcall_finish(trace_initcall_finish_cb,
> >  					      &initcall_calltime);
> > +	ret |= register_trace_initcall_level(trace_initcall_level_cb, NULL);
> >  	WARN(ret, "Failed to register initcall tracepoints\n");
> >  }
> >  # define do_trace_initcall_start	trace_initcall_start
> >  # define do_trace_initcall_finish	trace_initcall_finish
> > +# define do_trace_initcall_level	trace_initcall_level
> >  #else
> >  static inline void do_trace_initcall_start(initcall_t fn)
> >  {
> > @@ -1242,6 +1250,12 @@ static inline void do_trace_initcall_finish(initcall_t fn, int ret)
> >  		return;
> >  	trace_initcall_finish_cb(&initcall_calltime, fn, ret);
> >  }
> > +static inline void do_trace_initcall_level(const char *level)
> > +{
> > +	if (!initcall_debug)
> > +		return;
> > +	trace_initcall_level_cb(NULL, level);
> > +}
> >  #endif /* !TRACEPOINTS_ENABLED */
> > 
> >  int __init_or_module do_one_initcall(initcall_t fn)
> > @@ -1314,7 +1328,7 @@ static void __init do_initcall_level(int level, char *command_line)
> >  		   level, level,
> >  		   NULL, ignore_unknown_bootoption);
> > 
> > -	trace_initcall_level(initcall_level_names[level]);
> > +	do_trace_initcall_level(initcall_level_names[level]);
> >  	for (fn = initcall_levels[level]; fn < initcall_levels[level+1]; fn++)
> >  		do_one_initcall(initcall_from_entry(fn));
> >  }
> > @@ -1358,7 +1372,7 @@ static void __init do_pre_smp_initcalls(void)
> >  {
> >  	initcall_entry_t *fn;
> > 
> > -	trace_initcall_level("early");
> > +	do_trace_initcall_level("early");
> >  	for (fn = __initcall_start; fn < __initcall0_start; fn++)
> >  		do_one_initcall(initcall_from_entry(fn));
> >  }
> > --
> > 2.48.1
> 
> This all looks good to me.  Just to clarify, does tracing have to be enabled to get an
> the 'entering initcall level...' printk message?  Or will you get a printk message with
> tracing disabled, but initcall_debug specified on the command line?
> 

No, tracing doesn't have to be enabled. We are just attaching to the tracing subsystem
if it's there, just like it is done for the "calling" / "initcall returned" messages.

> What do we need to do to push this into mainline?  Based on our discussion in the SIG
> meeting, there's no official maintainer for init/main.c.  I recommend pushing this
> through Andrew Morton's tree, unless we can think of a better tree to push it through.
> Since it does affect a tracer, maybe through Steve's tree?
> 
> Another option is for you, Francesco, to become the maintainer of init/main.c (!)
> Let me know if you're interested in that.  We'll likely have some more boot-time
> things to work on in init/main.c, and it would be nice to have someone managing
> this stuff as it comes in.
> 

I fear I don't have the knowledge to maintain init/main.c, unfortunately. I can
for sure take a look at other MRs related to it, but that's it - I don't wan't to
take a commitment I cannot / am not able to honor.

> Thanks, Francesco, for proposing this patch.  I think it will obviate the need for a portion of
> my Boot Markers patch, that I suggested at Plumbers last year.
>  -- Tim
> 

Thank you for the review!

Francesco