| Message ID | 20220923112033.1958-9-thunder.leizhen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kallsyms: Optimizes the performance of lookup symbols | expand |
On 2022/9/23 19:20, 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. > > 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. Sorry, I forgot to change the description. I will fix it in v6, after I've collected review comments. > > Phase1: mod1-->mod2..(subsequent modules do not need to be compared) > | > Phase2: -->f1-->f2-->f3 > > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > --- > include/linux/module.h | 4 ++-- > kernel/livepatch/core.c | 13 ++----------- > kernel/module/kallsyms.c | 15 ++++++++++++--- > 3 files changed, 16 insertions(+), 16 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index 518296ea7f73af6..6e1a531d78e7e8b 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -879,8 +879,8 @@ static inline bool module_sig_ok(struct module *module) > } > #endif /* CONFIG_MODULE_SIG */ > > -int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, > - struct module *, unsigned long), > +int module_kallsyms_on_each_symbol(const char *modname, > + int (*fn)(void *, const char *, unsigned long), > void *data); > > #endif /* _LINUX_MODULE_H */ > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index 31b57ccf908017e..b02de4cb311c703 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -118,27 +118,19 @@ static struct klp_object *klp_find_object(struct klp_patch *patch, > } > > struct klp_find_arg { > - const char *objname; > const char *name; > unsigned long addr; > unsigned long count; > unsigned long pos; > }; > > -static int klp_find_callback(void *data, const char *name, > - struct module *mod, unsigned long addr) > +static int klp_find_callback(void *data, const char *name, unsigned long addr) > { > struct klp_find_arg *args = data; > > - if ((mod && !args->objname) || (!mod && args->objname)) > - return 0; > - > if (strcmp(args->name, name)) > return 0; > > - if (args->objname && strcmp(args->objname, mod->name)) > - return 0; > - > args->addr = addr; > args->count++; > > @@ -175,7 +167,6 @@ static int klp_find_object_symbol(const char *objname, const char *name, > unsigned long sympos, unsigned long *addr) > { > struct klp_find_arg args = { > - .objname = objname, > .name = name, > .addr = 0, > .count = 0, > @@ -183,7 +174,7 @@ static int klp_find_object_symbol(const char *objname, const char *name, > }; > > if (objname) > - module_kallsyms_on_each_symbol(klp_find_callback, &args); > + module_kallsyms_on_each_symbol(objname, klp_find_callback, &args); > else > kallsyms_on_each_match_symbol(klp_match_callback, name, &args); > > diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c > index f5c5c9175333df7..329cef573675d49 100644 > --- a/kernel/module/kallsyms.c > +++ b/kernel/module/kallsyms.c > @@ -495,8 +495,8 @@ unsigned long module_kallsyms_lookup_name(const char *name) > } > > #ifdef CONFIG_LIVEPATCH > -int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, > - struct module *, unsigned long), > +int module_kallsyms_on_each_symbol(const char *modname, > + int (*fn)(void *, const char *, unsigned long), > void *data) > { > struct module *mod; > @@ -510,6 +510,9 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, > if (mod->state == MODULE_STATE_UNFORMED) > continue; > > + if (strcmp(modname, mod->name)) > + continue; > + > /* Use rcu_dereference_sched() to remain compliant with the sparse tool */ > preempt_disable(); > kallsyms = rcu_dereference_sched(mod->kallsyms); > @@ -522,10 +525,16 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, > continue; > > ret = fn(data, kallsyms_symbol_name(kallsyms, i), > - mod, kallsyms_symbol_value(sym)); > + kallsyms_symbol_value(sym)); > if (ret != 0) > goto out; > } > + > + /* > + * The given module is found, the subsequent modules do not > + * need to be compared. > + */ > + break; > } > out: > mutex_unlock(&module_mutex); >
On 2022/9/24 9:11, Leizhen (ThunderTown) wrote: > > On 2022/9/23 19:20, 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. >> >> 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. > Sorry, I forgot to change the description. I will fix it in v6, after I've > collected review comments. Oh, It's Saturday, and I don't think anyone's seen v5 yet. So I'll post the v6. Please skip v5. >
diff --git a/include/linux/module.h b/include/linux/module.h index 518296ea7f73af6..6e1a531d78e7e8b 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -879,8 +879,8 @@ static inline bool module_sig_ok(struct module *module) } #endif /* CONFIG_MODULE_SIG */ -int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, - struct module *, unsigned long), +int module_kallsyms_on_each_symbol(const char *modname, + int (*fn)(void *, const char *, unsigned long), void *data); #endif /* _LINUX_MODULE_H */ diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 31b57ccf908017e..b02de4cb311c703 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -118,27 +118,19 @@ static struct klp_object *klp_find_object(struct klp_patch *patch, } struct klp_find_arg { - const char *objname; const char *name; unsigned long addr; unsigned long count; unsigned long pos; }; -static int klp_find_callback(void *data, const char *name, - struct module *mod, unsigned long addr) +static int klp_find_callback(void *data, const char *name, unsigned long addr) { struct klp_find_arg *args = data; - if ((mod && !args->objname) || (!mod && args->objname)) - return 0; - if (strcmp(args->name, name)) return 0; - if (args->objname && strcmp(args->objname, mod->name)) - return 0; - args->addr = addr; args->count++; @@ -175,7 +167,6 @@ static int klp_find_object_symbol(const char *objname, const char *name, unsigned long sympos, unsigned long *addr) { struct klp_find_arg args = { - .objname = objname, .name = name, .addr = 0, .count = 0, @@ -183,7 +174,7 @@ static int klp_find_object_symbol(const char *objname, const char *name, }; if (objname) - module_kallsyms_on_each_symbol(klp_find_callback, &args); + module_kallsyms_on_each_symbol(objname, klp_find_callback, &args); else kallsyms_on_each_match_symbol(klp_match_callback, name, &args); diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c index f5c5c9175333df7..329cef573675d49 100644 --- a/kernel/module/kallsyms.c +++ b/kernel/module/kallsyms.c @@ -495,8 +495,8 @@ unsigned long module_kallsyms_lookup_name(const char *name) } #ifdef CONFIG_LIVEPATCH -int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, - struct module *, unsigned long), +int module_kallsyms_on_each_symbol(const char *modname, + int (*fn)(void *, const char *, unsigned long), void *data) { struct module *mod; @@ -510,6 +510,9 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, if (mod->state == MODULE_STATE_UNFORMED) continue; + if (strcmp(modname, mod->name)) + continue; + /* Use rcu_dereference_sched() to remain compliant with the sparse tool */ preempt_disable(); kallsyms = rcu_dereference_sched(mod->kallsyms); @@ -522,10 +525,16 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, continue; ret = fn(data, kallsyms_symbol_name(kallsyms, i), - mod, kallsyms_symbol_value(sym)); + kallsyms_symbol_value(sym)); if (ret != 0) goto out; } + + /* + * The given module is found, the subsequent modules do not + * need to be compared. + */ + break; } out: mutex_unlock(&module_mutex);
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. 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@huawei.com> --- include/linux/module.h | 4 ++-- kernel/livepatch/core.c | 13 ++----------- kernel/module/kallsyms.c | 15 ++++++++++++--- 3 files changed, 16 insertions(+), 16 deletions(-)