module: Fix performance regression on modules with large symbol tables

Commit 554bdfe5acf3715e87c8d5e25a4f9a896ac9f014 (module: reduce string
table for loaded modules) introduced an optimization to shrink the size of
the resident string table.  Part of this involves calling bitmap_weight()
on the strmap bitmap once for each core symbol.  strmap contains one bit
for each byte of the module's strtab.

For kernel modules with a large number of symbols, the addition of the
bitmap_weight() operation to each iteration of the add_kallsyms() loop
resulted in a significant "insmod" performance regression from 2.6.31
to 2.6.32.  bitmap_weight() is expensive when the bitmap is large.

The proposed alternative optimizes the common case in this loop
(is_core_symbol() == true, and the symbol name is not a duplicate), while
penalizing the exceptional case of a duplicate symbol.

My test was run on a 600 MHz MIPS processor, using a kernel module with
15,000 "core" symbols and 10,000 symbols in .init.text.  .strtab takes up
250,227 bytes.

Original code: insmod takes 3.39 seconds
Patched code: insmod takes 0.07 seconds

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/kernel/module.c b/kernel/module.c
index cf9f1b6b3268e0315b8c63f3c506e7e79c3c6662..6c87305fd34168325b52ae1b00632d6b0dac257f 100644 (file)
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2246,22 +2246,48 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
                mod->symtab[i].st_info = elf_type(&mod->symtab[i], info);
 
        mod->core_symtab = dst = mod->module_core + info->symoffs;
+       mod->core_strtab = s = mod->module_core + info->stroffs;
        src = mod->symtab;
        *dst = *src;
+       *s++ = 0;
        for (ndst = i = 1; i < mod->num_symtab; ++i, ++src) {
+               const char *name;
                if (!is_core_symbol(src, info->sechdrs, info->hdr->e_shnum))
                        continue;
                dst[ndst] = *src;
-               dst[ndst].st_name = bitmap_weight(info->strmap,
-                                                 dst[ndst].st_name);
+
+               name = &mod->strtab[src->st_name];
+               if (unlikely(!test_bit(src->st_name, info->strmap))) {
+                       /* Symbol name has already been copied; find it. */
+                       char *dup;
+
+                       for (dup = mod->core_strtab; strcmp(dup, name); dup++)
+                               BUG_ON(dup > s);
+
+                       dst[ndst].st_name = dup - mod->core_strtab;
+               } else {
+                       /*
+                        * Copy the symbol name to mod->core_strtab.
+                        * "name" might point to the middle of a longer strtab
+                        * entry, so backtrack to the first "required" byte
+                        * of the string.
+                        */
+                       unsigned start = src->st_name, len = 0;
+
+                       for (; test_bit(start - 1, info->strmap) &&
+                              mod->strtab[start - 1]; start--)
+                               len++;
+
+                       dst[ndst].st_name = len + s - mod->core_strtab;
+                       len += strlen(name) + 1;
+
+                       memcpy(s, &mod->strtab[start], len);
+                       s += len;
+                       bitmap_clear(info->strmap, start, len);
+               }
                ++ndst;
        }
        mod->core_num_syms = ndst;
-
-       mod->core_strtab = s = mod->module_core + info->stroffs;
-       for (*s = 0, i = 1; i < info->sechdrs[info->index.str].sh_size; ++i)
-               if (test_bit(i, info->strmap))
-                       *++s = mod->strtab[i];
 }
 #else
 static inline void layout_symtab(struct module *mod, struct load_info *info)