Re: [PATCH 1/2] reduce symbol table for loaded modules

[Rusty Russell <rusty@rustcorp.com.au>]

On Tue, 30 Jun 2009 09:38:43 pm Jan Beulich wrote:
> Discard all symbols not interesting for kallsyms use: absolute,
> section, and in the common case (!KALLSYMS_ALL) data ones.

I like the idea, but implementation could probably be polished a little bit.

> +static unsigned int filter_symbols(Elf_Sym *dst,
> +				   const Elf_Sym *src, unsigned int nsrc,
> +				   const Elf_Shdr *sechdrs, unsigned int shnum)

How about splitting the test out to an:

	bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs, unsigned int shnum)

Then split filter_symbols into num_core_symbols() and copy_core_symbols().

> +static unsigned long layout_symtab(struct module *mod,

How about append_symbols() instead?

> +				   Elf_Shdr *sechdrs,
> +				   unsigned int symindex,
> +				   const Elf_Ehdr *hdr,
> +				   const char *secstrings)
> +{
> +	unsigned long symoffs;
> +	Elf_Shdr *symsect = sechdrs + symindex;
> +

Comment would help me here:

/* Put symbol section at end of init part of module. */
> +	symsect->sh_flags |= SHF_ALLOC;
> +	symsect->sh_entsize = get_offset(mod, &mod->init_size, symsect,
> +					 symindex) | INIT_OFFSET_MASK;
> +	DEBUGP("\t%s\n", secstrings + symsect->sh_name);
> +

Here too:
/* Append room for core symbols at end of core part. */
> +	symoffs = ALIGN(mod->core_size, symsect->sh_addralign ?: 1);
> +	mod->core_size = symoffs
> +			 + filter_symbols(NULL,
> +			 		  (void *)hdr + symsect->sh_offset,
> +					  symsect->sh_size / sizeof(Elf_Sym),
> +					  sechdrs, hdr->e_shnum)
> +			 * sizeof(Elf_Sym);
> +
> +	return symoffs;
> +}
> +
>  static void add_kallsyms(struct module *mod,
>  			 Elf_Shdr *sechdrs,
> +			 unsigned int shnum,
>  			 unsigned int symindex,
>  			 unsigned int strindex,
> +			 unsigned long symoffs,
>  			 const char *secstrings)
>  {
>  	unsigned int i;
> @@ -1857,12 +1914,26 @@ static void add_kallsyms(struct module *
>  	for (i = 0; i < mod->num_symtab; i++)
>  		mod->symtab[i].st_info
>  			= elf_type(&mod->symtab[i], sechdrs, secstrings, mod);
> +
> +	mod->core_symtab = mod->module_core + symoffs;
> +	mod->core_num_syms = filter_symbols(mod->core_symtab,
> +					    mod->symtab, mod->num_symtab,
> +					    sechdrs, shnum);

Hmm, could avoid symoffs if copy_core_symbols worked backwards.  Probably
not worthwhile tho.

>  	trim_init_extable(mod);
> +#ifdef CONFIG_KALLSYMS
> +	mod->num_symtab = mod->core_num_syms;
> +	mod->symtab = mod->core_symtab;
> +#endif
>  	module_free(mod, mod->module_init);
>  	mod->module_init = NULL;
>  	mod->init_size = 0;

core_num_syms/core_symtab is really a temporary; we should note that in
the module.h header next to those fields.

Thanks!
Rusty.