(2015/02/10 2:31), Josh Poimboeuf wrote: > This patch set implements a livepatch consistency model, targeted for 3.21. > Now that we have a solid livepatch code base, this is the biggest remaining > missing piece. > > This code stems from the design proposal made by Vojtech [1] in November. It > makes live patching safer in general. Specifically, it allows you to apply > patches which change function prototypes. It also lays the groundwork for > future code changes which will enable data and data semantic changes. Interesting, How would you do that? > It's basically a hybrid of kpatch and kGraft, combining kpatch's backtrace > checking with kGraft's per-task consistency. When patching, tasks are > carefully transitioned from the old universe to the new universe. A task can > only be switched to the new universe if it's not using a function that is to be > patched or unpatched. After all tasks have moved to the new universe, the > patching process is complete. > > How it transitions various tasks to the new universe: > > - The stacks of all sleeping tasks are checked. Each task that is not sleeping > on a to-be-patched function is switched. > > - Other user tasks are handled by do_notify_resume() (see patch 9/9). If a > task is I/O bound, it switches universes when returning from a system call. > If it's CPU bound, it switches when returning from an interrupt. If it's > sleeping on a patched function, the user can send SIGSTOP and SIGCONT to > force it to switch upon return from the signal handler. Ah, OK. So you can handle those without hooking switch_to :) > > - Idle "swapper" tasks which are sleeping on a to-be-patched function can be > switched from within the outer idle loop. > > - An interrupt handler will inherit the universe of the task it interrupts. > > - kthreads which are sleeping on to-be-patched functions are not yet handled > (more on this below). > > > I think this approach provides the best benefits of both kpatch and kGraft: > > advantages vs kpatch: > - no stop machine latency Good! :) > - higher patch success rate (can patch in-use functions) > - patching failures are more predictable (primary failure mode is attempting to > patch a kthread which is sleeping forever on a patched function, more on this > below) > > advantages vs kGraft: > - less code complexity (don't have to hack up the code of all the different > kthreads) > - less impact to processes (don't have to signal all sleeping tasks) > > disadvantages vs kpatch: > - no system-wide switch point (not really a functional limitation, just forces > the patch author to be more careful. but that's probably a good thing anyway) OK, we must check carefully that the old function and new function can be co-exist. > My biggest concerns and questions related to this patch set are: > > 1) To safely examine the task stacks, the transition code locks each task's rq > struct, which requires using the scheduler's internal rq locking functions. > It seems to work well, but I'm not sure if there's a cleaner way to safely > do stack checking without stop_machine(). We'd better ask scheduler people. > > 2) As mentioned above, kthreads which are always sleeping on a patched function > will never transition to the new universe. This is really a minor issue > (less than 1% of patches). It's not necessarily something that needs to be > resolved with this patch set, but it would be good to have some discussion > about it regardless. > > To overcome this issue, I have 1/2 an idea: we could add some stack checking > code to the ftrace handler itself to transition the kthread to the new > universe after it re-enters the function it was originally sleeping on, if > the stack doesn't already have have any other to-be-patched functions. > Combined with the klp_transition_work_fn()'s periodic stack checking of > sleeping tasks, that would handle most of the cases (except when trying to > patch the high-level thread_fn itself). It makes sense to me. (I just did similar thing) > > But then how do you make the kthread wake up? As far as I can tell, > wake_up_process() doesn't seem to work on a kthread (unless I messed up my > testing somehow). What does kGraft do in this case? Hmm, at a glance, the code itself can work on kthread too... Maybe you can also send you testing patch too. Thank you, > > > [1] https://lkml.org/lkml/2014/11/7/354 > > > Josh Poimboeuf (9): > livepatch: simplify disable error path > livepatch: separate enabled and patched states > livepatch: move patching functions into patch.c > livepatch: get function sizes > sched: move task rq locking functions to sched.h > livepatch: create per-task consistency model > proc: add /proc/<pid>/universe to show livepatch status > livepatch: allow patch modules to be removed > livepatch: update task universe when exiting kernel > > arch/x86/include/asm/thread_info.h | 4 +- > arch/x86/kernel/signal.c | 4 + > fs/proc/base.c | 11 ++ > include/linux/livepatch.h | 38 ++-- > include/linux/sched.h | 3 + > kernel/fork.c | 2 + > kernel/livepatch/Makefile | 2 +- > kernel/livepatch/core.c | 360 ++++++++++--------------------------- > kernel/livepatch/patch.c | 206 +++++++++++++++++++++ > kernel/livepatch/patch.h | 26 +++ > kernel/livepatch/transition.c | 318 ++++++++++++++++++++++++++++++++ > kernel/livepatch/transition.h | 16 ++ > kernel/sched/core.c | 34 +--- > kernel/sched/idle.c | 4 + > kernel/sched/sched.h | 33 ++++ > 15 files changed, 747 insertions(+), 314 deletions(-) > create mode 100644 kernel/livepatch/patch.c > create mode 100644 kernel/livepatch/patch.h > create mode 100644 kernel/livepatch/transition.c > create mode 100644 kernel/livepatch/transition.h > -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@xxxxxxxxxxx -- 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