Hi Miroslav,

On Fri, Feb 27, 2026 at 2:05 AM Miroslav Benes <[email protected]> wrote:
>
> Hi,
>
> I have a couple of questions before reviewing the code itself. See below.
> I removed the code completely as it seems better to have it compact. Sorry
> if it is too confusing in the end and I apologize for being late to the
> party. We can always merge the first 7 patches when they are settled and
> keep this one separate.

Yes, I was also thinking this patch will need more discussions than the
rest of the set.

> On Wed, 25 Feb 2026, Song Liu wrote:
>
> > Add selftests for the klp-build toolchain. This includes kernel side test
> > code and .patch files. The tests cover both livepatch to vmlinux and kernel
> > modules.
> >
> > Check tools/testing/selftests/livepatch/test_patches/README for
> > instructions to run these tests.
> >
> > Signed-off-by: Song Liu <[email protected]>
> >
> > ---
> >
> > AI was used to wrote the test code and .patch files in this.
>
> This should go to the changelog directly. See new
> Documentation/process/generated-content.rst.

Thanks for pointing this out. I didn't know we had this guidance.

>
> > ---
> >  kernel/livepatch/Kconfig                      |  20 +++
> >  kernel/livepatch/Makefile                     |   2 +
> >  kernel/livepatch/tests/Makefile               |   6 +
> >  kernel/livepatch/tests/klp_test_module.c      | 111 ++++++++++++++
> >  kernel/livepatch/tests/klp_test_module.h      |   8 +
> >  kernel/livepatch/tests/klp_test_vmlinux.c     | 138 ++++++++++++++++++
> >  kernel/livepatch/tests/klp_test_vmlinux.h     |  16 ++
> >  kernel/livepatch/tests/klp_test_vmlinux_aux.c |  59 ++++++++
> >  .../selftests/livepatch/test_patches/README   |  15 ++
> >  .../test_patches/klp_test_hash_change.patch   |  30 ++++
> >  .../test_patches/klp_test_module.patch        |  18 +++
> >  .../klp_test_nonstatic_to_static.patch        |  40 +++++
> >  .../klp_test_static_to_nonstatic.patch        |  39 +++++
> >  .../test_patches/klp_test_vmlinux.patch       |  18 +++
> >  14 files changed, 520 insertions(+)
> >  create mode 100644 kernel/livepatch/tests/Makefile
> >  create mode 100644 kernel/livepatch/tests/klp_test_module.c
> >  create mode 100644 kernel/livepatch/tests/klp_test_module.h
> >  create mode 100644 kernel/livepatch/tests/klp_test_vmlinux.c
> >  create mode 100644 kernel/livepatch/tests/klp_test_vmlinux.h
> >  create mode 100644 kernel/livepatch/tests/klp_test_vmlinux_aux.c
> >  create mode 100644 tools/testing/selftests/livepatch/test_patches/README
> >  create mode 100644 
> > tools/testing/selftests/livepatch/test_patches/klp_test_hash_change.patch
> >  create mode 100644 
> > tools/testing/selftests/livepatch/test_patches/klp_test_module.patch
> >  create mode 100644 
> > tools/testing/selftests/livepatch/test_patches/klp_test_nonstatic_to_static.patch
> >  create mode 100644 
> > tools/testing/selftests/livepatch/test_patches/klp_test_static_to_nonstatic.patch
> >  create mode 100644 
> > tools/testing/selftests/livepatch/test_patches/klp_test_vmlinux.patch
>
> 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.

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

CC Joe to chime in here.

>
> The patch mentiones kpatch in some places. Could you replace it, please?

I was using kpatch for testing. I can replace it with insmod.

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

Thanks,
Song

Reply via email to