| Message ID | 20221219204619.2205248-6-allenwebb@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Generate modules.builtin.alias from match ids | expand |
On Mon, Dec 19, 2022 at 02:46:13PM -0600, Allen Webb wrote: > Implement MODULE_DEVICE_TABLE for build-in modules to make it possible > to generate a builtin.alias file to complement modules.alias. > > Signed-off-by: Allen Webb <allenwebb@google.com> > --- > include/linux/module.h | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index ec61fb53979a..3d1b04ca6350 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -243,7 +243,20 @@ extern void cleanup_module(void); > extern typeof(name) __mod_##type##__##name##_device_table \ > __attribute__ ((unused, alias(__stringify(name)))) > #else /* !MODULE */ > -#define MODULE_DEVICE_TABLE(type, name) > +/* > + * The names may not be unique for built-in modules, so include the module name > + * to guarantee uniqueness. What "names" are you referring to here with the words, "The names"? And built-in modules have the same rules as external names, they have to be unique so I do not understand the problem you are trying to solve here, which means you need to describe it better in both the changelog text and the comment. > + * > + * Note that extern is needed because modpost reads these symbols to generate > + * modalias entries for each match id in each device table. They are not used > + * at runtime. This comment isn't explaining much about what the #define is to be used for, is it? confused, greg k-h
On Tue, Dec 20, 2022 at 12:45 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Mon, Dec 19, 2022 at 02:46:13PM -0600, Allen Webb wrote: > > Implement MODULE_DEVICE_TABLE for build-in modules to make it possible > > to generate a builtin.alias file to complement modules.alias. > > > > Signed-off-by: Allen Webb <allenwebb@google.com> > > --- > > include/linux/module.h | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/module.h b/include/linux/module.h > > index ec61fb53979a..3d1b04ca6350 100644 > > --- a/include/linux/module.h > > +++ b/include/linux/module.h > > @@ -243,7 +243,20 @@ extern void cleanup_module(void); > > extern typeof(name) __mod_##type##__##name##_device_table \ > > __attribute__ ((unused, alias(__stringify(name)))) > > #else /* !MODULE */ > > -#define MODULE_DEVICE_TABLE(type, name) > > +/* > > + * The names may not be unique for built-in modules, so include the module name > > + * to guarantee uniqueness. > > What "names" are you referring to here with the words, "The names"? > > And built-in modules have the same rules as external names, they have to > be unique so I do not understand the problem you are trying to solve > here, which means you need to describe it better in both the changelog > text and the comment. I changed the comment to: /* * Creates an alias so file2alias.c can find device table for built in modules. * * The module name is included for two reasons: * - Adding the module name to the alias avoids creating two aliases with the * same name. Historically MODULE_DEVICE_TABLE was a no-op for built-in * modules, so there was nothing to stop different modules from having the * same device table name and consequently the same alias when building as a * module. * - The module name is needed by files2alias.c to associate a particular * device table with its associated module since files2alias would otherwise * see the module name as `vmlinuz.o` for built-in modules. */ > > > + * > > + * Note that extern is needed because modpost reads these symbols to generate > > + * modalias entries for each match id in each device table. They are not used > > + * at runtime. > > This comment isn't explaining much about what the #define is to be used > for, is it? I will drop this. I originally added the comment because Christophe Leroy said: "'extern' keyword is pointless of function prototypes and deprecated. Don't add new occurences." This is clearly not a typical function prototype and the guidance from: https://www.kernel.org/doc/html/latest/process/coding-style.html#function-prototypes should not apply. > > confused, > > greg k-h
diff --git a/include/linux/module.h b/include/linux/module.h index ec61fb53979a..3d1b04ca6350 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -243,7 +243,20 @@ extern void cleanup_module(void); extern typeof(name) __mod_##type##__##name##_device_table \ __attribute__ ((unused, alias(__stringify(name)))) #else /* !MODULE */ -#define MODULE_DEVICE_TABLE(type, name) +/* + * The names may not be unique for built-in modules, so include the module name + * to guarantee uniqueness. + * + * Note that extern is needed because modpost reads these symbols to generate + * modalias entries for each match id in each device table. They are not used + * at runtime. + */ +#define MODULE_DEVICE_TABLE(type, name) \ +extern void *CONCATENATE( \ + CONCATENATE(__mod_##type##__##name##__, \ + __KBUILD_MODNAME), \ + _device_table) \ + __attribute__ ((unused, alias(__stringify(name)))) #endif /* Version of form [<epoch>:]<version>[-<extra-version>].
Implement MODULE_DEVICE_TABLE for build-in modules to make it possible to generate a builtin.alias file to complement modules.alias. Signed-off-by: Allen Webb <allenwebb@google.com> --- include/linux/module.h | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)