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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tao,

On Tue, 14 Sep 2021 17:01:51 +0800
Tao Liu <ltao@xxxxxxxxxx> wrote:

> 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@xxxxxxxxxx>
> ---
>  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;

with big parts of the function removed some variables are no longer
used. Please remove their declaration.

> -	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;

same like above.

Thanks
Philipp


> -	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;

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux