Thread (60 messages) flat view 60 messages, 6 authors, 2015-11-16

Re: samples: livepatch: init reloc list and mark as klp module

From: Miroslav Benes <hidden>
Date: 2015-11-12 10:45:25
Also in: lkml

On Thu, 12 Nov 2015, Jessica Yu wrote:
quoted hunk ↗ jump to hunk
+++ Petr Mladek [11/11/15 16:42 +0100]:
quoted
On Mon 2015-11-09 23:45:54, Jessica Yu wrote:
quoted
Intialize the list of relocation sections in the sample
klp_object (even if the list will be empty in this case).
Also mark module as a livepatch module so that the module
loader can appropriately initialize it.

Signed-off-by: Jessica Yu <redacted>
---
 samples/livepatch/livepatch-sample.c | 2 ++
 1 file changed, 2 insertions(+)
diff --git a/samples/livepatch/livepatch-sample.c
b/samples/livepatch/livepatch-sample.c
index fb8c861..2ef9345 100644
--- a/samples/livepatch/livepatch-sample.c
+++ b/samples/livepatch/livepatch-sample.c
@@ -89,3 +90,4 @@ static void livepatch_exit(void)
 module_init(livepatch_init);
 module_exit(livepatch_exit);
 MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
This looks a bit error prone. I wonder if we could detect this
information another way. For example, by a check for the
livepatch-related elf sections. If it is missing,
we do not need to preserve struct load_info even
when it is a livepatch.
Yeah, I agree that it is unnecessary for a livepatch module without
reloc secs to keep a copy of the load_info struct. My justification
for using MODULE_INFO is that I was trying to be consistent with the
way how other module "characteristics" are checked in the module
loader. For example, if the module came from the staging tree, the
module loader simply checks get_modinfo(info, "staging")). If the
module is a livepatch module, we check get_modinfo(info,
"livepatch")). I also thought that it might be useful additional
information for the user to be able to issue the modinfo command on a
module to see if it's a livepatch module or not (but maybe this
information won't be so useful after all, that's quite subjective).
Yup, in my opinion this is a good way to do it. We already impose quite a 
lot on a patch module and this does not make a big difference. Easy 
identification of a patch module is good bonus as well.
But if we want to do a more thorough check, we could, like you said,
check for the livepatch-related elf sections before copying load_info.
I wouldn't do that. It could be even more error prone.

I'd like to think that we can live with load_info struct even for patch 
modules which do not use relocations. Don't know.

Miroslav
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help