Re: [PATCH v1 5/7] modpost: Create modalias for builtin modules

[Petr Pavlu <petr.pavlu@suse.com>]

On 4/29/25 14:49, Alexey Gladkov wrote:
> On Tue, Apr 29, 2025 at 12:04:44PM +0200, Alexey Gladkov wrote:
>>> I'm not sure it's best to overload this data in this way. I think mixing
>>> actual files and "logical" modules in the modules list is somewhat
>>> confusing.
>>>
>>> An alternative would be to keep a single module struct for vmlinux and
>>> record the discovered aliases under it?
>>
>> It is possible to extend struct module_alias and add the module name. The
>> problem is that alias is added by module_alias_printf() and we will have
>> to add the module name to the arguments to each do_entry handler in
>> addition to struct module where there is already a name (but in our case
>> it is vmlinux).
>>
>> I can do that if you think it's a better way.
> 
> If I don't add separate entries for each builtin module, the patch will
> look like this:
> [...]

I see, that didn't turn out as well as I envisioned. One more approach
would be to track builtin modules separately. A patch is below. I'm not
sure if it's better.


diff --git a/include/linux/module.h b/include/linux/module.h
index 7250b4a527ec..6225793ddcd4 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -257,14 +257,10 @@ extern void cleanup_module(void);
 	__PASTE(type,			\
 	__PASTE(__, name)))))))
 
-#ifdef MODULE
 /* Creates an alias so file2alias.c can find device table. */
 #define MODULE_DEVICE_TABLE(type, name)			\
 extern typeof(name) __mod_device_table(type, name)	\
   __attribute__ ((unused, alias(__stringify(name))))
-#else  /* !MODULE */
-#define MODULE_DEVICE_TABLE(type, name)
-#endif
 
 /* Version of form [<epoch>:]<version>[-<extra-version>].
  * Or for CVS/RCS ID version, everything but the number is stripped.
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index dff1799a4c79..28a4c045f66c 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1471,8 +1471,8 @@ static const struct devtable devtable[] = {
 /* Create MODULE_ALIAS() statements.
  * At this time, we cannot write the actual output C source yet,
  * so we write into the mod->dev_table_buf buffer. */
-void handle_moddevtable(struct module *mod, struct elf_info *info,
-			Elf_Sym *sym, const char *symname)
+void handle_moddevtable(struct module *mod, struct elf_info *info, Elf_Sym *sym,
+			const char *symname)
 {
 	void *symval;
 	char *zeros = NULL;
@@ -1509,6 +1509,10 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
 	typelen = name - type;
 	name += strlen("__");
 
+	if (mod->is_vmlinux)
+		// XXX Check if the module doesn't already exist?
+		mod = new_module(modname, modnamelen, true);
+
 	/* Handle all-NULL symbols allocated into .bss */
 	if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
 		zeros = calloc(1, sym->st_size);
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index be89921d60b6..f39e3456e021 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -168,9 +168,12 @@ char *get_line(char **stringp)
 	return orig;
 }
 
-/* A list of all modules we processed */
+/* A list of all modules (vmlinux or *.ko) we processed */
 LIST_HEAD(modules);
 
+/* A list of all builtin modules we processed */
+LIST_HEAD(builtin_modules);
+
 static struct module *find_module(const char *filename, const char *modname)
 {
 	struct module *mod;
@@ -183,7 +186,7 @@ static struct module *find_module(const char *filename, const char *modname)
 	return NULL;
 }
 
-static struct module *new_module(const char *name, size_t namelen)
+struct module *new_module(const char *name, size_t namelen, bool is_builtin)
 {
 	struct module *mod;
 
@@ -207,7 +210,10 @@ static struct module *new_module(const char *name, size_t namelen)
 	 */
 	mod->is_gpl_compatible = true;
 
-	list_add_tail(&mod->list, &modules);
+	if (is_builtin)
+		list_add_tail(&mod->list, &builtin_modules);
+	else
+		list_add_tail(&mod->list, &modules);
 
 	return mod;
 }
@@ -1573,7 +1579,7 @@ static void read_symbols(const char *modname)
 	}
 
 	/* strip trailing .o */
-	mod = new_module(modname, strlen(modname) - strlen(".o"));
+	mod = new_module(modname, strlen(modname) - strlen(".o"), false);
 
 	/* save .no_trim_symbol section for later use */
 	if (info.no_trim_symbol_len) {
@@ -2021,11 +2027,23 @@ static void write_if_changed(struct buffer *b, const char *fname)
 static void write_vmlinux_export_c_file(struct module *mod)
 {
 	struct buffer buf = { };
+	struct module_alias *alias, *next;
 
 	buf_printf(&buf,
-		   "#include <linux/export-internal.h>\n");
+		   "#include <linux/export-internal.h>\n"
+		   "#include <linux/module.h>\n");
 
 	add_exported_symbols(&buf, mod);
+
+	list_for_each_entry(mod, &builtin_modules, list) {
+		list_for_each_entry_safe(alias, next, &mod->aliases, node) {
+			buf_printf(&buf, "MODULE_ALIAS_MODNAME(\"%s\", \"%s\");\n",
+					mod->name, alias->str);
+			list_del(&alias->node);
+			free(alias);
+		}
+	}
+
 	write_if_changed(&buf, ".vmlinux.export.c");
 	free(buf.p);
 }
@@ -2114,7 +2132,7 @@ static void read_dump(const char *fname)
 
 		mod = find_module(fname, modname);
 		if (!mod) {
-			mod = new_module(modname, strlen(modname));
+			mod = new_module(modname, strlen(modname), false);
 			mod->dump_file = fname;
 		}
 		s = sym_add_exported(symname, mod, gpl_only, namespace);
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 9133e4c3803f..f2b6d25f00ff 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -107,7 +107,7 @@ struct module_alias {
 };
 
 /**
- * struct module - represent a module (vmlinux or *.ko)
+ * struct module - represent a module (vmlinux, a builtin module, or *.ko)
  *
  * @dump_file: path to the .symvers file if loaded from a file
  * @aliases: list head for module_aliases
@@ -199,6 +199,8 @@ static inline bool is_valid_name(struct elf_info *elf, Elf_Sym *sym)
 	return !is_mapping_symbol(name);
 }
 
+struct module *new_module(const char *name, size_t namelen, bool is_builtin);
+
 /* symsearch.c */
 void symsearch_init(struct elf_info *elf);
 void symsearch_finish(struct elf_info *elf);

-- 
Cheers,
Petr