Re: [RFC PATCH 0/2] kpatch: dynamic kernel patching

[Frederic Weisbecker]

On Mon, May 05, 2014 at 08:43:04PM +0200, Ingo Molnar wrote:
> 
> * Frederic Weisbecker <fweisbec@xxxxxxxxx> wrote:
> 
> > On Mon, May 05, 2014 at 08:26:38AM -0500, Josh Poimboeuf wrote:
> > > On Mon, May 05, 2014 at 10:55:37AM +0200, Ingo Molnar wrote:
> > > > 
> > > > * Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> > > > 
> > > > > [...]
> > > > > 
> > > > > kpatch checks the backtraces of all tasks in stop_machine() to 
> > > > > ensure that no instances of the old function are running when the 
> > > > > new function is applied.  I think the biggest downside of this 
> > > > > approach is that stop_machine() has to idle all other CPUs during 
> > > > > the patching process, so it inserts a small amount of latency (a few 
> > > > > ms on an idle system).
> > > > 
> > > > When live patching the kernel, how about achieving an even 'cleaner' 
> > > > state for all tasks in the system: to freeze all tasks, as the suspend 
> > > > and hibernation code (and kexec) does, via freeze_processes()?
> > > > 
> > > > That means no tasks in the system have any real kernel execution 
> > > > state, and there's also no problem with long-sleeping tasks, as 
> > > > freeze_processes() is supposed to be fast as well.
> > > > 
> > > > I.e. go for the most conservative live patching state first, and relax 
> > > > it only once the initial model is upstream and is working robustly.
> > > 
> > > I had considered doing this before, but the problem I found is 
> > > that many kernel threads are unfreezable.  So we wouldn't be able 
> > > to check whether its safe to replace any functions in use by those 
> > > kernel threads.
> > 
> > OTOH many kernel threads are parkable. Which achieves kind of 
> > similar desired behaviour: the kernel threads then aren't running.
> > 
> > And in fact we could implement freezing on top of park for kthreads.
> > 
> > But unfortunately there are still quite some of them which don't 
> > support parking.
> 
> Well, if distros are moving towards live patching (and they are!), 
> then it looks rather necessary to me that something scary as flipping 
> out live kernel instructions with substantially different code should 
> be as safe as possible, and only then fast.

Sure I won't argue that.

> 
> If a kernel refuses to patch with certain threads running, that will 
> drive those kernel threads being fixed and such. It's a deterministic, 
> recoverable, reportable bug situation, so fixing it should be fast.
> 
> We learned these robustness lessons the hard way with kprobes and 
> ftrace dynamic code patching... which are utterly simple compared to 
> live kernel patching!

Yeah, agreed. More rationale behind: we want to put the kthreads into
semantic sleeps, not just random sleeping point. This way we lower the
chances to execute new code messing up living state that is expecting old
code after random preemption or sleeping points.

But by semantic sleeps I mean more than just explicit calls to schedule()
as opposed to preemption points.
It also implies shutting down as well the living states handled by the kthread
such that some sort of re-initialization of the state is also needed when
the kthread gets back to run.

And that's exactly what good implementations of kthread park provide.

Consider kernel/watchdog.c as an example: when we park the lockup
detector kthread, it disables the perf event and the hrtimer before it goes
to actually park and sleep. When the kthread is later unparked, the kthread
restarts the hrtimer and the perf event.

If we live patch code that has obscure relations with perf or hrtimer here,
we lower a lot the chances for a crash when the watchdog kthread is parked.

So I'm in favour of parking all possible kthreads before live patching. Freezing
alone doesn't provide the same state shutdown than parking.

Now since parking looks more widely implemented than kthread freezing, we could
even think about implementing kthread freezing using parking as backend.


> 
> Thanks,
> 
> 	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/