Messages in this thread Patch in this message | | | Date | Fri, 14 May 2021 09:12:29 +0200 | From | Jessica Yu <> | Subject | Re: [PATCH] module: check for exit sections in layout_sections() instead of module_init_section() |
| |
+++ Russell King - ARM Linux admin [12/05/21 17:06 +0100]: >Hi, > >On Wed, May 12, 2021 at 04:46:53PM +0200, Jessica Yu wrote: >> diff --git a/kernel/module.c b/kernel/module.c >> index 173a09175511..a5c9842371b1 100644 >> --- a/kernel/module.c >> +++ b/kernel/module.c >> @@ -2430,6 +2430,9 @@ static void layout_sections(struct module *mod, struct load_info *info) >> if ((s->sh_flags & masks[m][0]) != masks[m][0] >> || (s->sh_flags & masks[m][1]) >> || s->sh_entsize != ~0UL >> +#ifndef CONFIG_MODULE_UNLOAD >> + || module_exit_section(sname) >> +#endif >> || module_init_section(sname)) > >How about a helper to make this a bit easier in both these places to >make the code more undertsandable? I think the great value comes from >the resulting change in the second hunk. > >static bool module_evictable_section(const char *sname) >{ >#ifndef CONFIG_MODULE_UNLOAD > if (module_exit_section(sname)) > return true; >#endif > return module_init_section(sname); >} > >and then just use that above? > >> continue; >> s->sh_entsize = get_offset(mod, &mod->core_layout.size, s, i); >> @@ -2463,7 +2466,11 @@ static void layout_sections(struct module *mod, struct load_info *info) >> if ((s->sh_flags & masks[m][0]) != masks[m][0] >> || (s->sh_flags & masks[m][1]) >> || s->sh_entsize != ~0UL >> +#ifndef CONFIG_MODULE_UNLOAD >> + || (!module_init_section(sname) && !module_exit_section(sname))) >> +#else >> || !module_init_section(sname)) >> +#endif > >I find this a tad confusing, and this is the reason for my suggestion >above. With that, this becomes: > > || !module_evictable_section(sname)) > >which can be clearly seen is the opposite condition from the above >without doing mental logic gymnastics.
Thanks Russell for the feedback! Yeah, agreed that it could be made easier to read - will respin with a helper function.
Jessica
| |