| Message ID | 20231126071914.932241-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | init: move THIS_MODULE from <linux/export.h> to <linux/init.h> | expand |
On Sun, Nov 26, 2023 at 4:19 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Commit f50169324df4 ("module.h: split out the EXPORT_SYMBOL into > export.h") appropriately separated EXPORT_SYMBOL into <linux/export.h> > because modules and EXPORT_SYMBOL are orthogonal; modules are symbol > consumers, while EXPORT_SYMBOL are used by symbol providers, which > may not be necessarily a module. > > However, that commit also relocated THIS_MODULE. As explained in the > commit description, the intention was to define THIS_MODULE in a > lightweight header, but I do not believe <linux/export.h> was the > suitable location because EXPORT_SYMBOL and THIS_MODULE are unrelated. > > Move it to another lightweight header, <linux/init.h>. The reason for > choosing <linux/init.h> is to make <linux/moduleparam.h> self-contained > without relying on <linux/linkage.h> incorrectly including > <linux/export.h>. > > With this adjustment, the role of <linux/export.h> becomes clearer as > it only defines EXPORT_SYMBOL. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- Applied to kbuild. I did not get any report from the 0day bot so far, but I hope it will get a little more compile tests before getting into linux-next. > > include/linux/export.h | 18 ------------------ > include/linux/init.h | 7 +++++++ > 2 files changed, 7 insertions(+), 18 deletions(-) > > diff --git a/include/linux/export.h b/include/linux/export.h > index 9911508a9604..0bbd02fd351d 100644 > --- a/include/linux/export.h > +++ b/include/linux/export.h > @@ -6,15 +6,6 @@ > #include <linux/linkage.h> > #include <linux/stringify.h> > > -/* > - * Export symbols from the kernel to modules. Forked from module.h > - * to reduce the amount of pointless cruft we feed to gcc when only > - * exporting a simple symbol or two. > - * > - * Try not to add #includes here. It slows compilation and makes kernel > - * hackers place grumpy comments in header files. > - */ > - > /* > * This comment block is used by fixdep. Please do not remove. > * > @@ -23,15 +14,6 @@ > * side effect of the *.o build rule. > */ > > -#ifndef __ASSEMBLY__ > -#ifdef MODULE > -extern struct module __this_module; > -#define THIS_MODULE (&__this_module) > -#else > -#define THIS_MODULE ((struct module *)0) > -#endif > -#endif /* __ASSEMBLY__ */ > - > #ifdef CONFIG_64BIT > #define __EXPORT_SYMBOL_REF(sym) \ > .balign 8 ASM_NL \ > diff --git a/include/linux/init.h b/include/linux/init.h > index 01b52c9c7526..3fa3f6241350 100644 > --- a/include/linux/init.h > +++ b/include/linux/init.h > @@ -179,6 +179,13 @@ extern void (*late_time_init)(void); > > extern bool initcall_debug; > > +#ifdef MODULE > +extern struct module __this_module; > +#define THIS_MODULE (&__this_module) > +#else > +#define THIS_MODULE ((struct module *)0) > +#endif > + > #endif > > #ifndef MODULE > -- > 2.40.1 >
On Sun, Nov 26, 2023 at 04:19:14PM +0900, Masahiro Yamada wrote: > Commit f50169324df4 ("module.h: split out the EXPORT_SYMBOL into > export.h") appropriately separated EXPORT_SYMBOL into <linux/export.h> > because modules and EXPORT_SYMBOL are orthogonal; modules are symbol > consumers, while EXPORT_SYMBOL are used by symbol providers, which > may not be necessarily a module. > > However, that commit also relocated THIS_MODULE. As explained in the > commit description, the intention was to define THIS_MODULE in a > lightweight header, but I do not believe <linux/export.h> was the > suitable location because EXPORT_SYMBOL and THIS_MODULE are unrelated. > > Move it to another lightweight header, <linux/init.h>. The reason for > choosing <linux/init.h> is to make <linux/moduleparam.h> self-contained > without relying on <linux/linkage.h> incorrectly including > <linux/export.h>. > > With this adjustment, the role of <linux/export.h> becomes clearer as > it only defines EXPORT_SYMBOL. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> Do you want this this to go through modules-next or your tree? I'm fine it goes either way. Luis
[Re: [PATCH] init: move THIS_MODULE from <linux/export.h> to <linux/init.h>] On 03/12/2023 (Sun 19:06) Masahiro Yamada wrote: > On Sun, Nov 26, 2023 at 4:19???PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > Commit f50169324df4 ("module.h: split out the EXPORT_SYMBOL into > > export.h") appropriately separated EXPORT_SYMBOL into <linux/export.h> > > because modules and EXPORT_SYMBOL are orthogonal; modules are symbol > > consumers, while EXPORT_SYMBOL are used by symbol providers, which > > may not be necessarily a module. > > > > However, that commit also relocated THIS_MODULE. As explained in the > > commit description, the intention was to define THIS_MODULE in a > > lightweight header, but I do not believe <linux/export.h> was the > > suitable location because EXPORT_SYMBOL and THIS_MODULE are unrelated. > > > > Move it to another lightweight header, <linux/init.h>. The reason for > > choosing <linux/init.h> is to make <linux/moduleparam.h> self-contained > > without relying on <linux/linkage.h> incorrectly including > > <linux/export.h>. > > > > With this adjustment, the role of <linux/export.h> becomes clearer as > > it only defines EXPORT_SYMBOL. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > Applied to kbuild. > > I did not get any report from the 0day bot so far, > but I hope it will get a little more compile tests > before getting into linux-next. Haven't touched that kind of header shuffle for over 10 years? But yeah, it is near impossible to not trip over some implicit header inclusion somewhere in some driver or a less common arch and hence break the build at least once when doing this kind of stuff. Paul. -- > > > > > > > include/linux/export.h | 18 ------------------ > > include/linux/init.h | 7 +++++++ > > 2 files changed, 7 insertions(+), 18 deletions(-) > > > > diff --git a/include/linux/export.h b/include/linux/export.h > > index 9911508a9604..0bbd02fd351d 100644 > > --- a/include/linux/export.h > > +++ b/include/linux/export.h > > @@ -6,15 +6,6 @@ > > #include <linux/linkage.h> > > #include <linux/stringify.h> > > > > -/* > > - * Export symbols from the kernel to modules. Forked from module.h > > - * to reduce the amount of pointless cruft we feed to gcc when only > > - * exporting a simple symbol or two. > > - * > > - * Try not to add #includes here. It slows compilation and makes kernel > > - * hackers place grumpy comments in header files. > > - */ > > - > > /* > > * This comment block is used by fixdep. Please do not remove. > > * > > @@ -23,15 +14,6 @@ > > * side effect of the *.o build rule. > > */ > > > > -#ifndef __ASSEMBLY__ > > -#ifdef MODULE > > -extern struct module __this_module; > > -#define THIS_MODULE (&__this_module) > > -#else > > -#define THIS_MODULE ((struct module *)0) > > -#endif > > -#endif /* __ASSEMBLY__ */ > > - > > #ifdef CONFIG_64BIT > > #define __EXPORT_SYMBOL_REF(sym) \ > > .balign 8 ASM_NL \ > > diff --git a/include/linux/init.h b/include/linux/init.h > > index 01b52c9c7526..3fa3f6241350 100644 > > --- a/include/linux/init.h > > +++ b/include/linux/init.h > > @@ -179,6 +179,13 @@ extern void (*late_time_init)(void); > > > > extern bool initcall_debug; > > > > +#ifdef MODULE > > +extern struct module __this_module; > > +#define THIS_MODULE (&__this_module) > > +#else > > +#define THIS_MODULE ((struct module *)0) > > +#endif > > + > > #endif > > > > #ifndef MODULE > > -- > > 2.40.1 > > > > > -- > Best Regards > Masahiro Yamada
On Tue, Dec 5, 2023 at 2:54 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Sun, Nov 26, 2023 at 04:19:14PM +0900, Masahiro Yamada wrote: > > Commit f50169324df4 ("module.h: split out the EXPORT_SYMBOL into > > export.h") appropriately separated EXPORT_SYMBOL into <linux/export.h> > > because modules and EXPORT_SYMBOL are orthogonal; modules are symbol > > consumers, while EXPORT_SYMBOL are used by symbol providers, which > > may not be necessarily a module. > > > > However, that commit also relocated THIS_MODULE. As explained in the > > commit description, the intention was to define THIS_MODULE in a > > lightweight header, but I do not believe <linux/export.h> was the > > suitable location because EXPORT_SYMBOL and THIS_MODULE are unrelated. > > > > Move it to another lightweight header, <linux/init.h>. The reason for > > choosing <linux/init.h> is to make <linux/moduleparam.h> self-contained > > without relying on <linux/linkage.h> incorrectly including > > <linux/export.h>. > > > > With this adjustment, the role of <linux/export.h> becomes clearer as > > it only defines EXPORT_SYMBOL. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> I will fold your reviewed-by tag. Thanks. > > Do you want this this to go through modules-next or your tree? I'm fine > it goes either way. > > Luis
diff --git a/include/linux/export.h b/include/linux/export.h index 9911508a9604..0bbd02fd351d 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -6,15 +6,6 @@ #include <linux/linkage.h> #include <linux/stringify.h> -/* - * Export symbols from the kernel to modules. Forked from module.h - * to reduce the amount of pointless cruft we feed to gcc when only - * exporting a simple symbol or two. - * - * Try not to add #includes here. It slows compilation and makes kernel - * hackers place grumpy comments in header files. - */ - /* * This comment block is used by fixdep. Please do not remove. * @@ -23,15 +14,6 @@ * side effect of the *.o build rule. */ -#ifndef __ASSEMBLY__ -#ifdef MODULE -extern struct module __this_module; -#define THIS_MODULE (&__this_module) -#else -#define THIS_MODULE ((struct module *)0) -#endif -#endif /* __ASSEMBLY__ */ - #ifdef CONFIG_64BIT #define __EXPORT_SYMBOL_REF(sym) \ .balign 8 ASM_NL \ diff --git a/include/linux/init.h b/include/linux/init.h index 01b52c9c7526..3fa3f6241350 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -179,6 +179,13 @@ extern void (*late_time_init)(void); extern bool initcall_debug; +#ifdef MODULE +extern struct module __this_module; +#define THIS_MODULE (&__this_module) +#else +#define THIS_MODULE ((struct module *)0) +#endif + #endif #ifndef MODULE
Commit f50169324df4 ("module.h: split out the EXPORT_SYMBOL into export.h") appropriately separated EXPORT_SYMBOL into <linux/export.h> because modules and EXPORT_SYMBOL are orthogonal; modules are symbol consumers, while EXPORT_SYMBOL are used by symbol providers, which may not be necessarily a module. However, that commit also relocated THIS_MODULE. As explained in the commit description, the intention was to define THIS_MODULE in a lightweight header, but I do not believe <linux/export.h> was the suitable location because EXPORT_SYMBOL and THIS_MODULE are unrelated. Move it to another lightweight header, <linux/init.h>. The reason for choosing <linux/init.h> is to make <linux/moduleparam.h> self-contained without relying on <linux/linkage.h> incorrectly including <linux/export.h>. With this adjustment, the role of <linux/export.h> becomes clearer as it only defines EXPORT_SYMBOL. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- include/linux/export.h | 18 ------------------ include/linux/init.h | 7 +++++++ 2 files changed, 7 insertions(+), 18 deletions(-)