Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1679119 > unrolled thread

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

Started byJosh Poimboeuf <jpoimboe@redhat.com>
First post2017-06-30 22:40 +0200
Last post2017-07-01 09:00 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 3/3] livepatch: add shadow variable sample program Josh Poimboeuf <jpoimboe@redhat.com> - 2017-06-30 22:40 +0200
    Re: [PATCH 3/3] livepatch: add shadow variable sample program Miroslav Benes <mbenes@suse.cz> - 2017-07-01 09:00 +0200

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

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-06-30 22:40 +0200
SubjectRe: [PATCH 3/3] livepatch: add shadow variable sample program
Message-ID<tYaRQ-1G1-9@gated-at.bofh.it>
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

[toc] | [next] | [standalone]


#1679274

FromMiroslav Benes <mbenes@suse.cz>
Date2017-07-01 09:00 +0200
Message-ID<tYkxP-80e-5@gated-at.bofh.it>
In reply to#1679119
On Fri, 30 Jun 2017, Josh Poimboeuf wrote:

> 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.

Exactly.

It is on my TODO list right after the fake signal. I've been occupied by 
different things recently but I'll definitely return to it soon.

Regards,
Miroslav

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web