mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Joe Lawrence <joe.lawrence@redhat.com>
To: Miroslav Benes <mbenes@suse.cz>, Petr Mladek <pmladek@suse.com>
Cc: Jiri Kosina <jikos@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Jason Baron <jbaron@akamai.com>, Jessica Yu <jeyu@kernel.org>,
	Evgenii Shatokhin <eshatokhin@virtuozzo.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8 8/8] livepatch: Atomic replace and cumulative patches documentation
Date: Fri, 23 Feb 2018 11:55:51 -0500	[thread overview]
Message-ID: <a50b564b-e0c8-321f-cf90-8c001e52499b@redhat.com> (raw)
In-Reply-To: <alpine.LSU.2.21.1802231139030.26550@pobox.suse.cz>

On 02/23/2018 05:41 AM, Miroslav Benes wrote:
> On Wed, 21 Feb 2018, Petr Mladek wrote:
> 
>> User documentation for the atomic replace feature. It makes it easier
>> to maintain livepatches using so-called cumulative patches.
>>
>> Signed-off-by: Petr Mladek <pmladek@suse.com>
> 
> Acked-by: Miroslav Benes <mbenes@suse.cz>
> 
> Joe, are you planning to extend shadow vars documentation you mentioned 
> before (v7), or do you want Petr to do it?

I created a sample cumulative patch for testing (I can post later),
but I had a quick question regarding the callbacks...

>From v8 of the patch:

  + Only the (un)patching callbacks from the _new_ cumulative livepatch are
    executed. Any callbacks from the replaced patches are ignored.

maybe I'm not testing this as intended, but I'm not seeing this
behavior with the latest version of the patch.  See below.


Init
====

Setup dynamic debug messages:

  % echo "file kernel/livepatch/* +p" > /sys/kernel/debug/dynamic_debug/control
  % echo "func klp_try_switch_task -p" >> /sys/kernel/debug/dynamic_debug/control


Test 1
======

Copy the original callbacks demo, leave the atomic .replace feature off:

