On Fri 2022-09-09 21:00:15, Zhen Lei wrote:
> Currently we traverse all symbols of all modules to find the specified
> function for the specified module. But in reality, we just need to find
> the given module and then traverse all the symbols in it.
I agree that it might be noticeable speedup.
> In order to achieve this purpose, split the call to hook 'fn' into two
> phases:
> 1. Finds the given module. Pass pointer 'mod'. Hook 'fn' directly returns
> the comparison result of the module name without comparing the function
> name.
> 2. Finds the given function in that module. Pass pointer 'mod = NULL'.
> Hook 'fn' skip the comparison of module name and directly compare
> function names.
>
> Phase1: mod1-->mod2..(subsequent modules do not need to be compared)
> |
> Phase2: -->f1-->f2-->f3
>
> Signed-off-by: Zhen Lei <thunder.leizhen@xxxxxxxxxx>
> ---
> kernel/livepatch/core.c | 7 ++-----
> kernel/module/kallsyms.c | 13 ++++++++++++-
> 2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 31b57ccf908017e..98e23137e4133bc 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -130,15 +130,12 @@ static int klp_find_callback(void *data, const char *name,
> {
> struct klp_find_arg *args = data;
>
> - if ((mod && !args->objname) || (!mod && args->objname))
> - return 0;
> + if (mod)
> + return strcmp(args->objname, mod->name);
>
> if (strcmp(args->name, name))
> return 0;
>
> - if (args->objname && strcmp(args->objname, mod->name))
> - return 0;
> -
> args->addr = addr;
> args->count++;
>
> diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
> index f5c5c9175333df7..b033613e6c7e3bb 100644
> --- a/kernel/module/kallsyms.c
> +++ b/kernel/module/kallsyms.c
> @@ -510,6 +510,11 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> if (mod->state == MODULE_STATE_UNFORMED)
> continue;
>
> + /* check mod->name first */
> + ret = fn(data, NULL, mod, 0);
> + if (ret)
> + continue;
Hmm, it somehow gets too complicated. The same fn() callback has to
behave correctly in 3 different situations. I would suggest to
simplify everything:
1. Pass the requested modname as a parameter to module_kallsyms_on_each_symbol()
/*
* Iterate over all symbols in the given @modname. For symbols from
* vmlinux use kallsyms_on_each_symbol() instead.
*/
int module_kallsyms_on_each_symbol(const char *modname,
int (*fn)(void *, const char *,
struct module *, unsigned long),
void *data)
and do here:
if (strcmp(modname, mod->name))
continue;
2. We do not even need to pass .objname in struct klp_find_arg
could simplify the callback:
static int klp_find_callback(void *data, const char *name,
struct module *mod, unsigned long addr)
{
struct klp_find_arg *args = data;
if (strcmp(args->name, name))
return 0;
args->addr = addr;
args->count++;
/*
* Finish the search when the symbol is found for the desired position
* or the position is not defined for a non-unique symbol.
*/
if ((args->pos && (args->count == args->pos)) ||
(!args->pos && (args->count > 1)))
return 1;
return 0;
}
3. As a result the *mod parameter won't be used by any existing
fn() callback and could be removed. This should be done as
a separate patch. It touches also ftrace_lookup_symbols().
Best Regards,
Petr
