| Message ID | 20190927093603.9140-5-yamada.masahiro@socionext.com |
|---|---|
| State | Accepted |
| Commit | c3a6cf19e695c8b0a9bf8b5933f863e12d878b7c |
| Headers | show |
| Series | module: various bug-fixes and clean-ups for module namespace | expand |
On Fri, Sep 27, 2019 at 6:37 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > include/linux/export.h has lots of code duplication between > EXPORT_SYMBOL and EXPORT_SYMBOL_NS. > > To improve the maintainability and readability, unify the > implementation. > > When the symbol has no namespace, pass the empty string "" to > the 'ns' parameter. > > The drawback of this change is, it grows the code size. > When the symbol has no namespace, sym->namespace was previously > NULL, but it is now am empty string "". So, it increases 1 byte Just a nit. I meant "an empty string" instead of "am empty string". > for every no namespace EXPORT_SYMBOL. > > A typical kernel configuration has 10K exported symbols, so it > increases 10KB in rough estimation. > > I did not come up with a good idea to refactor it without increasing > the code size. > > I am not sure how big a deal it is, but at least include/linux/export.h > looks nicer. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > include/linux/export.h | 100 +++++++++++++---------------------------- > kernel/module.c | 2 +- > 2 files changed, 33 insertions(+), 69 deletions(-) > > diff --git a/include/linux/export.h b/include/linux/export.h > index 621158ecd2e2..55245a405a2f 100644 > --- a/include/linux/export.h > +++ b/include/linux/export.h > @@ -48,45 +48,28 @@ extern struct module __this_module; > * absolute relocations that require runtime processing on relocatable > * kernels. > */ > -#define __KSYMTAB_ENTRY_NS(sym, sec, ns) \ > +#define __KSYMTAB_ENTRY(sym, sec, ns) \ > __ADDRESSABLE(sym) \ > asm(" .section \"___ksymtab" sec "+" #sym "\", \"a\" \n" \ > " .balign 4 \n" \ > - "__ksymtab_" #ns NS_SEPARATOR #sym ": \n" \ > + "__ksymtab_" ns NS_SEPARATOR #sym ": \n" \ > " .long " #sym "- . \n" \ > " .long __kstrtab_" #sym "- . \n" \ > " .long __kstrtabns_" #sym "- . \n" \ > " .previous \n") > > -#define __KSYMTAB_ENTRY(sym, sec) \ > - __ADDRESSABLE(sym) \ > - asm(" .section \"___ksymtab" sec "+" #sym "\", \"a\" \n" \ > - " .balign 4 \n" \ > - "__ksymtab_" #sym ": \n" \ > - " .long " #sym "- . \n" \ > - " .long __kstrtab_" #sym "- . \n" \ > - " .long 0 \n" \ > - " .previous \n") > - > struct kernel_symbol { > int value_offset; > int name_offset; > int namespace_offset; > }; > #else > -#define __KSYMTAB_ENTRY_NS(sym, sec, ns) \ > - static const struct kernel_symbol __ksymtab_##sym##__##ns \ > - asm("__ksymtab_" #ns NS_SEPARATOR #sym) \ > - __attribute__((section("___ksymtab" sec "+" #sym), used)) \ > - __aligned(sizeof(void *)) \ > - = { (unsigned long)&sym, __kstrtab_##sym, __kstrtabns_##sym } > - > -#define __KSYMTAB_ENTRY(sym, sec) \ > +#define __KSYMTAB_ENTRY(sym, sec, ns) \ > static const struct kernel_symbol __ksymtab_##sym \ > - asm("__ksymtab_" #sym) \ > + asm("__ksymtab_" ns NS_SEPARATOR #sym) \ > __attribute__((section("___ksymtab" sec "+" #sym), used)) \ > __aligned(sizeof(void *)) \ > - = { (unsigned long)&sym, __kstrtab_##sym, NULL } > + = { (unsigned long)&sym, __kstrtab_##sym, __kstrtabns_##sym } > > struct kernel_symbol { > unsigned long value; > @@ -97,29 +80,21 @@ struct kernel_symbol { > > #ifdef __GENKSYMS__ > > -#define ___EXPORT_SYMBOL(sym,sec) __GENKSYMS_EXPORT_SYMBOL(sym) > -#define ___EXPORT_SYMBOL_NS(sym,sec,ns) __GENKSYMS_EXPORT_SYMBOL(sym) > +#define ___EXPORT_SYMBOL(sym, sec, ns) __GENKSYMS_EXPORT_SYMBOL(sym) > > #else > > -#define ___export_symbol_common(sym, sec) \ > +/* For every exported symbol, place a struct in the __ksymtab section */ > +#define ___EXPORT_SYMBOL(sym, sec, ns) \ > extern typeof(sym) sym; \ > __CRC_SYMBOL(sym, sec); \ > static const char __kstrtab_##sym[] \ > __attribute__((section("__ksymtab_strings"), used, aligned(1))) \ > - = #sym \ > - > -/* For every exported symbol, place a struct in the __ksymtab section */ > -#define ___EXPORT_SYMBOL_NS(sym, sec, ns) \ > - ___export_symbol_common(sym, sec); \ > + = #sym; \ > static const char __kstrtabns_##sym[] \ > __attribute__((section("__ksymtab_strings"), used, aligned(1))) \ > - = #ns; \ > - __KSYMTAB_ENTRY_NS(sym, sec, ns) > - > -#define ___EXPORT_SYMBOL(sym, sec) \ > - ___export_symbol_common(sym, sec); \ > - __KSYMTAB_ENTRY(sym, sec) > + = ns; \ > + __KSYMTAB_ENTRY(sym, sec, ns) > > #endif > > @@ -130,8 +105,7 @@ struct kernel_symbol { > * be reused in other execution contexts such as the UEFI stub or the > * decompressor. > */ > -#define __EXPORT_SYMBOL_NS(sym, sec, ns) > -#define __EXPORT_SYMBOL(sym, sec) > +#define __EXPORT_SYMBOL(sym, sec, ns) > > #elif defined(CONFIG_TRIM_UNUSED_KSYMS) > > @@ -147,48 +121,38 @@ struct kernel_symbol { > #define __ksym_marker(sym) \ > static int __ksym_marker_##sym[0] __section(".discard.ksym") __used > > -#define __EXPORT_SYMBOL(sym, sec) \ > +#define __EXPORT_SYMBOL(sym, sec, ns) \ > __ksym_marker(sym); \ > - __cond_export_sym(sym, sec, __is_defined(__KSYM_##sym)) > -#define __cond_export_sym(sym, sec, conf) \ > - ___cond_export_sym(sym, sec, conf) > -#define ___cond_export_sym(sym, sec, enabled) \ > - __cond_export_sym_##enabled(sym, sec) > -#define __cond_export_sym_1(sym, sec) ___EXPORT_SYMBOL(sym, sec) > -#define __cond_export_sym_0(sym, sec) /* nothing */ > - > -#define __EXPORT_SYMBOL_NS(sym, sec, ns) \ > - __ksym_marker(sym); \ > - __cond_export_ns_sym(sym, sec, ns, __is_defined(__KSYM_##sym)) > -#define __cond_export_ns_sym(sym, sec, ns, conf) \ > - ___cond_export_ns_sym(sym, sec, ns, conf) > -#define ___cond_export_ns_sym(sym, sec, ns, enabled) \ > - __cond_export_ns_sym_##enabled(sym, sec, ns) > -#define __cond_export_ns_sym_1(sym, sec, ns) ___EXPORT_SYMBOL_NS(sym, sec, ns) > -#define __cond_export_ns_sym_0(sym, sec, ns) /* nothing */ > + __cond_export_sym(sym, sec, ns, __is_defined(__KSYM_##sym)) > +#define __cond_export_sym(sym, sec, ns, conf) \ > + ___cond_export_sym(sym, sec, ns, conf) > +#define ___cond_export_sym(sym, sec, ns, enabled) \ > + __cond_export_sym_##enabled(sym, sec, ns) > +#define __cond_export_sym_1(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns) > +#define __cond_export_sym_0(sym, sec, ns) /* nothing */ > > #else > > -#define __EXPORT_SYMBOL_NS(sym,sec,ns) ___EXPORT_SYMBOL_NS(sym,sec,ns) > -#define __EXPORT_SYMBOL(sym,sec) ___EXPORT_SYMBOL(sym,sec) > +#define __EXPORT_SYMBOL(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns) > > #endif /* CONFIG_MODULES */ > > #ifdef DEFAULT_SYMBOL_NAMESPACE > -#undef __EXPORT_SYMBOL > -#define __EXPORT_SYMBOL(sym, sec) \ > - __EXPORT_SYMBOL_NS(sym, sec, DEFAULT_SYMBOL_NAMESPACE) > +#include <linux/stringify.h> > +#define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, __stringify(DEFAULT_SYMBOL_NAMESPACE)) > +#else > +#define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, "") > #endif > > -#define EXPORT_SYMBOL(sym) __EXPORT_SYMBOL(sym, "") > -#define EXPORT_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_gpl") > -#define EXPORT_SYMBOL_GPL_FUTURE(sym) __EXPORT_SYMBOL(sym, "_gpl_future") > -#define EXPORT_SYMBOL_NS(sym, ns) __EXPORT_SYMBOL_NS(sym, "", ns) > -#define EXPORT_SYMBOL_NS_GPL(sym, ns) __EXPORT_SYMBOL_NS(sym, "_gpl", ns) > +#define EXPORT_SYMBOL(sym) _EXPORT_SYMBOL(sym, "") > +#define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "_gpl") > +#define EXPORT_SYMBOL_GPL_FUTURE(sym) _EXPORT_SYMBOL(sym, "_gpl_future") > +#define EXPORT_SYMBOL_NS(sym, ns) __EXPORT_SYMBOL(sym, "", #ns) > +#define EXPORT_SYMBOL_NS_GPL(sym, ns) __EXPORT_SYMBOL(sym, "_gpl", #ns) > > #ifdef CONFIG_UNUSED_SYMBOLS > -#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused") > -#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl") > +#define EXPORT_UNUSED_SYMBOL(sym) _EXPORT_SYMBOL(sym, "_unused") > +#define EXPORT_UNUSED_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "_unused_gpl") > #else > #define EXPORT_UNUSED_SYMBOL(sym) > #define EXPORT_UNUSED_SYMBOL_GPL(sym) > diff --git a/kernel/module.c b/kernel/module.c > index 32873bcce738..73f69ff86db5 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -1399,7 +1399,7 @@ static int verify_namespace_is_imported(const struct load_info *info, > char *imported_namespace; > > namespace = kernel_symbol_namespace(sym); > - if (namespace) { > + if (namespace && namespace[0]) { > imported_namespace = get_modinfo(info, "import_ns"); > while (imported_namespace) { > if (strcmp(namespace, imported_namespace) == 0) > -- > 2.17.1 > -- Best Regards Masahiro Yamada
On 27/09/2019 11.36, Masahiro Yamada wrote: > include/linux/export.h has lots of code duplication between > EXPORT_SYMBOL and EXPORT_SYMBOL_NS. > > To improve the maintainability and readability, unify the > implementation. > > When the symbol has no namespace, pass the empty string "" to > the 'ns' parameter. > > The drawback of this change is, it grows the code size. > When the symbol has no namespace, sym->namespace was previously > NULL, but it is now am empty string "". So, it increases 1 byte > for every no namespace EXPORT_SYMBOL. > > A typical kernel configuration has 10K exported symbols, so it > increases 10KB in rough estimation. > > I did not come up with a good idea to refactor it without increasing > the code size. Can't we put the "aMS" flags on the __ksymtab_strings section? That would make the empty strings free, and would also deduplicate the USB_STORAGE string. And while almost per definition we don't have exact duplicates among the names of exported symbols, we might have both a foo and __foo, so that could save even more. I don't know if we have it already, but we'd need each arch to tell us what symbol to use for @ in @progbits (e.g. % for arm). It seems most are fine with @, so maybe a generic version could be #ifndef ARCH_SECTION_TYPE_CHAR #define ARCH_SECTION_TYPE_CHAR "@" #endif and then it would be section("__ksymtab_strings,\"aMS\","ARCH_SECTION_TYPE_CHAR"progbits,1") But I don't know if any tooling relies on the strings not being deduplicated. Rasmus
On Fri, Sep 27, 2019 at 01:07:33PM +0200, Rasmus Villemoes wrote: >On 27/09/2019 11.36, Masahiro Yamada wrote: >> include/linux/export.h has lots of code duplication between >> EXPORT_SYMBOL and EXPORT_SYMBOL_NS. >> >> To improve the maintainability and readability, unify the >> implementation. >> >> When the symbol has no namespace, pass the empty string "" to >> the 'ns' parameter. >> >> The drawback of this change is, it grows the code size. >> When the symbol has no namespace, sym->namespace was previously >> NULL, but it is now am empty string "". So, it increases 1 byte >> for every no namespace EXPORT_SYMBOL. >> >> A typical kernel configuration has 10K exported symbols, so it >> increases 10KB in rough estimation. >> >> I did not come up with a good idea to refactor it without increasing >> the code size. > >Can't we put the "aMS" flags on the __ksymtab_strings section? That >would make the empty strings free, and would also deduplicate the >USB_STORAGE string. And while almost per definition we don't have exact >duplicates among the names of exported symbols, we might have both a foo >and __foo, so that could save even more. I was not aware of this possibility and I was a bit bothered that I was not able to deduplicate the namespace strings in the PREL case. So, at least I tried to avoid having the redundant empty strings in it. If this approach solves the deduplication problem _and_ reduces the complexity of the code, I am very much for it. I will only be able to look into this next week. >I don't know if we have it already, but we'd need each arch to tell us >what symbol to use for @ in @progbits (e.g. % for arm). It seems most >are fine with @, so maybe a generic version could be > >#ifndef ARCH_SECTION_TYPE_CHAR >#define ARCH_SECTION_TYPE_CHAR "@" >#endif > >and then it would be >section("__ksymtab_strings,\"aMS\","ARCH_SECTION_TYPE_CHAR"progbits,1") > >But I don't know if any tooling relies on the strings not being >deduplicated. Matthias Cheers, >Rasmus
+++ Rasmus Villemoes [27/09/19 13:07 +0200]: >On 27/09/2019 11.36, Masahiro Yamada wrote: >> include/linux/export.h has lots of code duplication between >> EXPORT_SYMBOL and EXPORT_SYMBOL_NS. >> >> To improve the maintainability and readability, unify the >> implementation. >> >> When the symbol has no namespace, pass the empty string "" to >> the 'ns' parameter. >> >> The drawback of this change is, it grows the code size. >> When the symbol has no namespace, sym->namespace was previously >> NULL, but it is now am empty string "". So, it increases 1 byte >> for every no namespace EXPORT_SYMBOL. >> >> A typical kernel configuration has 10K exported symbols, so it >> increases 10KB in rough estimation. >> >> I did not come up with a good idea to refactor it without increasing >> the code size. > >Can't we put the "aMS" flags on the __ksymtab_strings section? That >would make the empty strings free, and would also deduplicate the >USB_STORAGE string. And while almost per definition we don't have exact >duplicates among the names of exported symbols, we might have both a foo >and __foo, so that could save even more. > >I don't know if we have it already, but we'd need each arch to tell us >what symbol to use for @ in @progbits (e.g. % for arm). It seems most >are fine with @, so maybe a generic version could be > >#ifndef ARCH_SECTION_TYPE_CHAR >#define ARCH_SECTION_TYPE_CHAR "@" >#endif > >and then it would be >section("__ksymtab_strings,\"aMS\","ARCH_SECTION_TYPE_CHAR"progbits,1") FWIW, I've just tinkered with this, and unfortunately the strings don't get deduplicated for kernel modules :-( Apparently ld does not do the deduplication for SHF_MERGE|SHF_STRINGS sections for relocatable files (ld -r), which kernel modules are. See: https://sourceware.org/ml/binutils/2009-07/msg00291.html But, the strings do get deduplicated for vmlinux. Not sure if we can find a workaround for modules or if the benefit is significant enough if it only for vmlinux. Thanks, Jessica
On 29/10/2019 20.19, Jessica Yu wrote: > +++ Rasmus Villemoes [27/09/19 13:07 +0200]: >> On 27/09/2019 11.36, Masahiro Yamada wrote: >>> >>> A typical kernel configuration has 10K exported symbols, so it >>> increases 10KB in rough estimation. >>> >>> I did not come up with a good idea to refactor it without increasing >>> the code size. >> >> Can't we put the "aMS" flags on the __ksymtab_strings section? That >> would make the empty strings free, and would also deduplicate the >> USB_STORAGE string. And while almost per definition we don't have exact >> duplicates among the names of exported symbols, we might have both a foo >> and __foo, so that could save even more. >> >> I don't know if we have it already, but we'd need each arch to tell us >> what symbol to use for @ in @progbits (e.g. % for arm). It seems most >> are fine with @, so maybe a generic version could be >> >> #ifndef ARCH_SECTION_TYPE_CHAR >> #define ARCH_SECTION_TYPE_CHAR "@" >> #endif >> >> and then it would be >> section("__ksymtab_strings,\"aMS\","ARCH_SECTION_TYPE_CHAR"progbits,1") > > FWIW, I've just tinkered with this, and unfortunately the strings > don't get deduplicated for kernel modules :-( > > Apparently ld does not do the deduplication for SHF_MERGE|SHF_STRINGS > sections for relocatable files (ld -r), which kernel modules are. See: > > https://sourceware.org/ml/binutils/2009-07/msg00291.html I know <https://patches-gcc.linaro.org/patch/5858/> :) > But, the strings do get deduplicated for vmlinux. Not sure if we can > find a workaround for modules or if the benefit is significant enough > if it only for vmlinux. I think it's definitely worth if, even if it "only" benefits vmlinux for now. And I still hope to revisit the --force-section-merge some day, but it's very far down my priority list. Rasmus
+++ Rasmus Villemoes [29/10/19 22:11 +0100]: >On 29/10/2019 20.19, Jessica Yu wrote: >> +++ Rasmus Villemoes [27/09/19 13:07 +0200]: >>> On 27/09/2019 11.36, Masahiro Yamada wrote: >>>> >>>> A typical kernel configuration has 10K exported symbols, so it >>>> increases 10KB in rough estimation. >>>> >>>> I did not come up with a good idea to refactor it without increasing >>>> the code size. >>> >>> Can't we put the "aMS" flags on the __ksymtab_strings section? That >>> would make the empty strings free, and would also deduplicate the >>> USB_STORAGE string. And while almost per definition we don't have exact >>> duplicates among the names of exported symbols, we might have both a foo >>> and __foo, so that could save even more. >>> >>> I don't know if we have it already, but we'd need each arch to tell us >>> what symbol to use for @ in @progbits (e.g. % for arm). It seems most >>> are fine with @, so maybe a generic version could be >>> >>> #ifndef ARCH_SECTION_TYPE_CHAR >>> #define ARCH_SECTION_TYPE_CHAR "@" >>> #endif >>> >>> and then it would be >>> section("__ksymtab_strings,\"aMS\","ARCH_SECTION_TYPE_CHAR"progbits,1") >> >> FWIW, I've just tinkered with this, and unfortunately the strings >> don't get deduplicated for kernel modules :-( >> >> Apparently ld does not do the deduplication for SHF_MERGE|SHF_STRINGS >> sections for relocatable files (ld -r), which kernel modules are. See: >> >> https://sourceware.org/ml/binutils/2009-07/msg00291.html > >I know <https://patches-gcc.linaro.org/patch/5858/> :) That is exactly what we need! :) >> But, the strings do get deduplicated for vmlinux. Not sure if we can >> find a workaround for modules or if the benefit is significant enough >> if it only for vmlinux. > >I think it's definitely worth if, even if it "only" benefits vmlinux for >now. And I still hope to revisit the --force-section-merge some day, but >it's very far down my priority list. Yeah, I think it's worth having too. If you don't have any extra cycles at the moment, and it's far down your priority list, do you mind if I take a look and maybe try to push that patch of yours upstream again? I don't know how successful I'd be, but now since it's especially relevant for namespaces, it's definitely worth looking at again. Thanks! Jessica
On 31/10/2019 11.13, Jessica Yu wrote: > +++ Rasmus Villemoes [29/10/19 22:11 +0100]: >> On 29/10/2019 20.19, Jessica Yu wrote: >>> Apparently ld does not do the deduplication for SHF_MERGE|SHF_STRINGS >>> sections for relocatable files (ld -r), which kernel modules are. See: >>> >>> https://sourceware.org/ml/binutils/2009-07/msg00291.html >> >> I know <https://patches-gcc.linaro.org/patch/5858/> :) > > That is exactly what we need! :) > >>> But, the strings do get deduplicated for vmlinux. Not sure if we can >>> find a workaround for modules or if the benefit is significant enough >>> if it only for vmlinux. >> >> I think it's definitely worth if, even if it "only" benefits vmlinux for >> now. And I still hope to revisit the --force-section-merge some day, but >> it's very far down my priority list. > > Yeah, I think it's worth having too. > > If you don't have any extra cycles at the moment, and it's far down > your priority list, do you mind if I take a look and maybe try to push > that patch of yours upstream again? Knock yourself out :) IIRC, it did actually work for the powerpc I was targeting, but I don't remember if that was just "readelf/objdump inspection of the ELF files looks reasonable" or if I actually tried loading the modules. I've pushed the patch to https://github.com/Villemoes/binutils-gdb/commit/107b9302858fc5fc1a1690f4a36e1f80808ab421 so you don't have to copy-paste from a browser. I don't know how successful I'd > be, but now since it's especially relevant for namespaces, it's > definitely worth looking at again. Yeah, but even ignoring namespaces, it would be nice to have format strings etc. deduplicated. Please keep me cc'ed on any progress you make. Thanks, Rasmus
+++ Rasmus Villemoes [31/10/19 12:03 +0100]: >On 31/10/2019 11.13, Jessica Yu wrote: >> +++ Rasmus Villemoes [29/10/19 22:11 +0100]: >>> On 29/10/2019 20.19, Jessica Yu wrote: > >>>> Apparently ld does not do the deduplication for SHF_MERGE|SHF_STRINGS >>>> sections for relocatable files (ld -r), which kernel modules are. See: >>>> >>>> https://sourceware.org/ml/binutils/2009-07/msg00291.html >>> >>> I know <https://patches-gcc.linaro.org/patch/5858/> :) >> >> That is exactly what we need! :) >> >>>> But, the strings do get deduplicated for vmlinux. Not sure if we can >>>> find a workaround for modules or if the benefit is significant enough >>>> if it only for vmlinux. >>> >>> I think it's definitely worth if, even if it "only" benefits vmlinux for >>> now. And I still hope to revisit the --force-section-merge some day, but >>> it's very far down my priority list. >> >> Yeah, I think it's worth having too. >> >> If you don't have any extra cycles at the moment, and it's far down >> your priority list, do you mind if I take a look and maybe try to push >> that patch of yours upstream again? > >Knock yourself out :) IIRC, it did actually work for the powerpc I was >targeting, but I don't remember if that was just "readelf/objdump >inspection of the ELF files looks reasonable" or if I actually tried >loading the modules. I've pushed the patch to >https://github.com/Villemoes/binutils-gdb/commit/107b9302858fc5fc1a1690f4a36e1f80808ab421 >so you don't have to copy-paste from a browser. Thanks a bunch! >I don't know how successful I'd >> be, but now since it's especially relevant for namespaces, it's >> definitely worth looking at again. > >Yeah, but even ignoring namespaces, it would be nice to have format >strings etc. deduplicated. Please keep me cc'ed on any progress you make. Thanks, I'll keep you posted if I manage to get somewhere with it :)
diff --git a/include/linux/export.h b/include/linux/export.h index 621158ecd2e2..55245a405a2f 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -48,45 +48,28 @@ extern struct module __this_module; * absolute relocations that require runtime processing on relocatable * kernels. */ -#define __KSYMTAB_ENTRY_NS(sym, sec, ns) \ +#define __KSYMTAB_ENTRY(sym, sec, ns) \ __ADDRESSABLE(sym) \ asm(" .section \"___ksymtab" sec "+" #sym "\", \"a\" \n" \ " .balign 4 \n" \ - "__ksymtab_" #ns NS_SEPARATOR #sym ": \n" \ + "__ksymtab_" ns NS_SEPARATOR #sym ": \n" \ " .long " #sym "- . \n" \ " .long __kstrtab_" #sym "- . \n" \ " .long __kstrtabns_" #sym "- . \n" \ " .previous \n") -#define __KSYMTAB_ENTRY(sym, sec) \ - __ADDRESSABLE(sym) \ - asm(" .section \"___ksymtab" sec "+" #sym "\", \"a\" \n" \ - " .balign 4 \n" \ - "__ksymtab_" #sym ": \n" \ - " .long " #sym "- . \n" \ - " .long __kstrtab_" #sym "- . \n" \ - " .long 0 \n" \ - " .previous \n") - struct kernel_symbol { int value_offset; int name_offset; int namespace_offset; }; #else -#define __KSYMTAB_ENTRY_NS(sym, sec, ns) \ - static const struct kernel_symbol __ksymtab_##sym##__##ns \ - asm("__ksymtab_" #ns NS_SEPARATOR #sym) \ - __attribute__((section("___ksymtab" sec "+" #sym), used)) \ - __aligned(sizeof(void *)) \ - = { (unsigned long)&sym, __kstrtab_##sym, __kstrtabns_##sym } - -#define __KSYMTAB_ENTRY(sym, sec) \ +#define __KSYMTAB_ENTRY(sym, sec, ns) \ static const struct kernel_symbol __ksymtab_##sym \ - asm("__ksymtab_" #sym) \ + asm("__ksymtab_" ns NS_SEPARATOR #sym) \ __attribute__((section("___ksymtab" sec "+" #sym), used)) \ __aligned(sizeof(void *)) \ - = { (unsigned long)&sym, __kstrtab_##sym, NULL } + = { (unsigned long)&sym, __kstrtab_##sym, __kstrtabns_##sym } struct kernel_symbol { unsigned long value; @@ -97,29 +80,21 @@ struct kernel_symbol { #ifdef __GENKSYMS__ -#define ___EXPORT_SYMBOL(sym,sec) __GENKSYMS_EXPORT_SYMBOL(sym) -#define ___EXPORT_SYMBOL_NS(sym,sec,ns) __GENKSYMS_EXPORT_SYMBOL(sym) +#define ___EXPORT_SYMBOL(sym, sec, ns) __GENKSYMS_EXPORT_SYMBOL(sym) #else -#define ___export_symbol_common(sym, sec) \ +/* For every exported symbol, place a struct in the __ksymtab section */ +#define ___EXPORT_SYMBOL(sym, sec, ns) \ extern typeof(sym) sym; \ __CRC_SYMBOL(sym, sec); \ static const char __kstrtab_##sym[] \ __attribute__((section("__ksymtab_strings"), used, aligned(1))) \ - = #sym \ - -/* For every exported symbol, place a struct in the __ksymtab section */ -#define ___EXPORT_SYMBOL_NS(sym, sec, ns) \ - ___export_symbol_common(sym, sec); \ + = #sym; \ static const char __kstrtabns_##sym[] \ __attribute__((section("__ksymtab_strings"), used, aligned(1))) \ - = #ns; \ - __KSYMTAB_ENTRY_NS(sym, sec, ns) - -#define ___EXPORT_SYMBOL(sym, sec) \ - ___export_symbol_common(sym, sec); \ - __KSYMTAB_ENTRY(sym, sec) + = ns; \ + __KSYMTAB_ENTRY(sym, sec, ns) #endif @@ -130,8 +105,7 @@ struct kernel_symbol { * be reused in other execution contexts such as the UEFI stub or the * decompressor. */ -#define __EXPORT_SYMBOL_NS(sym, sec, ns) -#define __EXPORT_SYMBOL(sym, sec) +#define __EXPORT_SYMBOL(sym, sec, ns) #elif defined(CONFIG_TRIM_UNUSED_KSYMS) @@ -147,48 +121,38 @@ struct kernel_symbol { #define __ksym_marker(sym) \ static int __ksym_marker_##sym[0] __section(".discard.ksym") __used -#define __EXPORT_SYMBOL(sym, sec) \ +#define __EXPORT_SYMBOL(sym, sec, ns) \ __ksym_marker(sym); \ - __cond_export_sym(sym, sec, __is_defined(__KSYM_##sym)) -#define __cond_export_sym(sym, sec, conf) \ - ___cond_export_sym(sym, sec, conf) -#define ___cond_export_sym(sym, sec, enabled) \ - __cond_export_sym_##enabled(sym, sec) -#define __cond_export_sym_1(sym, sec) ___EXPORT_SYMBOL(sym, sec) -#define __cond_export_sym_0(sym, sec) /* nothing */ - -#define __EXPORT_SYMBOL_NS(sym, sec, ns) \ - __ksym_marker(sym); \ - __cond_export_ns_sym(sym, sec, ns, __is_defined(__KSYM_##sym)) -#define __cond_export_ns_sym(sym, sec, ns, conf) \ - ___cond_export_ns_sym(sym, sec, ns, conf) -#define ___cond_export_ns_sym(sym, sec, ns, enabled) \ - __cond_export_ns_sym_##enabled(sym, sec, ns) -#define __cond_export_ns_sym_1(sym, sec, ns) ___EXPORT_SYMBOL_NS(sym, sec, ns) -#define __cond_export_ns_sym_0(sym, sec, ns) /* nothing */ + __cond_export_sym(sym, sec, ns, __is_defined(__KSYM_##sym)) +#define __cond_export_sym(sym, sec, ns, conf) \ + ___cond_export_sym(sym, sec, ns, conf) +#define ___cond_export_sym(sym, sec, ns, enabled) \ + __cond_export_sym_##enabled(sym, sec, ns) +#define __cond_export_sym_1(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns) +#define __cond_export_sym_0(sym, sec, ns) /* nothing */ #else -#define __EXPORT_SYMBOL_NS(sym,sec,ns) ___EXPORT_SYMBOL_NS(sym,sec,ns) -#define __EXPORT_SYMBOL(sym,sec) ___EXPORT_SYMBOL(sym,sec) +#define __EXPORT_SYMBOL(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns) #endif /* CONFIG_MODULES */ #ifdef DEFAULT_SYMBOL_NAMESPACE -#undef __EXPORT_SYMBOL -#define __EXPORT_SYMBOL(sym, sec) \ - __EXPORT_SYMBOL_NS(sym, sec, DEFAULT_SYMBOL_NAMESPACE) +#include <linux/stringify.h> +#define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, __stringify(DEFAULT_SYMBOL_NAMESPACE)) +#else +#define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, "") #endif -#define EXPORT_SYMBOL(sym) __EXPORT_SYMBOL(sym, "") -#define EXPORT_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_gpl") -#define EXPORT_SYMBOL_GPL_FUTURE(sym) __EXPORT_SYMBOL(sym, "_gpl_future") -#define EXPORT_SYMBOL_NS(sym, ns) __EXPORT_SYMBOL_NS(sym, "", ns) -#define EXPORT_SYMBOL_NS_GPL(sym, ns) __EXPORT_SYMBOL_NS(sym, "_gpl", ns) +#define EXPORT_SYMBOL(sym) _EXPORT_SYMBOL(sym, "") +#define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "_gpl") +#define EXPORT_SYMBOL_GPL_FUTURE(sym) _EXPORT_SYMBOL(sym, "_gpl_future") +#define EXPORT_SYMBOL_NS(sym, ns) __EXPORT_SYMBOL(sym, "", #ns) +#define EXPORT_SYMBOL_NS_GPL(sym, ns) __EXPORT_SYMBOL(sym, "_gpl", #ns) #ifdef CONFIG_UNUSED_SYMBOLS -#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused") -#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl") +#define EXPORT_UNUSED_SYMBOL(sym) _EXPORT_SYMBOL(sym, "_unused") +#define EXPORT_UNUSED_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "_unused_gpl") #else #define EXPORT_UNUSED_SYMBOL(sym) #define EXPORT_UNUSED_SYMBOL_GPL(sym) diff --git a/kernel/module.c b/kernel/module.c index 32873bcce738..73f69ff86db5 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1399,7 +1399,7 @@ static int verify_namespace_is_imported(const struct load_info *info, char *imported_namespace; namespace = kernel_symbol_namespace(sym); - if (namespace) { + if (namespace && namespace[0]) { imported_namespace = get_modinfo(info, "import_ns"); while (imported_namespace) { if (strcmp(namespace, imported_namespace) == 0)
include/linux/export.h has lots of code duplication between EXPORT_SYMBOL and EXPORT_SYMBOL_NS. To improve the maintainability and readability, unify the implementation. When the symbol has no namespace, pass the empty string "" to the 'ns' parameter. The drawback of this change is, it grows the code size. When the symbol has no namespace, sym->namespace was previously NULL, but it is now am empty string "". So, it increases 1 byte for every no namespace EXPORT_SYMBOL. A typical kernel configuration has 10K exported symbols, so it increases 10KB in rough estimation. I did not come up with a good idea to refactor it without increasing the code size. I am not sure how big a deal it is, but at least include/linux/export.h looks nicer. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- include/linux/export.h | 100 +++++++++++++---------------------------- kernel/module.c | 2 +- 2 files changed, 33 insertions(+), 69 deletions(-) -- 2.17.1