Re: [PATCH] selftests/livepatch: add test for module function patching

[Joe Lawrence <joe.lawrence@redhat.com>]

On Tue, Mar 24, 2026 at 03:22:27PM +0100, Miroslav Benes wrote:
> On Fri, 20 Mar 2026 17:11:17 -0300, Pablo Hugen <phugen@redhat.com> wrote:
> > Add a target module and livepatch pair that verify module function
> > patching via a proc entry. Two test cases cover both the
> > klp_enable_patch path (target loaded before livepatch) and the
> > klp_module_coming path (livepatch loaded before target).
> 
> We sort of test the same in test-callbacks.sh. Just using different
> means. I think I would not mind having this as well.
> 
> Petr, Joe, what do you think?
> 

I was *just* in the middle of replying to the patch when yours came in,
so I'll just move over here.  I had noticed the same thing re:
test-callbacks.sh despite originally suggested writing this test to
Pablo (and forgot about the callbacks test module).  With that, I agree
that it's a nice basic sanity check that's obvious about what it's
testing.

> >
> >
> > diff --git a/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
> > new file mode 100644
> > index 000000000000..9643984d2402
> > --- /dev/null
> > +++ b/tools/testing/selftests/livepatch/test_modules/test_klp_mod_target.c
> > @@ -0,0 +1,39 @@
> > [ ... skip 11 lines ... ]
> > +
> > +static noinline int test_klp_mod_target_show(struct seq_file *m, void *v)
> > +{
> > +	seq_printf(m, "%s: %s\n", THIS_MODULE->name, "original output");
> > +	return 0;
> > +}
> 
> A nit but is 'noinline' keyword needed here? proc_create_single() below
> takes a function pointer so hopefully test_klp_mod_target_show() stays
> even without it?
> 

No strong preference either way.  I won't fault a livepatch developer
for being paranoid w/respect to the compiler :D

Acked-by: Joe Lawrence <joe.lawrence@redhat.com>
--
Joe