Re: [PATCH v4 13/15] livepatch: change to a per-task consistency model
From: Miroslav Benes <mbenes@suse.cz>
Date: 2017-02-07 08:21:54
Also in:
linux-s390, lkml
quoted
And finally, the section "Limitations" has this text under the first bullet: + The patch must not change the semantic of the patched functions. The current implementation guarantees only that either the old or the new function is called. The functions are patched one by one. It means that the patch must _not_ change the semantic of the function. I think it is confusing. The consistency model allows us to change the semantic of a function. To certain degree. Of course, there are cases that cannot be patched, or have to be patched carefully. For example if a function takes a lock by calling foo_lock(), foo_lock() is not on a stack afterwards. Then the locking semantics may be changed with a livepatch. One has to make sure to patch also the caller foo_lock() to enforce the consistency. And so on... But I do not consider a limitation of livepatch. It is a feature of the consistency model, which is weaker than kGraft's or kpatch's (or stronger. It depends on your point of view.) So, I propose to remove this text and better describe the properties of the consistency model above in the section 3. Maybe a quote from an old mail thread (Nov 2014) would be sufficient. I don't remember what was mentioned and what not. What do you think?I'll remove the above limitation. I'm not sure how to improve the consistency model section. It already has at least some mentions of changed function semantics and locking semantics. I'll leave it alone for now, unless you have a specific suggestion.
Fair enough. Let's see if I can come up with something.
quoted
quoted
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index 6602b34..ed90ad1 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h@@ -68,7 +92,7 @@ struct klp_func { * @funcs: function entries for functions to be patched in the object * @kobj: kobject for sysfs resources * @mod: kernel module associated with the patched object - * (NULL for vmlinux) + * (NULL for vmlinux)This looks superfluous.This is a minor whitespace fix -- remove a space before tab. I figured I'd go ahead and fix it since I'm already changing some of the surrounding code.
Ok, no problem. Thanks, Miroslav