All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Joe Lawrence <joe.lawrence@redhat.com>
Cc: Miroslav Benes <mbenes@suse.cz>, Song Liu <song@kernel.org>,
	live-patching@vger.kernel.org, jpoimboe@kernel.org,
	jikos@kernel.org, kernel-team@meta.com
Subject: Re: [PATCH v3 8/8] livepatch: Add tests for klp-build toolchain
Date: Thu, 5 Mar 2026 16:20:51 +0100	[thread overview]
Message-ID: <aamfUxhAM_EZ0MtR@pathway.suse.cz> (raw)
In-Reply-To: <aaiI7zv2JVgXZkPm@redhat.com>

On Wed 2026-03-04 14:33:03, Joe Lawrence wrote:
> On Mon, Mar 02, 2026 at 09:38:17AM +0100, Miroslav Benes wrote:
> > Him
> > 
> > > > We store test modules in tools/testing/selftests/livepatch/test_modules/
> > > > now. Could you move klp_test_module.c there, please? You might also reuse
> > > > existing ones for the purpose perhaps.
> > > 
> > > IIUC, tools/testing/selftests/livepatch/test_modules/ is more like an out
> > > of tree module. In the case of testing klp-build, we prefer to have it to
> > > work the same as in-tree modules. This is important because klp-build
> > > is a toolchain, and any changes of in-tree Makefiles may cause issues
> > > with klp-build. Current version can catch these issues easily. If we build
> > > the test module as an OOT module, we may miss some of these issues.
> > > In the longer term, we should consider adding klp-build support to build
> > > livepatch for OOT modules. But for now, good test coverage for in-tree
> > > modules are more important.
> > 
> > Ok. I thought it would not matter but it is a fair point.
> > 
> > > > What about vmlinux? I understand that it provides a lot more flexibility
> > > > to have separate functions for testing but would it be somehow sufficient
> > > > to use the existing (real) kernel functions? Like cmdline_proc_show() and
> > > > such which we use everywhere else? Or would it be to limited? I am fine if
> > > > you find it necessary in the end. I just think that reusing as much as
> > > > possible is generally a good approach.
> > > 
> > > I think using existing functions would be too limited, and Joe seems to
> > > agree with this based on his experience. To be able to test corner cases
> > > of the compiler/linker, such as LTO, we need special code patterns.
> > > OTOH, if we want to use an existing kernel function for testing, it needs
> > > to be relatively stable, i.e., not being changed very often. It is not always
> > > easy to find some known to be stable code that follows specific patterns.
> > > If we add dedicated code as test targets, things will be much easier
> > > down the road.
> > 
> > Fair enough.
> > 
> Do the helpers in functions.sh for safely loading and unloading
> livepatches (that wait for the transition, etc.) aid here?
> 
> > > > And a little bit of bikeshedding at the end. I think it would be more
> > > > descriptive if the new config options and tests (test modules) have
> > > > klp-build somewhere in the name to keep it clear. What do you think?
> > > 
> > > Technically, we can also use these tests to test other toolchains, for
> > > example, kpatch-build. I don't know ksplice or kGraft enough to tell
> > > whether they can benefit from these tests or not. OTOH, I am OK
> > > changing the name/description of these config options.
> > 
> > I would prefer it, thank you. Unless someone else objects of course.
> > 
> 
> To continue the bike shedding, in my branch, I had dumped this all under
> a new tools/testing/klp-build subdirectory as my focus was to put
> klp-build through the paces.  It does load the generated livepatches in
> the runtime testing, but as only as a sanity check.  With that, it
> didn't touch CONFIG or intermix testing with the livepatch/ set.

The question is what you expect from the klp-build testing.

My understanding is that the current test primary checks whether:

  + klp-build machinery is able to put together a working livepatch
  + "objtool klp diff" is able to find and match the right symbols

I assume this because the README contains a very simple steps:

    <paste>
    3. Verify the correctness with:

      modprobe klp_test_module
      kpatch load livepatch-patch.ko
      grep -q unpatched /sys/kernel/klp_test/*/* && echo FAIL || echo PASS
    </paste>

A separate directory would be perfectly fine in this case.

We could keep the existing selftests/livepatch as is for
testing the kernel/livepatch features, e.g. transition,
shadow variables, callbacks, cooperation with ftrace/kprobe.

An integration with the existing selftests/livepatch would be
better if you wanted to do more complicated tests. You
might want to reuse the existing framework for checking
the dmesg output.

Honestly, I guess that we would want to integrate both tests
sooner or later anyway. You might want to test how klp-build
handles the extra features, e.g. shadow variables, ...

But the integration might be non-trivial.

> If we do end up supplementing the livepatch/ with klp-build tests, then
> I agree that naming them (either filename prefix or subdirectory) would
> be nice.
> 
> But first, is it goal for klp-build to be the build tool (rather than
> simple module kbuild) for the livepatching .ko selftests?

Best Regards,
Petr

PS: Sigh, I hope that I'll find time to send v2 of the patchset reworking
    the callbacks, shadow variables, and states. It modifies/reworks
    the existing selftests modules a lot, see v1 at
    https://lore.kernel.org/all/20250115082431.5550-1-pmladek@suse.com/

  parent reply	other threads:[~2026-03-05 15:20 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-26  0:54 [PATCH v3 0/8] objtool/klp: klp-build LTO support and tests Song Liu
2026-02-26  0:54 ` [PATCH v3 1/8] objtool/klp: Remove redundent strcmp in correlate_symbols Song Liu
2026-03-05 19:38   ` Josh Poimboeuf
2026-02-26  0:54 ` [PATCH v3 2/8] objtool/klp: Remove trailing '_' in demangle_name() Song Liu
2026-02-26  0:54 ` [PATCH v3 3/8] objtool/klp: Use sym->demangled_name for symbol_name hash Song Liu
2026-03-05 19:43   ` Josh Poimboeuf
2026-02-26  0:54 ` [PATCH v3 4/8] objtool/klp: Also demangle global objects Song Liu
2026-02-26  0:54 ` [PATCH v3 5/8] objtool/klp: Remove .llvm suffix in demangle_name() Song Liu
2026-02-26  0:54 ` [PATCH v3 6/8] objtool/klp: Match symbols based on demangled_name for global variables Song Liu
2026-02-26  0:54 ` [PATCH v3 7/8] objtool/klp: Correlate locals to globals Song Liu
2026-03-05 19:51   ` Josh Poimboeuf
2026-03-05 23:10     ` Song Liu
2026-02-26  0:54 ` [PATCH v3 8/8] livepatch: Add tests for klp-build toolchain Song Liu
2026-02-27 10:04   ` Miroslav Benes
2026-02-27 17:26     ` Song Liu
2026-03-02  8:38       ` Miroslav Benes
2026-03-04 19:33         ` Joe Lawrence
2026-03-04 23:12           ` Song Liu
2026-03-05  1:39             ` Joe Lawrence
2026-03-05  5:03               ` Song Liu
2026-03-05 14:08                 ` Petr Mladek
2026-03-05 15:18                   ` Joe Lawrence
2026-03-05 15:20           ` Petr Mladek [this message]
2026-03-05 18:50           ` Josh Poimboeuf
2026-03-06  9:13             ` Petr Mladek
2026-03-05 19:33   ` Josh Poimboeuf

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=aamfUxhAM_EZ0MtR@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=song@kernel.org \
    /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.