On Fri, Apr 01, 2016 at 03:39:44PM +0200, Petr Mladek wrote: > On Fri 2016-03-25 14:34:47, Josh Poimboeuf wrote: > > These patches are still a work in progress, but Jiri asked that I share > > them before I go on vacation next week. Based on origin/master because > > it has CONFIG_STACK_VALIDATION. > > I have to follow Mirek and say that it is a great work. > > > There's also a func->immediate flag which allows users to specify that > > certain functions in the patch can be applied without per-task > > consistency. This might be useful if you want to patch a common > > function like schedule(), and the function change doesn't need > > consistency but the rest of the patch does. > > We probably should not set func->transition flag when func->immediate > is set or when the related func->object is set. It currently happens > only when patch->immediate is set. Agreed, I'll skip setting func->transition if func->immediate is set. > > Still have a lot of TODOs, some of them are listed here. If you see > > something objectionable, it might be a good idea to make sure it's not > > already on the TODO list :-) > > > > TODO: > > - come up with a better name than universe? KLP_STATE_PREV/NEXT? > > KLP_UNPATCHED/PATCHED? there were some objections to the name in v1. > > The name "universe" has an advantage if we later allow to > enable/disable more patches in parallel. The integer might hold > an identifier of the last applied patch. I have been playing with > this for kGraft one year ago and it was really challenging. > We should avoid it if possible. It is not really needed > if we are able to complete any transition in a reasonable time. Agreed, we should really avoid having more than two universes at a time, and I don't foresee a reason to do that in the future. I'll probably get rid of the universe name in favor of something more descriptive. > If we support only one transition at a time, a simple boolean > or even bit should be enough. The most descriptive name would > be klp_transition_patch_applied but it is quite long. Yeah, I'll change it to a bool. > Note that similar information is provided by TIF_KLP_NEED_UPDATE > flag. We use only this bit in kGraft. It saves some space in > task_struct but it brings other challenges. We need to prevent > migration using a global "kgr_immutable" flag until ftrace handlers > for all patched functions are in place. We need to set the flag > back when the ftrace handler is called and the global "kgr_immutable" > flag is set. > > > - update documentation for sysfs, proc, livepatch > > Also we should publish somewhere the information about TIF_KLP_NEED_UPDATE > flag, e.g. /proc/<pid>/klp_need_update. It is handy to see what > process blocks the transition. We have something similar in > kGraft, see in_progress_show() at > https://git.kernel.org/cgit/linux/kernel/git/jirislaby/kgraft.git/commit/?h=kgraft-4.4&id=1c82fbd7b1fe240f4ed178a6506a93033f6a4bed Patch 13/14 exposes the per-task patched state in /proc, so I think that already does what you're asking for? It doesn't expose TIF_KLP_NEED_UPDATE, but that's more of an internal detail I think. -- Josh -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
