Re: [PATCH v6 1/2] module: Extend module_blacklist parameter to built-in modules

On Wed, Jul 22, 2026 at 03:57:54PM +0200, Petr Pavlu wrote:
> > diff --git a/include/linux/init.h b/include/linux/init.h
> > index 40331923b9f4..1cf163715264 100644
> > --- a/include/linux/init.h
> > +++ b/include/linux/init.h
> > @@ -271,7 +271,28 @@ extern struct module __this_module;
> >  		__initcall_name(initcall, __iid, id),		\
> >  		__initcall_section(__sec, __iid))
> >  
> > -#define ___define_initcall(fn, id, __sec)			\
> > +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> > +#define __initcall_fn_ptr(fn, __iid, id)	__initcall_stub(fn, __iid, id)
> > +#else
> > +#define __initcall_fn_ptr(fn, __iid, id)	fn
> > +#endif
> 
> Nit: I suggest moving the __initcall_fn_ptr() definition into the
> existing '#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS' block that defines
> ____define_initcall() to make the code shorter and group the related
> implementations.

Hi Petr,

Thanks for the thorough review.

Done. Moved __initcall_fn_ptr() into the existing #ifdef block in init.h.

> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 7566815fabbe..fc1525e8f63c 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -86,7 +86,8 @@ extern void cleanup_module(void);
> >   * builtin) or at module insertion time (if a module).  There can only
> >   * be one per module.
> >   */
> > -#define module_init(x)	__initcall(x);
> > +#define module_init(initfn)					\
> > +	____define_initcall_modname(initfn, 6, .initcall6, __initcall_id(initfn))
> 
> The initcall numbering and section naming should remain defined only in
> one place, in include/linux/init.h.
> 
> Could something like the following work?
> 
> In include/linux/module.h:
> 
> #define module_init(initfn) __builtin_module_initcall(initfn);
> 
> In include/linux/init.h:
> 
> #define __define_initcall_modname(fn, id)				\
> 	__define_initcall(fn, id)					\
> 	static const char __initstr_##fn[] __used __aligned(1)		\
> 		__section(".init.rodata") = KBUILD_MODNAME;		\
> 	static const struct initcall_modname __modname_##fn __used	\
> 		__section(".initcall.modnames") = {			\
> 			.initcall_fn = __initcall_fn_ptr(		\
> 			    fn, .initcall##id, id),			\
> 			.modname = __initstr_##fn			\
> 		};
> 
> #define __builtin_module_initcall(fn)	__define_initcall_modname(fn, 6)

Adopted this suggestion. This keeps the initcall section naming and level
numbering localised strictly to include/linux/init.h.

> > +static const char *__init get_builtin_modname(initcall_t fn)
> > +{
> > +	struct initcall_modname *p;
> > +
> > +	for (p = __start_initcall_modnames; p < __stop_initcall_modnames; p++) {
> > +		if (dereference_function_descriptor(p->initcall_fn) ==
> > +		    dereference_function_descriptor(fn))
> 
> Using dereference_function_descriptor() looks unnecessary.

Agreed, removed dereference_function_descriptor() in get_builtin_modname()
and switched to direct pointer comparison.

> Nit: This hunk leaves three consecutive blank lines after
> module_frob_arch_sections().

Acknowledged.

I will include these updates in v7.


Kind regards,
-- 
Aaron Tomlin