In order to avoid symbol conflicts if they appear in the same binary, a
more unique alias identifier can be generated.
Signed-off-by: Alexey Gladkov <legion@kernel.org>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/module.h | 14 ++++++++++++--
scripts/mod/file2alias.c | 18 ++++++++++++++----
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 88048561360f..e7506684069d 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -249,10 +249,20 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
/* What your module does. */
#define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
+/* Format: __mod_device_table__<counter>__kmod_<modname>__<type>__<name> */
+#define __mod_device_table(type, name) \
+ __PASTE(__mod_device_table__, \
+ __PASTE(__COUNTER__, \
+ __PASTE(__, \
+ __PASTE(__KBUILD_MODNAME, \
+ __PASTE(__, \
+ __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 \
+#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)
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 00586119a25b..dff1799a4c79 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1476,8 +1476,8 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
{
void *symval;
char *zeros = NULL;
- const char *type, *name;
- size_t typelen;
+ const char *type, *name, *modname;
+ size_t typelen, modnamelen;
static const char *prefix = "__mod_device_table__";
/* We're looking for a section relative symbol */
@@ -1488,10 +1488,20 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
return;
- /* All our symbols are of form __mod_device_table__<type>__<name>. */
+ /* All our symbols are of form __mod_device_table__<counter>__kmod_<modname>__<type>__<name>. */
if (!strstarts(symname, prefix))
return;
- type = symname + strlen(prefix);
+
+ modname = strstr(symname, "__kmod_");
+ if (!modname)
+ return;
+ modname += strlen("__kmod_");
+
+ type = strstr(modname, "__");
+ if (!type)
+ return;
+ modnamelen = type - modname;
+ type += strlen("__");
name = strstr(type, "__");
if (!name)
--
2.49.0On Tue, May 27, 2025 at 6:08 PM Alexey Gladkov <legion@kernel.org> wrote: > > In order to avoid symbol conflicts if they appear in the same binary, a > more unique alias identifier can be generated. > > Signed-off-by: Alexey Gladkov <legion@kernel.org> > Reviewed-by: Petr Pavlu <petr.pavlu@suse.com> > --- > include/linux/module.h | 14 ++++++++++++-- > scripts/mod/file2alias.c | 18 ++++++++++++++---- > 2 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index 88048561360f..e7506684069d 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -249,10 +249,20 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name); > /* What your module does. */ > #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description) > > +/* Format: __mod_device_table__<counter>__kmod_<modname>__<type>__<name> */ This format relies on module-name mangling, but I hope we will be able to stop doing it some day. Can we come up with a different idea in case <modname> contains hyphens? -- Best Regards Masahiro Yamada
On Mon, Jun 02, 2025 at 04:52:36PM +0900, Masahiro Yamada wrote: > On Tue, May 27, 2025 at 6:08 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > In order to avoid symbol conflicts if they appear in the same binary, a > > more unique alias identifier can be generated. > > > > Signed-off-by: Alexey Gladkov <legion@kernel.org> > > Reviewed-by: Petr Pavlu <petr.pavlu@suse.com> > > --- > > include/linux/module.h | 14 ++++++++++++-- > > scripts/mod/file2alias.c | 18 ++++++++++++++---- > > 2 files changed, 26 insertions(+), 6 deletions(-) > > > > diff --git a/include/linux/module.h b/include/linux/module.h > > index 88048561360f..e7506684069d 100644 > > --- a/include/linux/module.h > > +++ b/include/linux/module.h > > @@ -249,10 +249,20 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name); > > /* What your module does. */ > > #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description) > > > > +/* Format: __mod_device_table__<counter>__kmod_<modname>__<type>__<name> */ > > This format relies on module-name mangling, but > I hope we will be able to stop doing it some day. I didn't like this approach either when I found out how it was implemented. We can only add the module name to the structure to which the alias is made. But the problem with this is that right now there is no common structure for DEVICE_TABLE. Each module comes up with its own. It is possible to add a common structure, but that would be a big refactoring. > Can we come up with a different idea > in case <modname> contains hyphens? The hyphen is not a problem for my implementation. In the __KBUILD_MODNAME macro, hyphens are replaced by underscores. scripts/Makefile.lib: name-fix-token = $(subst $(comma),_,$(subst -,_,$1)) name-fix = $(call stringify,$(call name-fix-token,$1)) basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget)) modname_flags = -DKBUILD_MODNAME=$(call name-fix,$(modname)) \ -D__KBUILD_MODNAME=kmod_$(call name-fix-token,$(modname)) -- Rgrds, legion
On Mon, Jun 2, 2025 at 5:24 PM Alexey Gladkov <legion@kernel.org> wrote: > > On Mon, Jun 02, 2025 at 04:52:36PM +0900, Masahiro Yamada wrote: > > On Tue, May 27, 2025 at 6:08 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > > > In order to avoid symbol conflicts if they appear in the same binary, a > > > more unique alias identifier can be generated. > > > > > > Signed-off-by: Alexey Gladkov <legion@kernel.org> > > > Reviewed-by: Petr Pavlu <petr.pavlu@suse.com> > > > --- > > > include/linux/module.h | 14 ++++++++++++-- > > > scripts/mod/file2alias.c | 18 ++++++++++++++---- > > > 2 files changed, 26 insertions(+), 6 deletions(-) > > > > > > diff --git a/include/linux/module.h b/include/linux/module.h > > > index 88048561360f..e7506684069d 100644 > > > --- a/include/linux/module.h > > > +++ b/include/linux/module.h > > > @@ -249,10 +249,20 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name); > > > /* What your module does. */ > > > #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description) > > > > > > +/* Format: __mod_device_table__<counter>__kmod_<modname>__<type>__<name> */ > > > > This format relies on module-name mangling, but > > I hope we will be able to stop doing it some day. > > I didn't like this approach either when I found out how it was > implemented. Yeah, I dislike it. I hope we can stop this historical mistake: https://lore.kernel.org/lkml/20250602130609.402581-1-masahiroy@kernel.org/ Once we stop doing that, __KBUILD_MODNAME will not match to KBUILD_MODNAME. Also, you need to be careful about the rust side, as you did not take care of it. https://github.com/torvalds/linux/blob/v6.15/rust/kernel/device_id.rs#L157 > > We can only add the module name to the structure to which the alias is > made. But the problem with this is that right now there is no common > structure for DEVICE_TABLE. Each module comes up with its own. > > It is possible to add a common structure, but that would be a big > refactoring. -- Best Regards Masahiro Yamada
On Tue, Jun 03, 2025 at 03:00:07AM +0900, Masahiro Yamada wrote: > On Mon, Jun 2, 2025 at 5:24 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > On Mon, Jun 02, 2025 at 04:52:36PM +0900, Masahiro Yamada wrote: > > > On Tue, May 27, 2025 at 6:08 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > > > > > In order to avoid symbol conflicts if they appear in the same binary, a > > > > more unique alias identifier can be generated. > > > > > > > > Signed-off-by: Alexey Gladkov <legion@kernel.org> > > > > Reviewed-by: Petr Pavlu <petr.pavlu@suse.com> > > > > --- > > > > include/linux/module.h | 14 ++++++++++++-- > > > > scripts/mod/file2alias.c | 18 ++++++++++++++---- > > > > 2 files changed, 26 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/include/linux/module.h b/include/linux/module.h > > > > index 88048561360f..e7506684069d 100644 > > > > --- a/include/linux/module.h > > > > +++ b/include/linux/module.h > > > > @@ -249,10 +249,20 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name); > > > > /* What your module does. */ > > > > #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description) > > > > > > > > +/* Format: __mod_device_table__<counter>__kmod_<modname>__<type>__<name> */ > > > > > > This format relies on module-name mangling, but > > > I hope we will be able to stop doing it some day. > > > > I didn't like this approach either when I found out how it was > > implemented. > > Yeah, I dislike it. > > I hope we can stop this historical mistake: > https://lore.kernel.org/lkml/20250602130609.402581-1-masahiroy@kernel.org/ > > Once we stop doing that, __KBUILD_MODNAME will not match to KBUILD_MODNAME. Do I understand you correctly that I cannot use __KBUILD_MODNAME now ? > Also, you need to be careful about the rust side, as > you did not take care of it. > > https://github.com/torvalds/linux/blob/v6.15/rust/kernel/device_id.rs#L157 Oh. This will make it much more complicated because I don't know rust well. :( I found a few more issues with modules when they compile as part of the kernel, but was hoping to fix them after these patches. -- Rgrds, legion
On Tue, Jun 3, 2025 at 3:44 AM Alexey Gladkov <legion@kernel.org> wrote: > > On Tue, Jun 03, 2025 at 03:00:07AM +0900, Masahiro Yamada wrote: > > On Mon, Jun 2, 2025 at 5:24 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > > > On Mon, Jun 02, 2025 at 04:52:36PM +0900, Masahiro Yamada wrote: > > > > On Tue, May 27, 2025 at 6:08 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > > > > > > > In order to avoid symbol conflicts if they appear in the same binary, a > > > > > more unique alias identifier can be generated. > > > > > > > > > > Signed-off-by: Alexey Gladkov <legion@kernel.org> > > > > > Reviewed-by: Petr Pavlu <petr.pavlu@suse.com> > > > > > --- > > > > > include/linux/module.h | 14 ++++++++++++-- > > > > > scripts/mod/file2alias.c | 18 ++++++++++++++---- > > > > > 2 files changed, 26 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/include/linux/module.h b/include/linux/module.h > > > > > index 88048561360f..e7506684069d 100644 > > > > > --- a/include/linux/module.h > > > > > +++ b/include/linux/module.h > > > > > @@ -249,10 +249,20 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name); > > > > > /* What your module does. */ > > > > > #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description) > > > > > > > > > > +/* Format: __mod_device_table__<counter>__kmod_<modname>__<type>__<name> */ > > > > > > > > This format relies on module-name mangling, but > > > > I hope we will be able to stop doing it some day. > > > > > > I didn't like this approach either when I found out how it was > > > implemented. > > > > Yeah, I dislike it. > > > > I hope we can stop this historical mistake: > > https://lore.kernel.org/lkml/20250602130609.402581-1-masahiroy@kernel.org/ > > > > Once we stop doing that, __KBUILD_MODNAME will not match to KBUILD_MODNAME. > > Do I understand you correctly that I cannot use __KBUILD_MODNAME now ? Honestly, I dislike __KBUILD_MODNAME, but I know it is more challenging. So, at least you need to fix the rust issue. > > Also, you need to be careful about the rust side, as > > you did not take care of it. > > > > https://github.com/torvalds/linux/blob/v6.15/rust/kernel/device_id.rs#L157 > > Oh. This will make it much more complicated because I don't know rust > well. :( > > I found a few more issues with modules when they compile as part of the > kernel, but was hoping to fix them after these patches. > > -- > Rgrds, legion > -- Best Regards Masahiro Yamada
On Tue, May 27, 2025 at 6:08 PM Alexey Gladkov <legion@kernel.org> wrote:
>
> In order to avoid symbol conflicts if they appear in the same binary, a
> more unique alias identifier can be generated.
Why must this be unique?
What problem would happen if the same symbol names
appear in MODULE_DEVICE_TABLE()?
>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> include/linux/module.h | 14 ++++++++++++--
> scripts/mod/file2alias.c | 18 ++++++++++++++----
> 2 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 88048561360f..e7506684069d 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -249,10 +249,20 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
> /* What your module does. */
> #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
>
> +/* Format: __mod_device_table__<counter>__kmod_<modname>__<type>__<name> */
> +#define __mod_device_table(type, name) \
> + __PASTE(__mod_device_table__, \
> + __PASTE(__COUNTER__, \
> + __PASTE(__, \
> + __PASTE(__KBUILD_MODNAME, \
> + __PASTE(__, \
> + __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 \
> +#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)
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 00586119a25b..dff1799a4c79 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -1476,8 +1476,8 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> {
> void *symval;
> char *zeros = NULL;
> - const char *type, *name;
> - size_t typelen;
> + const char *type, *name, *modname;
> + size_t typelen, modnamelen;
> static const char *prefix = "__mod_device_table__";
>
> /* We're looking for a section relative symbol */
> @@ -1488,10 +1488,20 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
> return;
>
> - /* All our symbols are of form __mod_device_table__<type>__<name>. */
> + /* All our symbols are of form __mod_device_table__<counter>__kmod_<modname>__<type>__<name>. */
> if (!strstarts(symname, prefix))
> return;
> - type = symname + strlen(prefix);
> +
> + modname = strstr(symname, "__kmod_");
> + if (!modname)
> + return;
> + modname += strlen("__kmod_");
> +
> + type = strstr(modname, "__");
> + if (!type)
> + return;
> + modnamelen = type - modname;
> + type += strlen("__");
>
> name = strstr(type, "__");
> if (!name)
> --
> 2.49.0
>
--
Best Regards
Masahiro Yamada
On Mon, Jun 02, 2025 at 04:45:36PM +0900, Masahiro Yamada wrote:
> On Tue, May 27, 2025 at 6:08 PM Alexey Gladkov <legion@kernel.org> wrote:
> >
> > In order to avoid symbol conflicts if they appear in the same binary, a
> > more unique alias identifier can be generated.
>
> Why must this be unique?
>
> What problem would happen if the same symbol names
> appear in MODULE_DEVICE_TABLE()?
Before these patches this was not a problem as non-unique characters are
in separate object files when the module is compiled separately.
But when the modules are compiled into the kernel, there is a symbol
conflict when linking vmlinuz. We have modules that export multiple device
tables from different object files.
>
> >
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> > Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
> > ---
> > include/linux/module.h | 14 ++++++++++++--
> > scripts/mod/file2alias.c | 18 ++++++++++++++----
> > 2 files changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 88048561360f..e7506684069d 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -249,10 +249,20 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
> > /* What your module does. */
> > #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
> >
> > +/* Format: __mod_device_table__<counter>__kmod_<modname>__<type>__<name> */
> > +#define __mod_device_table(type, name) \
> > + __PASTE(__mod_device_table__, \
> > + __PASTE(__COUNTER__, \
> > + __PASTE(__, \
> > + __PASTE(__KBUILD_MODNAME, \
> > + __PASTE(__, \
> > + __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 \
> > +#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)
> > diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> > index 00586119a25b..dff1799a4c79 100644
> > --- a/scripts/mod/file2alias.c
> > +++ b/scripts/mod/file2alias.c
> > @@ -1476,8 +1476,8 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> > {
> > void *symval;
> > char *zeros = NULL;
> > - const char *type, *name;
> > - size_t typelen;
> > + const char *type, *name, *modname;
> > + size_t typelen, modnamelen;
> > static const char *prefix = "__mod_device_table__";
> >
> > /* We're looking for a section relative symbol */
> > @@ -1488,10 +1488,20 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> > if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
> > return;
> >
> > - /* All our symbols are of form __mod_device_table__<type>__<name>. */
> > + /* All our symbols are of form __mod_device_table__<counter>__kmod_<modname>__<type>__<name>. */
> > if (!strstarts(symname, prefix))
> > return;
> > - type = symname + strlen(prefix);
> > +
> > + modname = strstr(symname, "__kmod_");
> > + if (!modname)
> > + return;
> > + modname += strlen("__kmod_");
> > +
> > + type = strstr(modname, "__");
> > + if (!type)
> > + return;
> > + modnamelen = type - modname;
> > + type += strlen("__");
> >
> > name = strstr(type, "__");
> > if (!name)
> > --
> > 2.49.0
> >
>
>
> --
> Best Regards
> Masahiro Yamada
>
--
Rgrds, legion
On Mon, Jun 2, 2025 at 5:07 PM Alexey Gladkov <legion@kernel.org> wrote: > > On Mon, Jun 02, 2025 at 04:45:36PM +0900, Masahiro Yamada wrote: > > On Tue, May 27, 2025 at 6:08 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > > > In order to avoid symbol conflicts if they appear in the same binary, a > > > more unique alias identifier can be generated. > > > > Why must this be unique? > > > > What problem would happen if the same symbol names > > appear in MODULE_DEVICE_TABLE()? > > Before these patches this was not a problem as non-unique characters are > in separate object files when the module is compiled separately. > > But when the modules are compiled into the kernel, there is a symbol > conflict when linking vmlinuz. We have modules that export multiple device > tables from different object files. This is because the __mod_device_table__* symbols are global, but I suspect they do not need to be. Let's test this https://lore.kernel.org/lkml/20250602105539.392362-1-masahiroy@kernel.org/T/#u -- Best Regards Masahiro Yamada
On Mon, Jun 02, 2025 at 07:58:41PM +0900, Masahiro Yamada wrote: > On Mon, Jun 2, 2025 at 5:07 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > On Mon, Jun 02, 2025 at 04:45:36PM +0900, Masahiro Yamada wrote: > > > On Tue, May 27, 2025 at 6:08 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > > > > > In order to avoid symbol conflicts if they appear in the same binary, a > > > > more unique alias identifier can be generated. > > > > > > Why must this be unique? > > > > > > What problem would happen if the same symbol names > > > appear in MODULE_DEVICE_TABLE()? > > > > Before these patches this was not a problem as non-unique characters are > > in separate object files when the module is compiled separately. > > > > But when the modules are compiled into the kernel, there is a symbol > > conflict when linking vmlinuz. We have modules that export multiple device > > tables from different object files. > > This is because the __mod_device_table__* symbols are global, but > I suspect they do not need to be. > > Let's test this > https://lore.kernel.org/lkml/20250602105539.392362-1-masahiroy@kernel.org/T/#u I tested this patch with the config: make allmodconfig make mod2yesconfig and it works. -- Rgrds, legion
On Mon, Jun 2, 2025 at 11:04 PM Alexey Gladkov <legion@kernel.org> wrote: > > On Mon, Jun 02, 2025 at 07:58:41PM +0900, Masahiro Yamada wrote: > > On Mon, Jun 2, 2025 at 5:07 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > > > On Mon, Jun 02, 2025 at 04:45:36PM +0900, Masahiro Yamada wrote: > > > > On Tue, May 27, 2025 at 6:08 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > > > > > > > In order to avoid symbol conflicts if they appear in the same binary, a > > > > > more unique alias identifier can be generated. > > > > > > > > Why must this be unique? > > > > > > > > What problem would happen if the same symbol names > > > > appear in MODULE_DEVICE_TABLE()? > > > > > > Before these patches this was not a problem as non-unique characters are > > > in separate object files when the module is compiled separately. > > > > > > But when the modules are compiled into the kernel, there is a symbol > > > conflict when linking vmlinuz. We have modules that export multiple device > > > tables from different object files. > > > > This is because the __mod_device_table__* symbols are global, but > > I suspect they do not need to be. > > > > Let's test this > > https://lore.kernel.org/lkml/20250602105539.392362-1-masahiroy@kernel.org/T/#u > > I tested this patch with the config: > > make allmodconfig > make mod2yesconfig > > and it works. Good. Then, __COUNTER__ is unnecessary. -- Best Regards Masahiro Yamada
On Tue, Jun 03, 2025 at 01:18:25AM +0900, Masahiro Yamada wrote: > > > > Before these patches this was not a problem as non-unique characters are > > > > in separate object files when the module is compiled separately. > > > > > > > > But when the modules are compiled into the kernel, there is a symbol > > > > conflict when linking vmlinuz. We have modules that export multiple device > > > > tables from different object files. > > > > > > This is because the __mod_device_table__* symbols are global, but > > > I suspect they do not need to be. > > > > > > Let's test this > > > https://lore.kernel.org/lkml/20250602105539.392362-1-masahiroy@kernel.org/T/#u > > > > I tested this patch with the config: > > > > make allmodconfig > > make mod2yesconfig > > > > and it works. > > Good. > Then, __COUNTER__ is unnecessary. I didn't immediately notice. The patch you suggested works, but these symbols remain in System.map and it seems in vmlinuz. -- Rgrds, legion
On Wed, Jun 4, 2025 at 8:26 PM Alexey Gladkov <legion@kernel.org> wrote: > > On Tue, Jun 03, 2025 at 01:18:25AM +0900, Masahiro Yamada wrote: > > > > > Before these patches this was not a problem as non-unique characters are > > > > > in separate object files when the module is compiled separately. > > > > > > > > > > But when the modules are compiled into the kernel, there is a symbol > > > > > conflict when linking vmlinuz. We have modules that export multiple device > > > > > tables from different object files. > > > > > > > > This is because the __mod_device_table__* symbols are global, but > > > > I suspect they do not need to be. > > > > > > > > Let's test this > > > > https://lore.kernel.org/lkml/20250602105539.392362-1-masahiroy@kernel.org/T/#u > > > > > > I tested this patch with the config: > > > > > > make allmodconfig > > > make mod2yesconfig > > > > > > and it works. > > > > Good. > > Then, __COUNTER__ is unnecessary. > > I didn't immediately notice. The patch you suggested works, but these > symbols remain in System.map and it seems in vmlinuz. > Ah, yes, if your patch set is applied. Currently, MODULE_DEVICE_TABLE() is no-op in vmlinux. This makes me realize that your v3 4/6 increased the vmlinux image, as MODULE_DEVICE_TABLE() is kept for modpost. -- Best Regards Masahiro Yamada
On Fri, Jun 6, 2025 at 2:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Jun 4, 2025 at 8:26 PM Alexey Gladkov <legion@kernel.org> wrote:
> >
> > On Tue, Jun 03, 2025 at 01:18:25AM +0900, Masahiro Yamada wrote:
> > > > > > Before these patches this was not a problem as non-unique characters are
> > > > > > in separate object files when the module is compiled separately.
> > > > > >
> > > > > > But when the modules are compiled into the kernel, there is a symbol
> > > > > > conflict when linking vmlinuz. We have modules that export multiple device
> > > > > > tables from different object files.
> > > > >
> > > > > This is because the __mod_device_table__* symbols are global, but
> > > > > I suspect they do not need to be.
> > > > >
> > > > > Let's test this
> > > > > https://lore.kernel.org/lkml/20250602105539.392362-1-masahiroy@kernel.org/T/#u
> > > >
> > > > I tested this patch with the config:
> > > >
> > > > make allmodconfig
> > > > make mod2yesconfig
> > > >
> > > > and it works.
> > >
> > > Good.
> > > Then, __COUNTER__ is unnecessary.
> >
> > I didn't immediately notice. The patch you suggested works, but these
> > symbols remain in System.map and it seems in vmlinuz.
> >
>
> Ah, yes, if your patch set is applied.
>
> Currently, MODULE_DEVICE_TABLE() is no-op in vmlinux.
>
> This makes me realize that your v3 4/6
> increased the vmlinux image, as MODULE_DEVICE_TABLE()
> is kept for modpost.
With your patch set, __mod_device_table_* will be
included in vmlinux.
My patch changes them from global to local ('D' is changed to 'd'),
but there is no difference in the fact that v3 4/6 will grow
the symbol table in vmlinux.
(1) Your patch set
$ arm-linux-gnueabihf-nm vmlinux | grep __mod_device | head -n 10
c0527678 D __mod_device_table__164__kmod_clk_scmi__scmi__scmi_id_table
c053f458 D __mod_device_table__164__kmod_reset_scmi__scmi__scmi_id_table
c05421bc D __mod_device_table__164__kmod_reset_uniphier_glue__of__uniphier_glue_reset_match
c05334ac D __mod_device_table__164__kmod_scmi_pm_domain__scmi__scmi_id_table
c054cbd0 D __mod_device_table__164__kmod_twl4030_power__of__twl4030_power_of_match
c0548e8c D __mod_device_table__165__kmod_omap3_rom_rng__of__omap_rom_rng_match
c05124a0 D __mod_device_table__165__kmod_simple_pm_bus__of__simple_pm_bus_of_match
c05559ac D __mod_device_table__165__kmod_timer_ti_dm__of__omap_timer_match
c0528a68 D __mod_device_table__166__kmod_adpll__of__ti_adpll_match
c0520a68 D __mod_device_table__166__kmod_gpio_en7523__of__airoha_gpio_of_match
(2) Your patch set + my one (extern -> static)
$ arm-linux-gnueabihf-nm vmlinux | grep __mod_device | head -n 10
c0527678 d __mod_device_table__164__kmod_clk_scmi__scmi__scmi_id_table
c053f458 d __mod_device_table__164__kmod_reset_scmi__scmi__scmi_id_table
c05421bc d __mod_device_table__164__kmod_reset_uniphier_glue__of__uniphier_glue_reset_match
c05334ac d __mod_device_table__164__kmod_scmi_pm_domain__scmi__scmi_id_table
c054cbd0 d __mod_device_table__164__kmod_twl4030_power__of__twl4030_power_of_match
c0548e8c d __mod_device_table__165__kmod_omap3_rom_rng__of__omap_rom_rng_match
c05124a0 d __mod_device_table__165__kmod_simple_pm_bus__of__simple_pm_bus_of_match
c05559ac d __mod_device_table__165__kmod_timer_ti_dm__of__omap_timer_match
c0528a68 d __mod_device_table__166__kmod_adpll__of__ti_adpll_match
c0520a68 d __mod_device_table__166__kmod_gpio_en7523__of__airoha_gpio_of_match
--
Best Regards
Masahiro Yamada
On Tue, Jun 03, 2025 at 01:18:25AM +0900, Masahiro Yamada wrote: > On Mon, Jun 2, 2025 at 11:04 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > On Mon, Jun 02, 2025 at 07:58:41PM +0900, Masahiro Yamada wrote: > > > On Mon, Jun 2, 2025 at 5:07 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > > > > > On Mon, Jun 02, 2025 at 04:45:36PM +0900, Masahiro Yamada wrote: > > > > > On Tue, May 27, 2025 at 6:08 PM Alexey Gladkov <legion@kernel.org> wrote: > > > > > > > > > > > > In order to avoid symbol conflicts if they appear in the same binary, a > > > > > > more unique alias identifier can be generated. > > > > > > > > > > Why must this be unique? > > > > > > > > > > What problem would happen if the same symbol names > > > > > appear in MODULE_DEVICE_TABLE()? > > > > > > > > Before these patches this was not a problem as non-unique characters are > > > > in separate object files when the module is compiled separately. > > > > > > > > But when the modules are compiled into the kernel, there is a symbol > > > > conflict when linking vmlinuz. We have modules that export multiple device > > > > tables from different object files. > > > > > > This is because the __mod_device_table__* symbols are global, but > > > I suspect they do not need to be. > > > > > > Let's test this > > > https://lore.kernel.org/lkml/20250602105539.392362-1-masahiroy@kernel.org/T/#u > > > > I tested this patch with the config: > > > > make allmodconfig > > make mod2yesconfig > > > > and it works. > > Good. > Then, __COUNTER__ is unnecessary. I will send a new version asap. Thanks! -- Rgrds, legion
© 2016 - 2025 Red Hat, Inc.