Re: [PATCH v4 5/8] kallsyms: Add helper kallsyms_on_each_match_symbol()

[Petr Mladek]

On Tue 2022-09-20 15:13:14, Zhen Lei wrote:
> Function kallsyms_on_each_symbol() traverses all symbols and submits each
> symbol to the hook 'fn' for judgment and processing. For some cases, the
> hook actually only handles the matched symbol, such as livepatch.
> 
> So that, we can first compress the name being looked up and then use
> it for comparison when traversing 'kallsyms_names', this greatly reduces
> the time consumed by traversing.
> 
> The pseudo code of the test case is as follows:
> static int tst_find(void *data, const char *name,
> 		    struct module *mod, unsigned long addr)
> {
> 	if (strcmp(name, "vmap") == 0)
> 		*(unsigned long *)data = addr;
>         return 0;
> }
> 
> static int tst_match(void *data, unsigned long addr)
> {
>         *(unsigned long *)data = addr;
>         return 0;
> }
> 
> start = sched_clock();
> kallsyms_on_each_match_symbol(tst_match, "vmap", &addr);
> end = sched_clock();
> 
> start = sched_clock();
> kallsyms_on_each_symbol(tst_find, &addr);
> end = sched_clock();
> 
> The test results are as follows (twice):
> kallsyms_on_each_match_symbol:   557400,   583900
> kallsyms_on_each_symbol      : 16659500, 16113950
> 
> kallsyms_on_each_match_symbol() consumes only 3.48% of
> kallsyms_on_each_symbol()'s time.
> 
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -305,6 +305,31 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
>  	return 0;
>  }
>  
> +int kallsyms_on_each_match_symbol(int (*fn)(void *, unsigned long),
> +				  const char *name, void *data)
> +{
> +	unsigned int i, off;
> +	int len, ret;
> +	char namebuf[KSYM_NAME_LEN];
> +
> +	len = kallsyms_name_to_tokens(name, namebuf);
> +	for (i = 0, off = 0; len && i < kallsyms_num_syms; i++) {
> +		if ((i & 0xfff) == 0)
> +			cond_resched();
> +
> +		if ((kallsyms_names[off] == len + 1) &&
> +		    !memcmp(&kallsyms_names[off + 2], namebuf, len)) {
> +			ret = fn(data, kallsyms_sym_address(i));
> +			if (ret != 0)
> +				return ret;
> +			cond_resched();
> +		}
> +		off += kallsyms_names[off] + 1;

Similar tricky code is used also in kallsyms_lookup_name(). Please,
avoid code duplication and put this into a helper funtion.

Best Regards,
Petr

> +	}
> +
> +	return 0;
> +}
> +
>  static unsigned long get_symbol_pos(unsigned long addr,
>  				    unsigned long *symbolsize,
>  				    unsigned long *offset)
> -- 
> 2.25.1