On Fri, Apr 01, 2016 at 03:39:44PM +0200, Petr Mladek wrote:

On Fri 2016-03-25 14:34:47, Josh Poimboeuf wrote:

quoted

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.

quoted

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.

quoted

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.

quoted

- 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