diff -upr samples/livepatch/livepatch-callbacks-{demo.c,demo2.c}
--- samples/livepatch/livepatch-callbacks-demo.c        2018-02-23 09:42:10.370223095 -0500
+++ samples/livepatch/livepatch-callbacks-demo2.c       2018-02-23 11:38:08.372557153 -0500
@@ -191,6 +191,7 @@ static struct klp_object objs[] = {
 static struct klp_patch patch = {
        .mod = THIS_MODULE,
        .objs = objs,
+       .replace = false,
 };
 
 static int livepatch_callbacks_demo_init(void)


Load the two modules, disable and unload:

  % dmesg -C
  % insmod samples/livepatch/livepatch-callbacks-demo.ko
  % insmod samples/livepatch/livepatch-callbacks-demo2.ko
  % echo 0 > /sys/kernel/livepatch/livepatch_callbacks_demo2/enabled
  % echo 0 > /sys/kernel/livepatch/livepatch_callbacks_demo/enabled
  % rmmod samples/livepatch/livepatch-callbacks-demo2.ko
  % rmmod samples/livepatch/livepatch-callbacks-demo.ko

All callbacks are called for both livepatch modules (expected):

  % dmesg
  [ 5755.608999] livepatch: enabling patch 'livepatch_callbacks_demo'
  [ 5755.609014] livepatch: 'livepatch_callbacks_demo': initializing patching transition
> [ 5755.609054] livepatch_callbacks_demo: pre_patch_callback: vmlinux
  [ 5755.609055] livepatch: 'livepatch_callbacks_demo': starting patching transition
  [ 5756.704163] livepatch: 'livepatch_callbacks_demo': completing patching transition
> [ 5756.704259] livepatch_callbacks_demo: post_patch_callback: vmlinux
  [ 5756.704260] livepatch: 'livepatch_callbacks_demo': patching complete
  [ 5760.231035] livepatch: enabling patch 'livepatch_callbacks_demo2'
  [ 5760.231038] livepatch: 'livepatch_callbacks_demo2': initializing patching transition
> [ 5760.231077] livepatch_callbacks_demo2: pre_patch_callback: vmlinux
  [ 5760.231078] livepatch: 'livepatch_callbacks_demo2': starting patching transition
  [ 5761.696067] livepatch: 'livepatch_callbacks_demo2': completing patching transition
> [ 5761.696166] livepatch_callbacks_demo2: post_patch_callback: vmlinux
  [ 5761.696166] livepatch: 'livepatch_callbacks_demo2': patching complete
  [ 5765.738278] livepatch: 'livepatch_callbacks_demo2': initializing unpatching transition
> [ 5765.738319] livepatch_callbacks_demo2: pre_unpatch_callback: vmlinux
  [ 5765.738319] livepatch: 'livepatch_callbacks_demo2': starting unpatching transition
  [ 5767.712143] livepatch: 'livepatch_callbacks_demo2': completing unpatching transition
> [ 5767.712502] livepatch_callbacks_demo2: post_unpatch_callback: vmlinux
  [ 5767.712503] livepatch: 'livepatch_callbacks_demo2': unpatching complete
  [ 5770.909701] livepatch: 'livepatch_callbacks_demo': initializing unpatching transition
> [ 5770.909743] livepatch_callbacks_demo: pre_unpatch_callback: vmlinux
  [ 5770.909743] livepatch: 'livepatch_callbacks_demo': starting unpatching transition
  [ 5772.704108] livepatch: 'livepatch_callbacks_demo': completing unpatching transition
> [ 5772.704215] livepatch_callbacks_demo: post_unpatch_callback: vmlinux
  [ 5772.704216] livepatch: 'livepatch_callbacks_demo': unpatching complete


Test 2
======

Make the second livepatch a cumulative one:

diff -upr samples/livepatch/livepatch-callbacks-{demo.c,demo2.c}
--- samples/livepatch/livepatch-callbacks-demo.c        2018-02-23 09:42:10.370223095 -0500
+++ samples/livepatch/livepatch-callbacks-demo2.c       2018-02-23 11:16:20.557557153 -0500
@@ -191,6 +191,7 @@ static struct klp_object objs[] = {
 static struct klp_patch patch = {
        .mod = THIS_MODULE,
        .objs = objs,
+       .replace = true,
 };
 
 static int livepatch_callbacks_demo_init(void)

Load the two modules, disable and unload:

  % dmesg -C
  % insmod samples/livepatch/livepatch-callbacks-demo.ko
  % insmod samples/livepatch/livepatch-callbacks-demo2.ko
  % echo 0 > /sys/kernel/livepatch/livepatch_callbacks_demo2/enabled
  % rmmod samples/livepatch/livepatch-callbacks-demo2.ko
  % rmmod samples/livepatch/livepatch-callbacks-demo.ko

Pre and post patch callbacks are called for both modules (expected), but
*no* unpatch callbacks are executed:

  % dmesg
  [ 5442.244332] livepatch: enabling patch 'livepatch_callbacks_demo'
  [ 5442.244334] livepatch: 'livepatch_callbacks_demo': initializing patching transition
> [ 5442.244372] livepatch_callbacks_demo: pre_patch_callback: vmlinux
  [ 5442.244372] livepatch: 'livepatch_callbacks_demo': starting patching transition
  [ 5444.704089] livepatch: 'livepatch_callbacks_demo': completing patching transition
> [ 5444.704183] livepatch_callbacks_demo: post_patch_callback: vmlinux
  [ 5444.704184] livepatch: 'livepatch_callbacks_demo': patching complete
  [ 5448.567820] livepatch: enabling patch 'livepatch_callbacks_demo2'
  [ 5448.567823] livepatch: 'livepatch_callbacks_demo2': initializing patching transition
> [ 5448.567860] livepatch_callbacks_demo2: pre_patch_callback: vmlinux
  [ 5448.567861] livepatch: 'livepatch_callbacks_demo2': starting patching transition
  [ 5451.744131] livepatch: 'livepatch_callbacks_demo2': completing patching transition
> [ 5451.744212] livepatch_callbacks_demo2: post_patch_callback: vmlinux
  [ 5451.744213] livepatch: 'livepatch_callbacks_demo2': patching complete
  [ 5455.002339] livepatch: 'livepatch_callbacks_demo2': initializing unpatching transition
  [ 5455.002378] livepatch: 'livepatch_callbacks_demo2': starting unpatching transition
  [ 5456.736068] livepatch: 'livepatch_callbacks_demo2': completing unpatching transition
  [ 5456.736132] livepatch: 'livepatch_callbacks_demo2': unpatching complete


BTW, should we just add this test-case to the doc / samples?

-- Joe

  reply	other threads:[~2018-02-23 16:55 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 [this message]
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

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=a50b564b-e0c8-321f-cf90-8c001e52499b@redhat.com \
    --to=joe.lawrence@redhat.com \
    --cc=eshatokhin@virtuozzo.com \
    --cc=jbaron@akamai.com \
    --cc=jeyu@kernel.org \
    --cc=jikos@kernel.org \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®