On Mon, Jun 30, 2025 at 3:32 PM Tzung-Bi Shih tzungbi@kernel.org wrote:
On Tue, May 20, 2025 at 09:04:53AM -0700, Daniel Latypov wrote:
(snip)
We have these drawbacks with the current ftrace stubs:
- doesn't compile on all arches
- silently doesn't work on inlined functions <== scariest one to me
I see. I did some experiments and realized that kprobe stubs also share the same concern. Thus I'm wondering if there is a way that kprobe stub detects the redirection may fail, at least it can skip the test case (e.g. register_kprobe() returns -ENOENT when it can't find the symbol via kprobe_lookup_name()). But it seems no way if the target function is partially inlined.
Yeah, any such approach will need to be cautious of inlining, so they'd have to always be "for power users only: only use this if you understand the risk" Skipping when we can detect the user is obviously doing the wrong thing makes that a *bit* more palatable.
It's not like static stubs can detect at runtime if you've failed to add the necessary corresponding KUNIT_STATIC_STUB_REDIRECT() either, for example.
But I still personally lean slightly against adding either kprobe or ftrace stubs, see below.
You mention you don't like how static stubs requires modifying the code-under-test. Since it gets eliminated by the preprocessor unless you're compiling for KUnit, is the concern more so about how it conceptually feels wrong to do so? For the Android GKI kernel, they have (or had) KUnit enabled so there is potentially concern about real runtime cost there, not sure if you have something similar in mind.
Not exactly. Ideally, I think we shouldn't modify the CUT. I'm wondering if there is a way to not change the CUT but also break the external dependencies.
But stepping back, ftrace_stubs technically require modifying the code to make sure funcs are marked as `noinline`, which this patch series does not do.
...
They could be partially inlined even though they are exported symbols.
So to summarize, right now we're stuck with having to modify the code. (Unless someone can come up with something really clever, but not too clever)
To make it concrete, the current approach would look like:
int func(char* arg1, int arg2) { KUNIT_STATIC_STUB_REDIRECT(func, arg1, arg2); ... // unchanged }
vs an ftrace/kprobe approach that needs a conditional `noinline`
KUNIT_STUBBABLE int func(char* arg1, int arg2) { ... // unchanged }
The latter is definitely simpler and less burdensome. But I don't know if it's simpler enough to warrant a second implementation existing for me personally.
E.g. we already have some people who justifiably say it's too hard to figure out KUnit, so this is another decision point where a user might get stuck with "how should I know which one I should use?" and give up.
Compatibility tangent: A smaller annoyance is KUNIT_STATIC_STUB_REDIRECT and KUNIT_STUBBABLE are incompatible (and always will be?)
E.g. imagine a func has KUNIT_STUBBABLE on it, but a person authoring a new test wants needs to run without kprobe support, so they must add KUNIT_STATIC_STUB_REDIRECT. I can imagine an author deciding to make the func have *both* macros on it... That feels like a worst case outcome from the perspective of "we shouldn't modify the CUT just for the sake of tests" :\
To be clear, the right approach in this scenario is to 1) swap to KUNIT_STATIC_STUB_REDIRECT and update the previous tests to use static stubs, 2) then add your new tests. But I can imagine that won't always happen, esp. if it crosses maintainer boundaries.