All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miroslav Benes <mbenes@suse.cz>
To: Petr Mladek <pmladek@suse.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>,
	Jiri Kosina <jikos@kernel.org>, Jason Baron <jbaron@akamai.com>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Jessica Yu <jeyu@kernel.org>,
	Evgenii Shatokhin <eshatokhin@virtuozzo.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v10 05/10] livepatch: Support separate list for replaced patches.
Date: Tue, 20 Mar 2018 14:30:10 +0100 (CET)	[thread overview]
Message-ID: <alpine.LSU.2.21.1803201425090.1312@pobox.suse.cz> (raw)
In-Reply-To: <20180320122538.t75rplwhmhtap5q2@pathway.suse.cz>


> > I don't know, does anybody really care about this case (patching on top
> > of a disabled patch)?  It just adds to the crazy matrix of possible
> > scenarios we have to keep in our heads, which means more bugs, for very
> > little (hypothetical) gain.
> 
> It depends if the we remove the replaced patches or not. If they are
> removed then replacing disabled patches is rather trivial from both
> coding and understanding side.

I agree. Since we already have the code, there is no point not to have the 
feature. It is not that complicated after all.
 
> I am going to add this as a separate patch as well. Let's discuss
> it with the code.
> 
> 
> > > White the atomic replace could make things easier for both developers
> > > and users.
> > 
> > I agree that atomic replace is a useful feature and I'm not arguing
> > against it, so maybe I missed your point?
> 
> Your suggestion allows easier code but it reduces the advantages of
> the atomic replace feature. We would achieve almost the same results
> with a normal livepatch where the functions behave like in
> the original code.
> 
> Also removing replaced patches can be seen as a clean up after
> each patch. It might be more code but the target system might be
> easier to debug. Also we do not need to mind about various
> disable scenarios.

I agree with this as well.

Yes, it was a bit painful to review, but I was quite content with the 
result. I don't want to go halfway and be stuck with NOPs when it is 
not complicated to remove them completely after the transition. It'd be 
odd in my opinion.

Regards,
Miroslav

  parent reply	other threads:[~2018-03-20 13:32 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-07  8:20 [PATCH v10 00/10] livepatch: Atomic replace feature Petr Mladek
2018-03-07  8:20 ` [PATCH v10 01/10] livepatch: Use lists to manage patches, objects and functions Petr Mladek
2018-03-13 22:38   ` Josh Poimboeuf
2018-03-07  8:20 ` [PATCH v10 02/10] livepatch: Free only structures with initialized kobject Petr Mladek
2018-03-13 22:38   ` Josh Poimboeuf
2018-03-14 15:50     ` Petr Mladek
2018-03-07  8:20 ` [PATCH v10 03/10] livepatch: Initial support for dynamic structures Petr Mladek
2018-03-13 22:44   ` Josh Poimboeuf
2018-03-19 13:10     ` Petr Mladek
2018-03-07  8:20 ` [PATCH v10 04/10] livepatch: Allow to unpatch only functions of the given type Petr Mladek
2018-03-07  8:20 ` [PATCH v10 05/10] livepatch: Support separate list for replaced patches Petr Mladek
2018-03-13 22:46   ` Josh Poimboeuf
2018-03-19 15:02     ` Petr Mladek
2018-03-19 21:43       ` Josh Poimboeuf
2018-03-20 12:25         ` Petr Mladek
2018-03-20 12:48           ` Evgenii Shatokhin
2018-03-20 13:30           ` Miroslav Benes [this message]
2018-03-20 20:15           ` Josh Poimboeuf
2018-03-23  9:45             ` Petr Mladek
2018-03-23 22:44               ` Josh Poimboeuf
2018-03-26 10:11                 ` Petr Mladek
2018-04-06 19:50                   ` Josh Poimboeuf
2018-04-10  8:34                     ` Petr Mladek
2018-04-10 13:21                       ` Miroslav Benes
2018-04-10 13:56                         ` Evgenii Shatokhin
2018-04-10 17:47                           ` Josh Poimboeuf
2018-04-11  7:56                             ` Miroslav Benes
2018-04-10 17:42                       ` Josh Poimboeuf
2018-04-11  8:07                         ` Miroslav Benes
2018-04-11 12:32                           ` Josh Poimboeuf
2018-04-11 13:39                             ` Miroslav Benes
2018-04-11 14:17                             ` Petr Mladek
2018-04-11 15:48                               ` Josh Poimboeuf
2018-04-16 14:58                                 ` Petr Mladek
2018-04-16 19:04                                   ` Josh Poimboeuf
2018-04-17  8:23                                     ` Miroslav Benes
2018-04-17 15:37                                     ` Petr Mladek
2018-04-17 17:57                                       ` Josh Poimboeuf
2018-03-07  8:20 ` [PATCH v10 06/10] livepatch: Add atomic replace Petr Mladek
2018-03-13 22:48   ` Josh Poimboeuf
2018-03-20 14:35     ` Petr Mladek
2018-03-20 21:26       ` Josh Poimboeuf
2018-03-22 15:43         ` Petr Mladek
2018-03-07  8:20 ` [PATCH v10 07/10] livepatch: Correctly handle atomic replace for not yet loaded modules Petr Mladek
2018-03-13 14:55   ` Petr Mladek
2018-03-07  8:20 ` [PATCH v10 08/10] livepatch: Improve dynamic struct klp_object detection and manipulation Petr Mladek
2018-03-07  8:20 ` [PATCH v10 09/10] livepatch: Allow to replace even disabled patches Petr Mladek
2018-03-07  8:20 ` [PATCH v10 10/10] livepatch: Atomic replace and cumulative patches documentation Petr Mladek
2018-03-07 21:55 ` [PATCH v10 00/10] livepatch: Atomic replace feature Joe Lawrence
2018-03-08 15:01   ` Petr Mladek
2018-03-08 15:09     ` Joe Lawrence
2018-03-12 18:57       ` Joe Lawrence
2018-03-20 13:16         ` Miroslav Benes
2018-03-26 10:56         ` Petr Mladek
2018-03-26 18:12           ` Joe Lawrence
2018-03-27  8:22             ` Petr Mladek
2018-08-17 10:17 ` Evgenii Shatokhin
2018-08-17 14:53   ` Petr Mladek
2018-08-17 15:33     ` Evgenii Shatokhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LSU.2.21.1803201425090.1312@pobox.suse.cz \
    --to=mbenes@suse.cz \
    --cc=eshatokhin@virtuozzo.com \
    --cc=jbaron@akamai.com \
    --cc=jeyu@kernel.org \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=pmladek@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.