Messages in this thread Patch in this message |  | | | From | Denys Vlasenko <> | | Subject | Re: [PATCH] module: reorder struct module to save space on 64 bit builds | | Date | Sat, 21 Jun 2008 19:26:15 +0200 |
| |
On Friday 20 June 2008 16:44, Richard Kennedy wrote: > reorder struct module to save space on 64 bit builds. > saves 1 cacheline_size (128 on default x86_64 & 64 on AMD > Opteron/athlon) when CONFIG_MODULE_UNLOAD=y. > > Signed-off-by: Richard Kennedy <richard@rsk.demon.co.uk> > --- > > Patch against 2.6.26-rc6. tested & running successfully on AMD64 desktop > machine. This patch reduces the data segment of each module by 1 > cacheline size. > > I also compiled with this patch for 32 bit & there was no change in > size.
Sometime ago I did something similar. I also shrank the struct module by ifdefing out fields which are not needed.
The patch appeared to fell through the cracks.
Here is it again with original submission text.
(Note: majoe reason for struct module's disproportionate size is this member:
#ifdef CONFIG_MODULE_UNLOAD /* Reference counts */ struct module_ref ref[NR_CPUS]; because every array member takes entire cacheline (by design):
struct module_ref { local_t count; } ____cacheline_aligned; I guess the solution is to not select CONFIG_MODULE_UNLOAD...
On Friday 14 September 2007 00:30, Denys Vlasenko wrote: > Hi Andrew, > > module.c and module.h conatains code for finding > exported symbols which are declared with EXPORT_UNUSED_SYMBOL, > and this code is compiled in even if CONFIG_UNUSED_SYMBOLS is not set > and thus there can be no EXPORT_UNUSED_SYMBOLs in modules anyway > (because EXPORT_UNUSED_SYMBOL(x) are compiled out to nothing then). > > This patch adds required #ifdefs. > > This shrinks module.o and each *.ko file. > > Patch also regroups some struct module members so > that on 64 bit CPUs we are not wasting 32 bits on padding here: > > const struct kernel_symbol *unused_syms; > unsigned int num_unused_syms; > const unsigned long *unused_crcs; > > It groups counters and pointers separately. > > Patch makes small edit to help text of CONFIG_MODULE_UNLOAD - > it explicitly says that without that option, kernel > will be also faster, not only "smaller and simpler". > When I realized how much churn is going on under the hood > in order to make module unloading possible, I felt that > users are not informed well enough about it in the help text. > > And finally, structure members which hold length of module > code (four such members there) and count of symbols > are converted from longs to ints. > > We cannot possibly have a module where 32 bits won't > be enough to hold such counts. > > For one, module loading checks module size for sanity > before loading, so such insanely big module will fail > that test first. > > In short, patch makes trivial changes which are "obviously correct" > (famous last words). > > Patch is compile tested with various combinations of CONFIGs. > > Please put it into -mm. > > Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com> --- linux-2.6.23-rc6/include/linux/module.h Wed Sep 12 22:35:32 2007 +++ linux-2.6.23-rc6.structmodule/include/linux/module.h Thu Sep 13 10:04:33 2007 @@ -264,28 +264,30 @@ struct kobject *holders_dir; /* Exported symbols */ - const struct kernel_symbol *syms; unsigned int num_syms; - const unsigned long *crcs; - /* GPL-only exported symbols. */ - const struct kernel_symbol *gpl_syms; unsigned int num_gpl_syms; - const unsigned long *gpl_crcs; - + /* symbols that will be GPL-only in the near future. */ + unsigned int num_gpl_future_syms; +#ifdef CONFIG_UNUSED_SYMBOLS /* unused exported symbols. */ - const struct kernel_symbol *unused_syms; unsigned int num_unused_syms; - const unsigned long *unused_crcs; /* GPL-only, unused exported symbols. */ - const struct kernel_symbol *unused_gpl_syms; unsigned int num_unused_gpl_syms; - const unsigned long *unused_gpl_crcs; - - /* symbols that will be GPL-only in the near future. */ +#endif + /* and respective pointers... */ + const struct kernel_symbol *syms; + const unsigned long *crcs; + const struct kernel_symbol *gpl_syms; + const unsigned long *gpl_crcs; const struct kernel_symbol *gpl_future_syms; - unsigned int num_gpl_future_syms; const unsigned long *gpl_future_crcs; +#ifdef CONFIG_UNUSED_SYMBOLS + const struct kernel_symbol *unused_syms; + const unsigned long *unused_crcs; + const struct kernel_symbol *unused_gpl_syms; + const unsigned long *unused_gpl_crcs; +#endif /* Exception table */ unsigned int num_exentries; @@ -301,10 +303,10 @@ void *module_core; /* Here are the sizes of the init and core sections */ - unsigned long init_size, core_size; + unsigned int init_size, core_size; /* The size of the executable code in each section. */ - unsigned long init_text_size, core_text_size; + unsigned int init_text_size, core_text_size; /* The handle returned from unwind_add_table. */ void *unwind_info; @@ -341,7 +343,7 @@ #ifdef CONFIG_KALLSYMS /* We keep the symbol and string tables for kallsyms. */ Elf_Sym *symtab; - unsigned long num_symtab; + unsigned int num_symtab; char *strtab; /* Section attributes */ --- linux-2.6.23-rc6/init/Kconfig Wed Sep 12 22:35:32 2007 +++ linux-2.6.23-rc6.structmodule/init/Kconfig Thu Sep 13 01:32:31 2007 @@ -611,8 +611,8 @@ help Without this option you will not be able to unload any modules (note that some modules may not be unloadable - anyway), which makes your kernel slightly smaller and - simpler. If unsure, say Y. + anyway), which makes your kernel smaller, faster + and simpler. If unsure, say Y. config MODULE_FORCE_UNLOAD bool "Forced module unloading" --- linux-2.6.23-rc6/kernel/module.c Tue Sep 11 22:34:01 2007 +++ linux-2.6.23-rc6.structmodule/kernel/module.c Thu Sep 13 10:14:17 2007 @@ -128,17 +128,19 @@ extern const struct kernel_symbol __stop___ksymtab_gpl[]; extern const struct kernel_symbol __start___ksymtab_gpl_future[]; extern const struct kernel_symbol __stop___ksymtab_gpl_future[]; -extern const struct kernel_symbol __start___ksymtab_unused[]; -extern const struct kernel_symbol __stop___ksymtab_unused[]; -extern const struct kernel_symbol __start___ksymtab_unused_gpl[]; -extern const struct kernel_symbol __stop___ksymtab_unused_gpl[]; extern const struct kernel_symbol __start___ksymtab_gpl_future[]; extern const struct kernel_symbol __stop___ksymtab_gpl_future[]; extern const unsigned long __start___kcrctab[]; extern const unsigned long __start___kcrctab_gpl[]; extern const unsigned long __start___kcrctab_gpl_future[]; +#ifdef CONFIG_UNUSED_SYMBOLS +extern const struct kernel_symbol __start___ksymtab_unused[]; +extern const struct kernel_symbol __stop___ksymtab_unused[]; +extern const struct kernel_symbol __start___ksymtab_unused_gpl[]; +extern const struct kernel_symbol __stop___ksymtab_unused_gpl[]; extern const unsigned long __start___kcrctab_unused[]; extern const unsigned long __start___kcrctab_unused_gpl[]; +#endif #ifndef CONFIG_MODVERSIONS #define symversion(base, idx) NULL @@ -158,6 +160,7 @@ return NULL; } +#ifdef CONFIG_UNUSED_SYMBOLS static void printk_unused_warning(const char *name) { printk(KERN_WARNING "Symbol %s is marked as UNUSED, " @@ -168,6 +171,7 @@ "mailinglist together with submitting your code for " "inclusion.\n"); } +#endif /* Find a symbol, return value, crc and module which owns it */ static unsigned long __find_symbol(const char *name, @@ -211,6 +215,7 @@ return ks->value; } +#ifdef CONFIG_UNUSED_SYMBOLS ks = lookup_symbol(name, __start___ksymtab_unused, __stop___ksymtab_unused); if (ks) { @@ -229,6 +234,7 @@ (ks - __start___ksymtab_unused_gpl)); return ks->value; } +#endif /* Now try modules. */ list_for_each_entry(mod, &modules, list) { @@ -248,13 +254,13 @@ return ks->value; } } +#ifdef CONFIG_UNUSED_SYMBOLS ks = lookup_symbol(name, mod->unused_syms, mod->unused_syms + mod->num_unused_syms); if (ks) { printk_unused_warning(name); *crc = symversion(mod->unused_crcs, (ks - mod->unused_syms)); return ks->value; } - if (gplok) { ks = lookup_symbol(name, mod->unused_gpl_syms, mod->unused_gpl_syms + mod->num_unused_gpl_syms); @@ -265,6 +271,7 @@ return ks->value; } } +#endif ks = lookup_symbol(name, mod->gpl_future_syms, (mod->gpl_future_syms + mod->num_gpl_future_syms)); @@ -1332,7 +1339,7 @@ } /* Update size with this section: return offset. */ -static long get_offset(unsigned long *size, Elf_Shdr *sechdr) +static long get_offset(unsigned int *size, Elf_Shdr *sechdr) { long ret; @@ -1570,10 +1577,12 @@ unsigned int gplfutureindex; unsigned int gplfuturecrcindex; unsigned int unwindex = 0; +#ifdef CONFIG_UNUSED_SYMBOLS unsigned int unusedindex; unsigned int unusedcrcindex; unsigned int unusedgplindex; unsigned int unusedgplcrcindex; +#endif struct module *mod; long err = 0; void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */ @@ -1654,13 +1663,15 @@ exportindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab"); gplindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab_gpl"); gplfutureindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab_gpl_future"); - unusedindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab_unused"); - unusedgplindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab_unused_gpl"); crcindex = find_sec(hdr, sechdrs, secstrings, "__kcrctab"); gplcrcindex = find_sec(hdr, sechdrs, secstrings, "__kcrctab_gpl"); gplfuturecrcindex = find_sec(hdr, sechdrs, secstrings, "__kcrctab_gpl_future"); +#ifdef CONFIG_UNUSED_SYMBOLS + unusedindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab_unused"); + unusedgplindex = find_sec(hdr, sechdrs, secstrings, "__ksymtab_unused_gpl"); unusedcrcindex = find_sec(hdr, sechdrs, secstrings, "__kcrctab_unused"); unusedgplcrcindex = find_sec(hdr, sechdrs, secstrings, "__kcrctab_unused_gpl"); +#endif setupindex = find_sec(hdr, sechdrs, secstrings, "__param"); exindex = find_sec(hdr, sechdrs, secstrings, "__ex_table"); obsparmindex = find_sec(hdr, sechdrs, secstrings, "__obsparm"); @@ -1813,27 +1824,34 @@ mod->gpl_crcs = (void *)sechdrs[gplcrcindex].sh_addr; mod->num_gpl_future_syms = sechdrs[gplfutureindex].sh_size / sizeof(*mod->gpl_future_syms); +#ifdef CONFIG_UNUSED_SYMBOLS mod->num_unused_syms = sechdrs[unusedindex].sh_size / sizeof(*mod->unused_syms); mod->num_unused_gpl_syms = sechdrs[unusedgplindex].sh_size / sizeof(*mod->unused_gpl_syms); +#endif mod->gpl_future_syms = (void *)sechdrs[gplfutureindex].sh_addr; if (gplfuturecrcindex) mod->gpl_future_crcs = (void *)sechdrs[gplfuturecrcindex].sh_addr; +#ifdef CONFIG_UNUSED_SYMBOLS mod->unused_syms = (void *)sechdrs[unusedindex].sh_addr; if (unusedcrcindex) mod->unused_crcs = (void *)sechdrs[unusedcrcindex].sh_addr; mod->unused_gpl_syms = (void *)sechdrs[unusedgplindex].sh_addr; if (unusedgplcrcindex) mod->unused_crcs = (void *)sechdrs[unusedgplcrcindex].sh_addr; +#endif #ifdef CONFIG_MODVERSIONS if ((mod->num_syms && !crcindex) || (mod->num_gpl_syms && !gplcrcindex) || - (mod->num_gpl_future_syms && !gplfuturecrcindex) || - (mod->num_unused_syms && !unusedcrcindex) || - (mod->num_unused_gpl_syms && !unusedgplcrcindex)) { + (mod->num_gpl_future_syms && !gplfuturecrcindex) +#ifdef CONFIG_UNUSED_SYMBOLS + || (mod->num_unused_syms && !unusedcrcindex) + || (mod->num_unused_gpl_syms && !unusedgplcrcindex) +#endif + ) { printk(KERN_WARNING "%s: No versions for exported symbols." " Tainting kernel.\n", mod->name); add_taint_module(mod, TAINT_FORCED_MODULE); @@ -2269,7 +2287,7 @@ struct module *mod = list_entry(p, struct module, list); char buf[8]; - seq_printf(m, "%s %lu", + seq_printf(m, "%s %u", mod->name, mod->init_size + mod->core_size); print_unload_info(m, mod); |  |