From: Petr Mladek <pmladek@suse.com>
To: Miroslav Benes <mbenes@suse.cz>
Cc: Evgenii Shatokhin <eshatokhin@virtuozzo.com>,
Jason Baron <jbaron@akamai.com>, Jiri Kosina <jikos@kernel.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Joe Lawrence <joe.lawrence@redhat.com>,
Jessica Yu <jeyu@kernel.org>,
live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8 0/8] livepatch: Atomic replace feature
Date: Tue, 6 Mar 2018 16:32:08 +0100 [thread overview]
Message-ID: <20180306153208.35cmwo7wcdfwa2hd@pathway.suse.cz> (raw)
In-Reply-To: <alpine.LSU.2.21.1803051352300.26605@pobox.suse.cz>
On Mon 2018-03-05 13:54:38, Miroslav Benes wrote:
> On Mon, 5 Mar 2018, Evgenii Shatokhin wrote:
>
> > Hi,
>
> Hi,
>
> > > The atomic replace allows to create cumulative patches. They
> > > are useful when you maintain many livepatches and want to remove
> > > one that is lower on the stack. In addition it is very useful when
> > > more patches touch the same function and there are dependencies
> > > between them.
> >
> > I have experimented with this updated patchset a bit.
> > It looks like replace operation fails if there is a loaded but disabled patch.
> >
> > Suppose there are 2 binary patches changing the same kernel functions. Load
> > patch1, then disable it (echo 0 > /sys/kernel/livepatch/patch1/enabled), then
> > try load patch2 with atomic replace. I get "Device or resource busy" error at
> > this point.
> >
> > It seems, __klp_enable_patch() returns -EBUSY for some reason. I haven't dug
> > deeper into that code yet.
>
> Yes, it is "enforce stacking" check in __klp_enable_patch():
>
> /* enforce stacking: only the first disabled patch can be enabled */
> if (patch->list.prev != &klp_patches &&
> !list_prev_entry(patch, list)->enabled)
> return -EBUSY;
>
> So not connected to this patch set. We've had the behaviour for quite some
> time.
We actually wanted to enabled this for the patches with the replace
flag. It is even described in
Documentation/livepatch/cumulative-patches.txt.
Of course, you could always unregister all the disabled patches that
block the new one. But it might be inconvenient.
I do not see any big reason why this should be disabled:
+ klp_add_nops() takes into account even disabled patches.
In the worst case, we might enable some NOPs that are
not really needed.
+ klp_throw_away_replaced_patches() removes all patches down
the stack.
I am going to enable this in a separate patch in v10.
Best Regards,
Petr
prev parent reply other threads:[~2018-03-06 15:32 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-21 13:29 [PATCH v8 0/8] livepatch: Atomic replace feature Petr Mladek
2018-02-21 13:29 ` [PATCH v8 1/8] livepatch: Use lists to manage patches, objects and functions Petr Mladek
2018-02-21 13:29 ` [PATCH v8 2/8] livepatch: Free only structures with initialized kobject Petr Mladek
2018-02-21 13:29 ` [PATCH v8 3/8] livepatch: Initial support for dynamic structures Petr Mladek
2018-02-21 13:29 ` [PATCH v8 4/8] livepatch: Allow to unpatch only functions of the given type Petr Mladek
2018-02-22 15:25 ` Miroslav Benes
2018-02-21 13:29 ` [PATCH v8 5/8] livepatch: Support separate list for replaced patches Petr Mladek
2018-02-21 13:29 ` [PATCH v8 6/8] livepatch: Add atomic replace Petr Mladek
2018-02-21 13:29 ` [PATCH v8 7/8] livepatch: Correctly handle atomic replace for not yet loaded modules Petr Mladek
2018-02-22 21:00 ` Miroslav Benes
2018-03-01 10:28 ` Petr Mladek
2018-03-02 22:00 ` Joe Lawrence
2018-03-05 9:54 ` Miroslav Benes
2018-03-05 14:42 ` Josh Poimboeuf
2018-03-06 14:10 ` Petr Mladek
2018-02-21 13:29 ` [PATCH v8 8/8] livepatch: Atomic replace and cumulative patches documentation Petr Mladek
2018-02-23 10:41 ` Miroslav Benes
2018-02-23 16:55 ` Joe Lawrence
2018-02-23 20:40 ` Miroslav Benes
2018-02-22 15:23 ` [PATCH v8 0/8] livepatch: Atomic replace feature Miroslav Benes
2018-03-05 10:56 ` Evgenii Shatokhin
2018-03-05 12:54 ` Miroslav Benes
2018-03-06 15:32 ` Petr Mladek [this message]
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=20180306153208.35cmwo7wcdfwa2hd@pathway.suse.cz \
--to=pmladek@suse.com \
--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=mbenes@suse.cz \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).