Re: [PATCH 3/3] livepatch: add shadow variable sample program

From: Josh Poimboeuf
Date: Fri Jun 30 2017 - 16:32:32 EST


On Mon, Jun 19, 2017 at 06:56:37PM +0200, Miroslav Benes wrote:
>
> > > > I often wonder whether it's really a good idea to even allow the
> > > > unloading of patch modules at all. It adds complexity to the livepatch
> > > > code. Is it worth it? I don't have an answer but I'd be interested in
> > > > other people's opinion.
> > >
> > > I could imagine a situation when a livepatch causes, for example,
> > > performance, problems on a server because of the redirection
> > > to the new code. Then it might be handy to disable the patch
> > > and ftrace handlers completely.
> >
> > Fair enough, though it sounds theoretical. It would be good to know
> > we're supporting actual real world use cases.
>
> We distribute cumulative "replace_all" patches at SUSE. replace_all means
> that all previous patches are reverted in the process of application. All
> livepatch modules with zero refcount are removed. This keeps a number of
> loaded modules low and system's state well defined, which is always a good
> thing, because a customer might run into problems and we'd have to debug
> it.

We used to have something similar in kpatch. And we recently discovered
that this "replace_all" feature would also be nice to have in livepatch.

We had a patch B which needed to partially revert patch A. We had to
manually do the revert at a function level, which basically means
repatching the function so that it resembles its original state.

It would be much more straightforward to be able to tell klp to revert
everything in patch A while applying patch B. Then the func stack would
never have more than one entry. And that would be good for performance
as well.

--
Josh