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

["Pablo Hugen" <phugen@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]

> We sort of test the same in test-callbacks.sh. Just using different
> means. I think I would not mind having this as well.

Ok. The original idea Joe suggested was to check the function output
of module targets directly. From what I can tell, test-callbacks.sh
covers the callbacks and test-ftrace.sh checks that ftrace
can still trace a function after it's been livepatched, neither
actually checks that a replacement function in a module target runs.
I can be wrong though, still getting familiar with the livepatch tests.

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

Fair point. Altough biased I think it is nice to have this explict sanity check.

> 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

Yeah I think you're right, not strictly needed, just wanted to be sure.
After some experience rebasing kpatch integration tests I've been bitten
enough times to be paranoid about that :)

But since I will work on a follow-up for the other suggestions, I think
I'll drop it there.

Anyways, thanks for the reviews!
Pablo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]