From: Brendan Higgins <brendanhiggins@google.com>
To: Alan Maguire <alan.maguire@oracle.com>
Cc: linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
kunit-dev@googlegroups.com, keescook@chromium.org,
yzaikin@google.com, akpm@linux-foundation.org,
yamada.masahiro@socionext.com, catalin.marinas@arm.com,
joe.lawrence@redhat.com, penguin-kernel@i-love.sakura.ne.jp,
schowdary@nvidia.com, urezki@gmail.com,
andriy.shevchenko@linux.intel.com, corbet@lwn.net,
linux-doc@vger.kernel.org, Knut Omang <knut.omang@oracle.com>
Subject: Re: [PATCH v3 linux-kselftest-test 4/6] kunit: allow kunit tests to be loaded as a module
Date: Thu, 7 Nov 2019 17:40:47 -0800 [thread overview]
Message-ID: <20191108014047.GA216971@google.com> (raw)
In-Reply-To: <1571335639-21675-5-git-send-email-alan.maguire@oracle.com>
On Thu, Oct 17, 2019 at 07:07:17PM +0100, Alan Maguire wrote:
> as tests are added to kunit, it will become less feasible to execute
^
nit: please capitalize "as".
> all built tests together. By supporting modular tests we provide
> a simple way to do selective execution on a running system; specifying
>
> CONFIG_KUNIT=y
> CONFIG_KUNIT_EXAMPLE_TEST=m
>
> ...means we can simply "insmod example-test.ko" to run the tests.
>
> To achieve this we need to
>
> o export the required symbols in kunit
> o for non-exported symbols, we need to utilize kunit_find_symbol;
> the simplest way is for the test suite init to call
> KUNIT_INIT_[FN|VAR]_SYMBOL() for each non-exported symbol.
> o support a new way of declaring test suites. Because a module cannot
> do multiple late_initcall()s, we provide a kunit_test_suites() macro
> to declare multiple suites within the same module at once.
>
> When compiled as a module, use of KUNIT_INIT_[FN|VAR]_symbol() will
> retrieve the symbol address via kunit_find_symbol() and assign a local
> variable with the same symbol name appropriately. When compiled builtin,
> these definitions are used to verify that the types we specify match
> the type of the symbol we are looking for. Compiler errors will be
> generated if not.
>
> One wrinkle here is that we cannot use the same names for local function
> pointer definitions; the reason for this is we have likely #included
> a definition for the function in question already, so an attempt to
> redefine it as a function pointer variable fails. As a result the
> KUNIT_INIT_FN_SYMBOL() macro requires a name for a local symbol we
> have defined as a function pointer (with a signature matching the
> desired function).
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> Signed-off-by: Knut Omang <knut.omang@oracle.com>
> ---
> include/kunit/test.h | 115 +++++++++++++++++++++++++++++++++++++----
> kernel/sysctl-test.c | 4 +-
> lib/Kconfig.debug | 2 +-
> lib/kunit/Kconfig | 4 +-
> lib/kunit/assert.c | 8 +++
> lib/kunit/example-test.c | 4 +-
> lib/kunit/string-stream-test.c | 44 ++++++++++++----
> lib/kunit/test-test.c | 32 ++++++++----
> lib/kunit/test.c | 9 ++++
> lib/kunit/try-catch.c | 2 +
> 10 files changed, 187 insertions(+), 37 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index c645d18..9a3835a 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -12,6 +12,7 @@
> #include <kunit/assert.h>
> #include <kunit/try-catch.h>
> #include <linux/kernel.h>
> +#include <linux/module.h>
> #include <linux/slab.h>
> #include <linux/types.h>
>
> @@ -78,6 +79,86 @@ struct kunit_resource {
> struct list_head node;
> };
>
> +/**
> + * KUNIT_VAR_SYMBOL - A helper for defining non-exported variable symbols
> + *
> + * @name: name of the symbol.
> + * @type: type of symbol.
> + *
> + * In the module case, we define the pointer to the symbol type where
> + * we will store the symbol address; KUNIT_INIT_VAR_SYMBOL() will assign
> + * the symbol name to the dereferenced kunit_<symbol_name>. Note that
> + * in the builtin case we still define kunit_<symbol_name>; the reason
> + * for this is it allows us to verify that the type value is correct
> + * in the builtin case and has not fallen out-of-sync with its original
Very clever! Can you maybe elaborate on how? I didn't understand until I
looked at the initialization code. It would probably be sufficient to
just tell the reader to look at the initialization code.
> + * definition.
> + */
> +#ifdef MODULE
> +#define KUNIT_VAR_SYMBOL(symbol, type) \
> + type * kunit_##symbol; \
> + type symbol
> +#else
> +#define KUNIT_VAR_SYMBOL(symbol, type) \
> + type * kunit_##symbol
> +#endif
> +
> +/**
> + * KUNIT_INIT_VAR_SYMBOL - A helper for initializing non-exported variable
> + * symbols
> + * @test: optional pointer to test context
> + * @name: name of symbol
> + *
> + * In the module case, initialization consists of using kunit_find_symbol()
> + * to find the address of the symbol, and if found, we set the variable
> + * to the dereferenced address value. As mentioned above, in the builtin
> + * case we simply assing kunit_<symbol_name> to &<symbol_name> ; this will
> + * generate a compilation warning if the type we specified in KUNIT_VAR_SYMBOL
> + * and the type of the symbol itself do not match.
> + */
> +#ifdef MODULE
> +#define KUNIT_INIT_VAR_SYMBOL(test, symbol) \
> + do { \
> + if (!(kunit_##symbol)) { \
> + kunit_##symbol = kunit_find_symbol(#symbol); \
> + if (!IS_ERR((kunit_##symbol))) \
> + symbol = *(kunit_##symbol); \
> + } \
> + if (test) \
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, \
> + kunit_##symbol); \
> + } while (0)
> +#else
> +#define KUNIT_INIT_VAR_SYMBOL(test, symbol) \
> + kunit_##symbol = &(symbol)
> +#endif
> +
> +/**
> + * KUNIT_INIT_FN_SYMBOL - A helper for initializing non-exported function
> + * symbols
> + * @test: optional pointer to test context
> + * @symbol: name of symbol
> + * @name: local name of function used to store function pointer to symbol
> + *
> + * In the module case, initialization consists of using kunit_find_symbol()
> + * to find the address of the symbol, and if found, we set function pointer
> + * name to the function address value. In the non-module case, we simply
> + * assign name to symbol; this will generate a compilation error if the
> + * type we specified for function pointer @name does not match the symbol
> + * function type.
> + */
> +#ifdef MODULE
> +#define KUNIT_INIT_FN_SYMBOL(test, symbol, name) \
> + do { \
> + if (!name) \
> + name = kunit_find_symbol(#symbol); \
> + if (test) \
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, name); \
> + } while (0)
> +#else
> +#define KUNIT_INIT_FN_SYMBOL(test, symbol, name) \
> + name = symbol
> +#endif
> +
Can you put all the KUNIT_*_SYMBOL stuff in another patchset along with
the kunit_find_symbol?
> struct kunit;
>
> /**
> @@ -197,31 +278,45 @@ struct kunit {
> int kunit_run_tests(struct kunit_suite *suite);
>
> /**
> - * kunit_test_suite() - used to register a &struct kunit_suite with KUnit.
> + * kunit_test_suites() - used to register one or more &struct kunit_suite
> + * with KUnit.
> *
> - * @suite: a statically allocated &struct kunit_suite.
> + * @suites: a statically allocated list of &struct kunit_suite.
> *
> - * Registers @suite with the test framework. See &struct kunit_suite for
> + * Registers @suites with the test framework. See &struct kunit_suite for
> * more information.
> *
> - * NOTE: Currently KUnit tests are all run as late_initcalls; this means
> + * When builtin, KUnit tests are all run as late_initcalls; this means
> * that they cannot test anything where tests must run at a different init
> * phase. One significant restriction resulting from this is that KUnit
> * cannot reliably test anything that is initialize in the late_init phase;
> * another is that KUnit is useless to test things that need to be run in
> * an earlier init phase.
> *
> + * An alternative is to build the tests as a module. Because modules
> + * do not support multiple late_initcall()s, we need to initialize an
> + * array of suites for a module.
> + *
> * TODO(brendanhiggins@google.com): Don't run all KUnit tests as
> * late_initcalls. I have some future work planned to dispatch all KUnit
> * tests from the same place, and at the very least to do so after
> * everything else is definitely initialized.
> */
> -#define kunit_test_suite(suite) \
> - static int kunit_suite_init##suite(void) \
> - { \
> - return kunit_run_tests(&suite); \
> - } \
> - late_initcall(kunit_suite_init##suite)
> +#define kunit_test_suites(...) \
> + static struct kunit_suite *suites[] = { __VA_ARGS__, NULL}; \
> + static int kunit_test_suites_init(void) \
> + { \
> + unsigned int i; \
> + for (i = 0; suites[i] != NULL; i++) \
> + kunit_run_tests(suites[i]); \
> + return 0; \
> + } \
> + late_initcall(kunit_test_suites_init); \
> + static void __exit kunit_test_suites_exit(void) \
> + { \
> + return; \
> + } \
> + module_exit(kunit_test_suites_exit)
>
> /*
> * Like kunit_alloc_resource() below, but returns the struct kunit_resource
next prev parent reply other threads:[~2019-11-08 1:40 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-17 18:07 [PATCH v3 linux-kselftest-test 0/6] kunit: support building core/tests as modules Alan Maguire
2019-10-17 18:07 ` [PATCH v3 linux-kselftest-test 1/6] kunit: move string-stream.h to lib/kunit/string-stream-impl.h Alan Maguire
2019-11-08 1:01 ` Brendan Higgins
2019-10-17 18:07 ` [PATCH v3 linux-kselftest-test 2/6] kunit: hide unexported try-catch interface in try-catch-impl.h Alan Maguire
2019-11-08 1:07 ` Brendan Higgins
2019-10-17 18:07 ` [PATCH v3 linux-kselftest-test 3/6] kunit: add kunit_find_symbol() function for symbol lookup Alan Maguire
2019-11-08 1:24 ` Brendan Higgins
2019-10-17 18:07 ` [PATCH v3 linux-kselftest-test 4/6] kunit: allow kunit tests to be loaded as a module Alan Maguire
2019-11-08 1:40 ` Brendan Higgins [this message]
2019-10-17 18:07 ` [PATCH v3 linux-kselftest-test 5/6] kunit: allow kunit " Alan Maguire
2019-11-08 2:43 ` Brendan Higgins
2019-11-08 3:02 ` Brendan Higgins
2019-11-08 15:30 ` Alan Maguire
2019-11-11 21:41 ` Brendan Higgins
[not found] ` <20191114063815.9403820706@mail.kernel.org>
[not found] ` <alpine.LRH.2.20.1911140750450.8907@dhcp-10-175-202-216.vpn.oracle.com>
[not found] ` <CAFd5g46hMRR8L1Yd64ypWCqs5CpFpY_BCXfSCx0uc68ZzbiPzQ@mail.gmail.com>
2019-11-14 21:33 ` Brendan Higgins
2019-10-17 18:07 ` [PATCH v3 linux-kselftest-test 6/6] kunit: update documentation to describe module-based build Alan Maguire
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191108014047.GA216971@google.com \
--to=brendanhiggins@google.com \
--cc=akpm@linux-foundation.org \
--cc=alan.maguire@oracle.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=catalin.marinas@arm.com \
--cc=corbet@lwn.net \
--cc=joe.lawrence@redhat.com \
--cc=keescook@chromium.org \
--cc=knut.omang@oracle.com \
--cc=kunit-dev@googlegroups.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=schowdary@nvidia.com \
--cc=urezki@gmail.com \
--cc=yamada.masahiro@socionext.com \
--cc=yzaikin@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).