| Message ID | 20221125084306.1063074-2-davidgow@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/3] kunit: Provide a static key to check if KUnit is actively running tests | expand |
On Fri, Nov 25, 2022 at 04:43:05PM +0800, David Gow wrote: > In order to detect if a KUnit test is running, and to access its > context, the 'kunit_test' member of the current task_struct is used. > Usually, this is accessed directly or via the kunit_fail_current_task() > function. > > In order to speed up the case where no test is running, add a wrapper, > kunit_get_current_test(), which uses the static key to fail early. > Equally, Speed up kunit_fail_current_test() by using the static key. > > This should make it convenient for code to call this > unconditionally in fakes or error paths, without worrying that this will > slow the code down significantly. > > If CONFIG_KUNIT=n (or m), this compiles away to nothing. If > CONFIG_KUNIT=y, it will compile down to a NOP (on most architectures) if > no KUnit test is currently running. > > Note that kunit_get_current_test() does not work if KUnit is built as a > module. This mirrors the existing restriction on kunit_fail_current_test(). > > Note that the definition of kunit_fail_current_test() still wraps an > empty, inline function if KUnit is not built-in. This is to ensure that > the printf format string __attribute__ will still work. > > Also update the documentation to suggest users use the new > kunit_get_current_test() function, update the example, and to describe > the behaviour when KUnit is disabled better. > > Cc: Jonathan Corbet <corbet@lwn.net> > Cc: Sadiya Kazi <sadiyakazi@google.com> > Signed-off-by: David Gow <davidgow@google.com> Reviewed-by: Kees Cook <keescook@chromium.org>
On Fri, Nov 25, 2022 at 2:13 PM 'David Gow' via KUnit Development <kunit-dev@googlegroups.com> wrote: > > In order to detect if a KUnit test is running, and to access its > context, the 'kunit_test' member of the current task_struct is used. > Usually, this is accessed directly or via the kunit_fail_current_task() > function. > > In order to speed up the case where no test is running, add a wrapper, > kunit_get_current_test(), which uses the static key to fail early. > Equally, Speed up kunit_fail_current_test() by using the static key. > > This should make it convenient for code to call this > unconditionally in fakes or error paths, without worrying that this will > slow the code down significantly. > > If CONFIG_KUNIT=n (or m), this compiles away to nothing. If > CONFIG_KUNIT=y, it will compile down to a NOP (on most architectures) if > no KUnit test is currently running. > > Note that kunit_get_current_test() does not work if KUnit is built as a > module. This mirrors the existing restriction on kunit_fail_current_test(). > > Note that the definition of kunit_fail_current_test() still wraps an > empty, inline function if KUnit is not built-in. This is to ensure that > the printf format string __attribute__ will still work. > > Also update the documentation to suggest users use the new > kunit_get_current_test() function, update the example, and to describe > the behaviour when KUnit is disabled better. > > Cc: Jonathan Corbet <corbet@lwn.net> > Cc: Sadiya Kazi <sadiyakazi@google.com> > Signed-off-by: David Gow <davidgow@google.com> > Reviewed-by: Daniel Latypov <dlatypov@google.com> > --- Thank you, David. This looks great to me. Reviewed-by: Sadiya Kazi <sadiyakazi@google.com> Best Regards, Sadiya Kazi > > As-is, the only code which will be directly affected by this (via the > kunit_fail_current_test() change) will be UBSAN's KUnit integration. > > Patches to port other tests to use kunit_get_current_test() will be sent > separately (other than the SLUB one in patch 3/3). KASAN in particular > are reworking their KUnit tests and integration, so we'll use this in a > follow up to avoid introducing a conflict. > > Changes since v3: > https://lore.kernel.org/linux-kselftest/20221119081252.3864249-2-davidgow@google.com/ > - Use DECLARE_STATIC_KEY_FALSE() -- Thanks Daniel. > - Some documentation rewording to make the behaviour a bit clearer. > - Thanks Daniel and Sadiya > > Changes since v2: > https://lore.kernel.org/all/20221025071907.1251820-2-davidgow@google.com/ > - Only add kunit_get_current_test() when KUnit is built-in, as the > static key isn't available otherwise. > - I'm going to try to put together some patches to make things like > this available when CONFIG_KUNIT=m in the future. > - Also update the documentation to note this. > > Changes since v1: > https://lore.kernel.org/linux-kselftest/20221021072854.333010-2-davidgow@google.com/ > - Fix a missing '}' which broke everything. Thanks Kees, kernel test > robot. > - Add the new kunit_get_current_test() function, as most of the cases > where we retrieve the current test (even to fail it) were accessing > current->kunit_test directly, not using kunit_fail_current_test(). > - Add some documentation comments. > - Update the documentation in usage.rst. > - The version in tips.rst was not updated, and will be removed: > https://lore.kernel.org/linux-kselftest/20221025055844.1231592-1-davidgow@google.com/ > > --- > Documentation/dev-tools/kunit/usage.rst | 30 +++++++++----- > include/kunit/test-bug.h | 53 +++++++++++++++++++++++-- > 2 files changed, 71 insertions(+), 12 deletions(-) > > diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst > index 22416ebb94ab..48f8196d5aad 100644 > --- a/Documentation/dev-tools/kunit/usage.rst > +++ b/Documentation/dev-tools/kunit/usage.rst > @@ -641,17 +641,23 @@ as shown in next section: *Accessing The Current Test*. > Accessing The Current Test > -------------------------- > > -In some cases, we need to call test-only code from outside the test file. > -For example, see example in section *Injecting Test-Only Code* or if > -we are providing a fake implementation of an ops struct. Using > -``kunit_test`` field in ``task_struct``, we can access it via > -``current->kunit_test``. > +In some cases, we need to call test-only code from outside the test file. This > +is helpful, for example, when providing a fake implementation of a function, or > +to fail any current test from within an error handler. > +We can do this via the ``kunit_test`` field in ``task_struct``, which we can > +access using the ``kunit_get_current_test()`` function in ``kunit/test-bug.h``. > > -The example below includes how to implement "mocking": > +``kunit_get_current_test()`` is safe to call even if KUnit is not enabled. If > +KUnit is not enabled, was built as a module (``CONFIG_KUNIT=m``), or no test is > +running in the current task, it will return ``NULL``. This compiles down to > +either a no-op or a static key check, so will have a negligible performance > +impact when no test is running. > + > +The example below uses this to implement a "mock" implementation of a function, ``foo``: > > .. code-block:: c > > - #include <linux/sched.h> /* for current */ > + #include <kunit/test-bug.h> /* for kunit_get_current_test */ > > struct test_data { > int foo_result; > @@ -660,7 +666,7 @@ The example below includes how to implement "mocking": > > static int fake_foo(int arg) > { > - struct kunit *test = current->kunit_test; > + struct kunit *test = kunit_get_current_test(); > struct test_data *test_data = test->priv; > > KUNIT_EXPECT_EQ(test, test_data->want_foo_called_with, arg); > @@ -691,7 +697,7 @@ Each test can have multiple resources which have string names providing the same > flexibility as a ``priv`` member, but also, for example, allowing helper > functions to create resources without conflicting with each other. It is also > possible to define a clean up function for each resource, making it easy to > -avoid resource leaks. For more information, see Documentation/dev-tools/kunit/api/test.rst. > +avoid resource leaks. For more information, see Documentation/dev-tools/kunit/api/resource.rst. > > Failing The Current Test > ------------------------ > @@ -719,3 +725,9 @@ structures as shown below: > static void my_debug_function(void) { } > #endif > > +``kunit_fail_current_test()`` is safe to call even if KUnit is not enabled. If > +KUnit is not enabled, was built as a module (``CONFIG_KUNIT=m``), or no test is > +running in the current task, it will do nothing. This compiles down to either a > +no-op or a static key check, so will have a negligible performance impact when > +no test is running. > + > diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h > index 5fc58081d511..c1b2e14eab64 100644 > --- a/include/kunit/test-bug.h > +++ b/include/kunit/test-bug.h > @@ -9,16 +9,63 @@ > #ifndef _KUNIT_TEST_BUG_H > #define _KUNIT_TEST_BUG_H > > -#define kunit_fail_current_test(fmt, ...) \ > - __kunit_fail_current_test(__FILE__, __LINE__, fmt, ##__VA_ARGS__) > - > #if IS_BUILTIN(CONFIG_KUNIT) > > +#include <linux/jump_label.h> /* For static branch */ > +#include <linux/sched.h> > + > +/* Static key if KUnit is running any tests. */ > +DECLARE_STATIC_KEY_FALSE(kunit_running); > + > +/** > + * kunit_get_current_test() - Return a pointer to the currently running > + * KUnit test. > + * > + * If a KUnit test is running in the current task, returns a pointer to its > + * associated struct kunit. This pointer can then be passed to any KUnit > + * function or assertion. If no test is running (or a test is running in a > + * different task), returns NULL. > + * > + * This function is safe to call even when KUnit is disabled. If CONFIG_KUNIT > + * is not enabled, it will compile down to nothing and will return quickly no > + * test is running. > + */ > +static inline struct kunit *kunit_get_current_test(void) > +{ > + if (!static_branch_unlikely(&kunit_running)) > + return NULL; > + > + return current->kunit_test; > +} > + > + > +/** > + * kunit_fail_current_test() - If a KUnit test is running, fail it. > + * > + * If a KUnit test is running in the current task, mark that test as failed. > + * > + * This macro will only work if KUnit is built-in (though the tests > + * themselves can be modules). Otherwise, it compiles down to nothing. > + */ > +#define kunit_fail_current_test(fmt, ...) do { \ > + if (static_branch_unlikely(&kunit_running)) { \ > + __kunit_fail_current_test(__FILE__, __LINE__, \ > + fmt, ##__VA_ARGS__); \ > + } \ > + } while (0) > + > + > extern __printf(3, 4) void __kunit_fail_current_test(const char *file, int line, > const char *fmt, ...); > > #else > > +static inline struct kunit *kunit_get_current_test(void) { return NULL; } > + > +/* We define this with an empty helper function so format string warnings work */ > +#define kunit_fail_current_test(fmt, ...) \ > + __kunit_fail_current_test(__FILE__, __LINE__, fmt, ##__VA_ARGS__) > + > static inline __printf(3, 4) void __kunit_fail_current_test(const char *file, int line, > const char *fmt, ...) > { > -- > 2.38.1.584.g0f3c55d4c2-goog > > -- > You received this message because you are subscribed to the Google Groups "KUnit Development" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20221125084306.1063074-2-davidgow%40google.com.
diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst index 22416ebb94ab..48f8196d5aad 100644 --- a/Documentation/dev-tools/kunit/usage.rst +++ b/Documentation/dev-tools/kunit/usage.rst @@ -641,17 +641,23 @@ as shown in next section: *Accessing The Current Test*. Accessing The Current Test -------------------------- -In some cases, we need to call test-only code from outside the test file. -For example, see example in section *Injecting Test-Only Code* or if -we are providing a fake implementation of an ops struct. Using -``kunit_test`` field in ``task_struct``, we can access it via -``current->kunit_test``. +In some cases, we need to call test-only code from outside the test file. This +is helpful, for example, when providing a fake implementation of a function, or +to fail any current test from within an error handler. +We can do this via the ``kunit_test`` field in ``task_struct``, which we can +access using the ``kunit_get_current_test()`` function in ``kunit/test-bug.h``. -The example below includes how to implement "mocking": +``kunit_get_current_test()`` is safe to call even if KUnit is not enabled. If +KUnit is not enabled, was built as a module (``CONFIG_KUNIT=m``), or no test is +running in the current task, it will return ``NULL``. This compiles down to +either a no-op or a static key check, so will have a negligible performance +impact when no test is running. + +The example below uses this to implement a "mock" implementation of a function, ``foo``: .. code-block:: c - #include <linux/sched.h> /* for current */ + #include <kunit/test-bug.h> /* for kunit_get_current_test */ struct test_data { int foo_result; @@ -660,7 +666,7 @@ The example below includes how to implement "mocking": static int fake_foo(int arg) { - struct kunit *test = current->kunit_test; + struct kunit *test = kunit_get_current_test(); struct test_data *test_data = test->priv; KUNIT_EXPECT_EQ(test, test_data->want_foo_called_with, arg); @@ -691,7 +697,7 @@ Each test can have multiple resources which have string names providing the same flexibility as a ``priv`` member, but also, for example, allowing helper functions to create resources without conflicting with each other. It is also possible to define a clean up function for each resource, making it easy to -avoid resource leaks. For more information, see Documentation/dev-tools/kunit/api/test.rst. +avoid resource leaks. For more information, see Documentation/dev-tools/kunit/api/resource.rst. Failing The Current Test ------------------------ @@ -719,3 +725,9 @@ structures as shown below: static void my_debug_function(void) { } #endif +``kunit_fail_current_test()`` is safe to call even if KUnit is not enabled. If +KUnit is not enabled, was built as a module (``CONFIG_KUNIT=m``), or no test is +running in the current task, it will do nothing. This compiles down to either a +no-op or a static key check, so will have a negligible performance impact when +no test is running. + diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h index 5fc58081d511..c1b2e14eab64 100644 --- a/include/kunit/test-bug.h +++ b/include/kunit/test-bug.h @@ -9,16 +9,63 @@ #ifndef _KUNIT_TEST_BUG_H #define _KUNIT_TEST_BUG_H -#define kunit_fail_current_test(fmt, ...) \ - __kunit_fail_current_test(__FILE__, __LINE__, fmt, ##__VA_ARGS__) - #if IS_BUILTIN(CONFIG_KUNIT) +#include <linux/jump_label.h> /* For static branch */ +#include <linux/sched.h> + +/* Static key if KUnit is running any tests. */ +DECLARE_STATIC_KEY_FALSE(kunit_running); + +/** + * kunit_get_current_test() - Return a pointer to the currently running + * KUnit test. + * + * If a KUnit test is running in the current task, returns a pointer to its + * associated struct kunit. This pointer can then be passed to any KUnit + * function or assertion. If no test is running (or a test is running in a + * different task), returns NULL. + * + * This function is safe to call even when KUnit is disabled. If CONFIG_KUNIT + * is not enabled, it will compile down to nothing and will return quickly no + * test is running. + */ +static inline struct kunit *kunit_get_current_test(void) +{ + if (!static_branch_unlikely(&kunit_running)) + return NULL; + + return current->kunit_test; +} + + +/** + * kunit_fail_current_test() - If a KUnit test is running, fail it. + * + * If a KUnit test is running in the current task, mark that test as failed. + * + * This macro will only work if KUnit is built-in (though the tests + * themselves can be modules). Otherwise, it compiles down to nothing. + */ +#define kunit_fail_current_test(fmt, ...) do { \ + if (static_branch_unlikely(&kunit_running)) { \ + __kunit_fail_current_test(__FILE__, __LINE__, \ + fmt, ##__VA_ARGS__); \ + } \ + } while (0) + + extern __printf(3, 4) void __kunit_fail_current_test(const char *file, int line, const char *fmt, ...); #else +static inline struct kunit *kunit_get_current_test(void) { return NULL; } + +/* We define this with an empty helper function so format string warnings work */ +#define kunit_fail_current_test(fmt, ...) \ + __kunit_fail_current_test(__FILE__, __LINE__, fmt, ##__VA_ARGS__) + static inline __printf(3, 4) void __kunit_fail_current_test(const char *file, int line, const char *fmt, ...) {