On Thu 2022-02-10 11:44 +0000, Christophe Leroy wrote: > This change is wrong, build fails with it because is_livepatch_module() > is nowhere defined. Yes, sorry about that. This was an omission/or oversight during the rebase attempt. > You could move is_livepatch_module() to include/linux/livepatch.h but as > a separate patch. Fair enough. Albeit, I'd prefer to revert and keep is_livepatch_module() in include/linux/module.h - this is likely the best solution. Note: set_livepatch_module() will remain for internal use only. > > #else /* !CONFIG_LIVEPATCH */ > > static inline bool is_livepatch_module(struct module *mod) > > { > > diff --git a/kernel/module/Makefile b/kernel/module/Makefile > > index 2902fc7d0ef1..ee20d864ad19 100644 > > --- a/kernel/module/Makefile > > +++ b/kernel/module/Makefile > > @@ -7,3 +7,6 @@ obj-$(CONFIG_MODULES) += main.o > > obj-$(CONFIG_MODULE_DECOMPRESS) += decompress.o > > obj-$(CONFIG_MODULE_SIG) += signing.o > > obj-$(CONFIG_MODULE_SIG_FORMAT) += signature.o > > +ifdef CONFIG_MODULES > > CONFIG_LIVEPATCH depends on CONFIG_MODULES so this ifdef is not needed Agreed. > Checkpatch reports the following: > > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? > #80: > new file mode 100644 > > CHECK: Comparison to NULL could be written "!mod->klp_info" > #109: FILE: kernel/module/livepatch.c:25: > + if (mod->klp_info == NULL) > > CHECK: Comparison to NULL could be written "!mod->klp_info->sechdrs" > #119: FILE: kernel/module/livepatch.c:35: > + if (mod->klp_info->sechdrs == NULL) { > > CHECK: Comparison to NULL could be written "!mod->klp_info->secstrings" > #127: FILE: kernel/module/livepatch.c:43: > + if (mod->klp_info->secstrings == NULL) { > > CHECK: No space is necessary after a cast > #142: FILE: kernel/module/livepatch.c:58: > + mod->klp_info->sechdrs[symndx].sh_addr = (unsigned long) > mod->core_kallsyms.symtab; Ok. > Given how simple this function is, it should be a 'static inline' in > internal.c Agreed. > CHECK: Alignment should match open parenthesis > #285: FILE: kernel/module/main.c:3033: > + pr_err("%s: module is marked as livepatch module, but livepatch > support is disabled", > + mod->name); Fair enough. Kind regards, -- Aaron Tomlin