Re: [Crash-utility] [PATCH v3 1/4] Improve the performance of symbol searching for kernel modules

Currently the sequence for symbol_search to search a symbol is: 1) kernel symname hash table, 2) iterate all kernel symbols, 3) iterate all kernel modules and their symbols. In the worst case, if a non-exist symbol been searched, all 3 stages will be went through. The time consuming status for each stage is like: stage 1 stage 2 stage 3 0.007000(ms) 0.593000(ms) 2.421000(ms) stage 3 takes too much time when comparing to stage 1. This patch introduces a symname hash table for kernel modules, to improve the performance of symbol searching. Functions symbol_search and symbol_exists are fundamental and widely used by other crash functions, thus the benefit of performance improvement can get accumulated. For example, "ps -m" and "irq" commands, which call the functions many times, will become faster with the patch. Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; --- defs.h | 1 + kernel.c | 1 + symbols.c | 167 +++++++++++++++++++++++++++++------------------------- 3 files changed, 91 insertions(+), 78 deletions(-)

diff --git a/symbols.c b/symbols.c index bf6d94d..44415da 100644 --- a/symbols.c +++ b/symbols.c

@@ -4494,53 +4561,14 @@ symbol_search(char *s) struct load_module *lm; int pseudos, search_init;

- sp_hashed = symname_hash_search(s); + sp_hashed = symname_hash_search(st->symname_hash, s, NULL); for (sp = sp_hashed ? sp_hashed : st->symtable; sp < st->symend; sp++) { if (STREQ(s, sp->name)) return(sp); } - pseudos = (strstr(s, "_MODULE_START_") || strstr(s, "_MODULE_END_")); - search_init = FALSE; - - for (i = 0; i < st->mods_installed; i++) { - lm = &st->load_modules[i]; - if (lm->mod_flags & MOD_INIT) - search_init = TRUE; - sp = lm->mod_symtable; - sp_end = lm->mod_symend; - - for ( ; sp <= sp_end; sp++) { - if (!pseudos && MODULE_PSEUDO_SYMBOL(sp)) - continue; - if (STREQ(s, sp->name)) - return(sp); - } - } - - if (!search_init) - return((struct syment *)NULL); - - pseudos = (strstr(s, "_MODULE_INIT_START_") || strstr(s, "_MODULE_INIT_END_")); - - for (i = 0; i < st->mods_installed; i++) { - lm = &st->load_modules[i]; - if (!lm->mod_init_symtable) - continue; - sp = lm->mod_init_symtable; - sp_end = lm->mod_init_symend; - - for ( ; sp < sp_end; sp++) { - if (!pseudos && MODULE_PSEUDO_SYMBOL(sp)) - continue; - - if (STREQ(s, sp->name)) - return(sp); - } - } - - return((struct syment *)NULL); + return symname_hash_search(st->mod_symname_hash, s, skip_symbols); } /* @@ -5436,29 +5464,11 @@ symbol_exists(char *symbol) struct syment *sp, *sp_end; struct load_module *lm;

- if ((sp = symname_hash_search(symbol))) + if (symname_hash_search(st->symname_hash, symbol, NULL)) return TRUE; - for (i = 0; i < st->mods_installed; i++) { - lm = &st->load_modules[i]; - sp = lm->mod_symtable; - sp_end = lm->mod_symend; - - for ( ; sp < sp_end; sp++) { - if (STREQ(symbol, sp->name)) - return(TRUE); - } - - if (lm->mod_init_symtable) { - sp = lm->mod_init_symtable; - sp_end = lm->mod_init_symend; - - for ( ; sp < sp_end; sp++) { - if (STREQ(symbol, sp->name)) - return(TRUE); - } - } - } + if (symname_hash_search(st->mod_symname_hash, symbol, NULL)) + return TRUE; return(FALSE); } @@ -5513,12 +5523,7 @@ per_cpu_symbol_search(char *symbol) int kernel_symbol_exists(char *symbol) { - struct syment *sp; - - if ((sp = symname_hash_search(symbol))) - return TRUE; - else - return FALSE; + return !!(symname_hash_search(st->symname_hash, symbol, NULL)); } /* @@ -5527,7 +5532,7 @@ kernel_symbol_exists(char *symbol) struct syment * kernel_symbol_search(char *symbol) { - return symname_hash_search(symbol); + return symname_hash_search(st->symname_hash, symbol, NULL); } /* @@ -12464,8 +12469,10 @@ store_load_module_symbols(bfd *bfd, int dynamic, void *minisyms, error(INFO, "%s: last symbol: %s is not _MODULE_END_%s?\n", lm->mod_name, lm->mod_load_symend->name, lm->mod_name); + mod_symtable_hash_remove_range(lm->mod_symtable, lm->mod_symend); lm->mod_symtable = lm->mod_load_symtable; lm->mod_symend = lm->mod_load_symend; + mod_symtable_hash_install_range(lm->mod_symtable, lm->mod_symend); lm->mod_flags &= ~MOD_EXT_SYMS; lm->mod_flags |= MOD_LOAD_SYMS; @@ -12495,6 +12502,7 @@ delete_load_module(ulong base_addr) req->name = lm->mod_namelist; gdb_interface(req); } + mod_symtable_hash_remove_range(lm->mod_symtable, lm->mod_symend); if (lm->mod_load_symtable) { free(lm->mod_load_symtable); namespace_ctl(NAMESPACE_FREE, @@ -12504,6 +12512,7 @@ delete_load_module(ulong base_addr) unlink_module(lm); lm->mod_symtable = lm->mod_ext_symtable; lm->mod_symend = lm->mod_ext_symend; + mod_symtable_hash_install_range(lm->mod_symtable, lm->mod_symend); lm->mod_flags &= ~(MOD_LOAD_SYMS|MOD_REMOTE|MOD_NOPATCH); lm->mod_flags |= MOD_EXT_SYMS; lm->mod_load_symtable = NULL; @@ -12532,6 +12541,7 @@ delete_load_module(ulong base_addr) req->name = lm->mod_namelist; gdb_interface(req); } + mod_symtable_hash_remove_range(lm->mod_symtable, lm->mod_symend); if (lm->mod_load_symtable) { free(lm->mod_load_symtable); namespace_ctl(NAMESPACE_FREE, @@ -12541,6 +12551,7 @@ delete_load_module(ulong base_addr) unlink_module(lm); lm->mod_symtable = lm->mod_ext_symtable; lm->mod_symend = lm->mod_ext_symend; + mod_symtable_hash_install_range(lm->mod_symtable, lm->mod_symend); lm->mod_flags &= ~(MOD_LOAD_SYMS|MOD_REMOTE|MOD_NOPATCH); lm->mod_flags |= MOD_EXT_SYMS; lm->mod_load_symtable = NULL;