* [PATCH 1/2] kunit: Provide a static key to check if KUnit is actively running tests @ 2022-10-21 7:28 David Gow 2022-10-21 7:28 ` [PATCH 2/2] kunit: Use the static key in kunit_fail_current_test() David Gow 0 siblings, 1 reply; 3+ messages in thread From: David Gow @ 2022-10-21 7:28 UTC (permalink / raw) To: Brendan Higgins, Daniel Latypov, Shuah Khan Cc: David Gow, kunit-dev, linux-kselftest, linux-kernel, Kees Cook KUnit does a few expensive things when enabled. This hasn't been a problem because KUnit was only enabled on test kernels, but with a few people enabling (but not _using_) KUnit on production systems, we need a runtime way of handling this. Provide a 'kunit_running' static key (defaulting to false), which allows us to hide any KUnit code behind a static branch. This should reduce the performance impact (on other code) of having KUnit enabled to a single NOP when no tests are running. Note that, while it looks unintuitive, tests always run entirely within __kunit_test_suites_init(), so it's safe to decrement the static key at the end of this function, rather than in __kunit_test_suites_exit(), which is only there to clean up results in debugfs. Signed-off-by: David Gow <davidgow@google.com> --- This should be a no-op (other than a possible performance improvement) functionality-wise, and lays the groundwork for a more optimised static stub implementation. --- include/kunit/test.h | 4 ++++ lib/kunit/test.c | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/include/kunit/test.h b/include/kunit/test.h index b1ab6b32216d..450a778a039e 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -16,6 +16,7 @@ #include <linux/container_of.h> #include <linux/err.h> #include <linux/init.h> +#include <linux/jump_label.h> #include <linux/kconfig.h> #include <linux/kref.h> #include <linux/list.h> @@ -27,6 +28,9 @@ #include <asm/rwonce.h> +/* Static key: true if any KUnit tests are currently running */ +extern struct static_key_false kunit_running; + struct kunit; /* Size of log associated with test. */ diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 90640a43cf62..314717b63080 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -20,6 +20,8 @@ #include "string-stream.h" #include "try-catch-impl.h" +DEFINE_STATIC_KEY_FALSE(kunit_running); + #if IS_BUILTIN(CONFIG_KUNIT) /* * Fail the current test and print an error message to the log. @@ -612,10 +614,14 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_ return 0; } + static_branch_inc(&kunit_running); + for (i = 0; i < num_suites; i++) { kunit_init_suite(suites[i]); kunit_run_tests(suites[i]); } + + static_branch_dec(&kunit_running); return 0; } EXPORT_SYMBOL_GPL(__kunit_test_suites_init); -- 2.38.0.135.g90850a2211-goog ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] kunit: Use the static key in kunit_fail_current_test() 2022-10-21 7:28 [PATCH 1/2] kunit: Provide a static key to check if KUnit is actively running tests David Gow @ 2022-10-21 7:28 ` David Gow 2022-10-21 14:16 ` Kees Cook 0 siblings, 1 reply; 3+ messages in thread From: David Gow @ 2022-10-21 7:28 UTC (permalink / raw) To: Brendan Higgins, Daniel Latypov, Shuah Khan Cc: David Gow, kunit-dev, linux-kselftest, linux-kernel, Kees Cook Speed up the case where kunit_fail_current_test() is called when no test is running. This should make it convenient for code to call this unconditionally in some error paths, without fear of causing a performance problem. If CONFIG_KUNIT=n, 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. kunit_fail_current_test() does not work if KUnit itself is built as a module, though this is a pre-existing limitation. 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. Signed-off-by: David Gow <davidgow@google.com> --- include/kunit/test-bug.h | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h index 5fc58081d511..ba9558a9f9c0 100644 --- a/include/kunit/test-bug.h +++ b/include/kunit/test-bug.h @@ -9,16 +9,29 @@ #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 */ + +/* Static key if KUnit is running any tests. */ +extern struct static_key_false kunit_running; + +#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 +/* 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.0.135.g90850a2211-goog ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] kunit: Use the static key in kunit_fail_current_test() 2022-10-21 7:28 ` [PATCH 2/2] kunit: Use the static key in kunit_fail_current_test() David Gow @ 2022-10-21 14:16 ` Kees Cook 0 siblings, 0 replies; 3+ messages in thread From: Kees Cook @ 2022-10-21 14:16 UTC (permalink / raw) To: David Gow, Brendan Higgins, Daniel Latypov, Shuah Khan Cc: kunit-dev, linux-kselftest, linux-kernel, Kees Cook On October 21, 2022 12:28:55 AM PDT, David Gow <davidgow@google.com> wrote: >Speed up the case where kunit_fail_current_test() is called when no test >is running. This should make it convenient for code to call this >unconditionally in some error paths, without fear of causing a >performance problem. A third patch showing these cases may help in understanding the utility. I get it, but I lack imagination on where/why they would be added. :) > >If CONFIG_KUNIT=n, 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. kunit_fail_current_test() does not work if KUnit >itself is built as a module, though this is a pre-existing limitation. > >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. > >Signed-off-by: David Gow <davidgow@google.com> >--- > include/kunit/test-bug.h | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > >diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h >index 5fc58081d511..ba9558a9f9c0 100644 >--- a/include/kunit/test-bug.h >+++ b/include/kunit/test-bug.h >@@ -9,16 +9,29 @@ > #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 */ >+ >+/* Static key if KUnit is running any tests. */ >+extern struct static_key_false kunit_running; >+ >+#define kunit_fail_current_test(fmt, ...) do { \ >+ if (static_branch_unlikely(&kunit_running)) { \ This trailing { should be dropped. >+ __kunit_fail_current_test(__FILE__, __LINE__, \ >+ fmt, ##__VA_ARGS__); \ Or a closing one added here. (The {}s are unbalanced, as 0day complained about.) >+ } while (0) >+ >+ > extern __printf(3, 4) void __kunit_fail_current_test(const char *file, int line, > const char *fmt, ...); > > #else > >+/* 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, ...) > { -- Kees Cook ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-21 14:16 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-21 7:28 [PATCH 1/2] kunit: Provide a static key to check if KUnit is actively running tests David Gow 2022-10-21 7:28 ` [PATCH 2/2] kunit: Use the static key in kunit_fail_current_test() David Gow 2022-10-21 14:16 ` Kees Cook
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).