On Tue, Aug 27, 2024 at 12:20:13AM GMT, Michal Wajdeczko wrote:
Currently, the 'static stub' API only allows function redirection for calls made from the kthread of the current test, which prevents the use of this functionality when the tested code is also used by other threads, outside of the KUnit test, like from the workqueue. Add another set of macros to allow redirection to the replacement functions, which, unlike the KUNIT_STATIC_STUB_REDIRECT, will affect all calls done during the test execution. These new stubs, named 'global', must be declared using dedicated KUNIT_DECLARE_GLOBAL_STUB() macro and then can be placed either as global static variables or as part of the other structures. To properly maintain stubs lifecycle, they can be activated only from the main KUnit context. Some precaution is taken to avoid changing or deactivating replacement functions if one is still used by other thread. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> --- Cc: Rae Moar <rmoar@xxxxxxxxxx> Cc: David Gow <davidgow@xxxxxxxxxx> Cc: Daniel Latypov <dlatypov@xxxxxxxxxx> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> --- v2: s/FIXED_STUB/GLOBAL_STUB (David, Lucas) make it little more thread safe (Rae, David) wait until stub call finishes before test end (David) wait until stub call finishes before changing stub (David) allow stub deactivation (Rae) prefer kunit log (David) --- include/kunit/static_stub.h | 158 ++++++++++++++++++++++++++++++++++++ lib/kunit/static_stub.c | 49 +++++++++++ 2 files changed, 207 insertions(+) diff --git a/include/kunit/static_stub.h b/include/kunit/static_stub.h index bf940322dfc0..42a70dcefb56 100644 --- a/include/kunit/static_stub.h +++ b/include/kunit/static_stub.h @@ -12,12 +12,15 @@ /* If CONFIG_KUNIT is not enabled, these stubs quietly disappear. */ #define KUNIT_STATIC_STUB_REDIRECT(real_fn_name, args...) do {} while (0) +#define KUNIT_GLOBAL_STUB_REDIRECT(stub_name, args...) do {} while (0) +#define KUNIT_DECLARE_GLOBAL_STUB(stub_name, stub_type) #else #include <kunit/test.h> #include <kunit/test-bug.h> +#include <linux/cleanup.h> /* for CLASS */ #include <linux/compiler.h> /* for {un,}likely() */ #include <linux/sched.h> /* for task_struct */ @@ -109,5 +112,160 @@ void __kunit_activate_static_stub(struct kunit *test, */ void kunit_deactivate_static_stub(struct kunit *test, void *real_fn_addr); +/** + * struct kunit_global_stub - Represents a context of global function stub. + * @replacement: The address of replacement function. + * @owner: The KUnit test that owns the stub, valid only when @busy > 0. + * @busy: The stub busyness counter incremented on entry to the replacement + * function, decremented on exit, used to signal if the stub is idle. + * @idle: The completion state to indicate when the stub is idle again. + * + * This structure is for KUnit internal use only. + * See KUNIT_DECLARE_GLOBAL_STUB(). + */ +struct kunit_global_stub { + void *replacement; + struct kunit *owner; + atomic_t busy; + struct completion idle; +}; + +/** + * KUNIT_DECLARE_GLOBAL_STUB() - Declare a global function stub. + * @stub_name: The name of the stub, must be a valid identifier + * @stub_type: The type of the function that this stub will replace + * + * This macro will declare new identifier of an anonymous type that will + * represent global stub function that could be used by KUnit. It can be stored + * outside of the KUnit code. If the CONFIG_KUNIT is not enabled this will + * be evaluated to an empty statement. + * + * The anonymous type introduced by this macro is mostly a wrapper to generic + * struct kunit_global_stub but with additional dummy member, that is never + * used directly, but is needed to maintain the type of the stub function. + */ +#define KUNIT_DECLARE_GLOBAL_STUB(stub_name, stub_type) \ +union { \ + struct kunit_global_stub base; \ + typeof(stub_type) dummy; \ +} stub_name + +/* Internal struct to define guard class */ +struct kunit_global_stub_guard { + struct kunit_global_stub *stub; + void *active_replacement; +}; + +/* Internal class used to guard stub calls */ +DEFINE_CLASS(kunit_global_stub_guard, + struct kunit_global_stub_guard, + ({ + struct kunit_global_stub *stub = _T.stub; + bool active = !!_T.active_replacement;
I'd call this `bool active_replacement` as it's not the same thing as the active below.
+ + if (active && !atomic_dec_return(&stub->busy)) + complete_all(&stub->idle); + }), + ({ + class_kunit_global_stub_guard_t guard; + bool active = !!atomic_inc_not_zero(&stub->busy); + + guard.stub = stub; + guard.active_replacement = active ? READ_ONCE(stub->replacement) : NULL; + + guard; + }), + struct kunit_global_stub *stub) + +/** + * KUNIT_GLOBAL_STUB_REDIRECT() - Call a fixed function stub if activated. + * @stub: The function stub declared using KUNIT_DECLARE_GLOBAL_STUB() + * @args: All of the arguments passed to this stub + * + * This is a function prologue which is used to allow calls to the current + * function to be redirected if a KUnit is running. If the KUnit is not + * running or stub is not yet activated the function will continue execution + * as normal. + * + * The function stub must be declared with KUNIT_DECLARE_GLOBAL_STUB() that is + * stored in a place that is accessible from both the test code, which will + * activate this stub using kunit_activate_global_stub(), and from the function, + * where we will do this redirection using KUNIT_GLOBAL_STUB_REDIRECT(). + * + * Unlike the KUNIT_STATIC_STUB_REDIRECT(), this redirection will work + * even if the caller is not in a KUnit context (like a worker thread). + * + * Example: + * + * .. code-block:: c + * + * KUNIT_DECLARE_GLOBAL_STUB(func_stub, int (*)(int n)); + * + * int real_func(int n) + * { + * KUNIT_GLOBAL_STUB_REDIRECT(func_stub, n); + * return n + 1; + * } + * + * int replacement_func(int n) + * { + * return n + 100; + * } + * + * void example_test(struct kunit *test) + * { + * KUNIT_EXPECT_EQ(test, real_func(1), 2); + * kunit_activate_global_stub(test, func_stub, replacement_func); + * KUNIT_EXPECT_EQ(test, real_func(1), 101); + * } + */ +#define KUNIT_GLOBAL_STUB_REDIRECT(stub, args...) do { \ + if (kunit_is_running()) { \ + typeof(stub) *__stub = &(stub); \ + CLASS(kunit_global_stub_guard, guard)(&__stub->base); \ + typeof(__stub->dummy) replacement = guard.active_replacement; \ + if (unlikely(replacement)) { \ + kunit_info(__stub->base.owner, "%s: redirecting to %ps\n", \ + __func__, replacement); \ + return replacement(args); \ + } \ + } \ +} while (0) + +void __kunit_activate_global_stub(struct kunit *test, struct kunit_global_stub *stub, + void *replacement_addr); + +/** + * kunit_activate_global_stub() - Setup a fixed function stub.
s/fixed/global/ here and every where else below
+ * @test: Test case that wants to activate a fixed function stub + * @stub: The location of the function stub pointer + * @replacement: The replacement function + * + * This helper setups a function stub with the replacement function. + * It will also automatically deactivate the stub at the test end. + * + * The redirection can be disabled with kunit_deactivate_global_stub(). + * The stub must be declared using KUNIT_DECLARE_GLOBAL_STUB(). + */ +#define kunit_activate_global_stub(test, stub, replacement) do { \ + typeof(stub) *__stub = &(stub); \ + typecheck_fn(typeof(__stub->dummy), (replacement)); \ + __kunit_activate_global_stub((test), &__stub->base, (replacement)); \ +} while (0) + +void __kunit_deactivate_global_stub(struct kunit *test, struct kunit_global_stub *stub); + +/** + * kunit_deactivate_global_stub() - Disable a fixed function stub. + * @test: Test case that wants to deactivate a fixed function stub + * @stub: The location of the function stub pointer + * + * The stub must be declared using KUNIT_DECLARE_GLOBAL_STUB(). + */ +#define kunit_deactivate_global_stub(test, stub) do { \ + typeof(stub) *__stub = &(stub); \ + __kunit_deactivate_global_stub((test), &__stub->base); \ +} while (0) + #endif #endif diff --git a/lib/kunit/static_stub.c b/lib/kunit/static_stub.c index 92b2cccd5e76..799a7271dc5b 100644 --- a/lib/kunit/static_stub.c +++ b/lib/kunit/static_stub.c @@ -121,3 +121,52 @@ void __kunit_activate_static_stub(struct kunit *test, } } EXPORT_SYMBOL_GPL(__kunit_activate_static_stub); + +static void sanitize_global_stub(void *data) +{ + struct kunit *test = kunit_get_current_test(); + struct kunit_global_stub *stub = data; + + KUNIT_EXPECT_NE(test, 0, atomic_read(&stub->busy));
shouldn't sanitize_ be unconditional and do nothing in this case?
+ KUNIT_EXPECT_PTR_EQ(test, test, READ_ONCE(stub->owner)); + + reinit_completion(&stub->idle); + if (!atomic_dec_and_test(&stub->busy)) { + kunit_info(test, "waiting for %ps\n", stub->replacement); + KUNIT_EXPECT_EQ(test, 0, wait_for_completion_interruptible(&stub->idle));
what's preventing stub->busy going to 1 again after this? Lucas De Marchi
+ } + + WRITE_ONCE(stub->owner, NULL); + WRITE_ONCE(stub->replacement, NULL); +} + +/* + * Helper function for kunit_activate_global_stub(). The macro does + * typechecking, so use it instead. + */ +void __kunit_activate_global_stub(struct kunit *test, + struct kunit_global_stub *stub, + void *replacement_addr) +{ + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stub); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, replacement_addr); + if (atomic_read(&stub->busy)) + kunit_release_action(test, sanitize_global_stub, stub); + else + init_completion(&stub->idle); + WRITE_ONCE(stub->owner, test); + WRITE_ONCE(stub->replacement, replacement_addr); + KUNIT_ASSERT_EQ(test, 1, atomic_inc_return(&stub->busy)); + KUNIT_ASSERT_EQ(test, 0, kunit_add_action_or_reset(test, sanitize_global_stub, stub)); +} +EXPORT_SYMBOL_GPL(__kunit_activate_global_stub); + +/* + * Helper function for kunit_deactivate_global_stub(). Use it instead. + */ +void __kunit_deactivate_global_stub(struct kunit *test, struct kunit_global_stub *stub) +{ + if (atomic_read(&stub->busy)) + kunit_release_action(test, sanitize_global_stub, stub); +} +EXPORT_SYMBOL_GPL(__kunit_deactivate_global_stub); -- 2.43.0