Alter the linker section of KUNIT_TABLE to move it out of INIT_DATA and
into DATA_DATA.
Data for KUnit tests does not need to be in the init section.
In order to run tests again after boot the KUnit data cannot be labeled as
init data as the kernel could write over it.
Add a KUNIT_INIT_TABLE in the next patch for KUnit tests that test init
data/functions.
Signed-off-by: Rae Moar <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index bae0fe4d499b..1107905d37fc 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -370,7 +370,8 @@
BRANCH_PROFILE() \
TRACE_PRINTKS() \
BPF_RAW_TP() \
- TRACEPOINT_STR()
+ TRACEPOINT_STR() \
+ KUNIT_TABLE()
/*
* Data section helpers
@@ -699,8 +700,7 @@
THERMAL_TABLE(governor) \
EARLYCON_TABLE() \
LSM_TABLE() \
- EARLY_LSM_TABLE() \
- KUNIT_TABLE()
+ EARLY_LSM_TABLE()
#define INIT_TEXT \
*(.init.text .init.text.*) \
base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
--
2.43.0.rc2.451.g8631bc7472-goog
Add KUNIT_INIT_TABLE to the INIT_DATA linker section.
Alter the KUnit macros to create init tests:
kunit_test_init_section_suites
Update lib/kunit/executor.c to run both the suites in KUNIT_TABLE and
KUNIT_INIT_TABLE.
Signed-off-by: Rae Moar <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 9 ++++-
include/kunit/test.h | 10 ++++-
include/linux/module.h | 2 +
kernel/module/main.c | 3 ++
lib/kunit/executor.c | 64 ++++++++++++++++++++++++++++---
lib/kunit/test.c | 26 +++++++++----
6 files changed, 99 insertions(+), 15 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 1107905d37fc..5dd3a61d673d 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -700,7 +700,8 @@
THERMAL_TABLE(governor) \
EARLYCON_TABLE() \
LSM_TABLE() \
- EARLY_LSM_TABLE()
+ EARLY_LSM_TABLE() \
+ KUNIT_INIT_TABLE()
#define INIT_TEXT \
*(.init.text .init.text.*) \
@@ -926,6 +927,12 @@
. = ALIGN(8); \
BOUNDED_SECTION_POST_LABEL(.kunit_test_suites, __kunit_suites, _start, _end)
+/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
+#define KUNIT_INIT_TABLE() \
+ . = ALIGN(8); \
+ BOUNDED_SECTION_POST_LABEL(.kunit_init_test_suites, \
+ __kunit_init_suites, _start, _end)
+
#ifdef CONFIG_BLK_DEV_INITRD
#define INIT_RAM_FS \
. = ALIGN(4); \
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 20ed9f9275c9..06e826a0b894 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -337,6 +337,9 @@ void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites);
void kunit_exec_run_tests(struct kunit_suite_set *suite_set, bool builtin);
void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr);
+struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_set,
+ struct kunit_suite_set suite_set);
+
#if IS_BUILTIN(CONFIG_KUNIT)
int kunit_run_all_tests(void);
#else
@@ -371,6 +374,11 @@ static inline int kunit_run_all_tests(void)
#define kunit_test_suite(suite) kunit_test_suites(&suite)
+#define __kunit_init_test_suites(unique_array, ...) \
+ static struct kunit_suite *unique_array[] \
+ __aligned(sizeof(struct kunit_suite *)) \
+ __used __section(".kunit_init_test_suites") = { __VA_ARGS__ }
+
/**
* kunit_test_init_section_suites() - used to register one or more &struct
* kunit_suite containing init functions or
@@ -392,7 +400,7 @@ static inline int kunit_run_all_tests(void)
* this manner.
*/
#define kunit_test_init_section_suites(__suites...) \
- __kunit_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe), \
+ __kunit_init_test_suites(__UNIQUE_ID(array), \
##__suites)
#define kunit_test_init_section_suite(suite) \
diff --git a/include/linux/module.h b/include/linux/module.h
index a98e188cf37b..9cd0009bd050 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -540,6 +540,8 @@ struct module {
struct static_call_site *static_call_sites;
#endif
#if IS_ENABLED(CONFIG_KUNIT)
+ int num_kunit_init_suites;
+ struct kunit_suite **kunit_init_suites;
int num_kunit_suites;
struct kunit_suite **kunit_suites;
#endif
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 98fedfdb8db5..36681911c05a 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2199,6 +2199,9 @@ static int find_module_sections(struct module *mod, struct load_info *info)
mod->kunit_suites = section_objs(info, ".kunit_test_suites",
sizeof(*mod->kunit_suites),
&mod->num_kunit_suites);
+ mod->kunit_init_suites = section_objs(info, ".kunit_init_test_suites",
+ sizeof(*mod->kunit_init_suites),
+ &mod->num_kunit_init_suites);
#endif
mod->extable = section_objs(info, "__ex_table",
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 1236b3cd2fbb..847329c51e91 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -12,6 +12,8 @@
*/
extern struct kunit_suite * const __kunit_suites_start[];
extern struct kunit_suite * const __kunit_suites_end[];
+extern struct kunit_suite * const __kunit_init_suites_start[];
+extern struct kunit_suite * const __kunit_init_suites_end[];
static char *action_param;
@@ -292,6 +294,33 @@ void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr)
}
}
+struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_set,
+ struct kunit_suite_set suite_set)
+{
+ struct kunit_suite_set total_suite_set = {NULL, NULL};
+ struct kunit_suite **total_suite_start = NULL;
+ size_t init_num_suites, num_suites, suite_size;
+
+ init_num_suites = init_suite_set.end - init_suite_set.start;
+ num_suites = suite_set.end - suite_set.start;
+ suite_size = sizeof(suite_set.start);
+
+ /* Allocate memory for array of all kunit suites */
+ total_suite_start = kmalloc_array(init_num_suites + num_suites, suite_size, GFP_KERNEL);
+ if (!total_suite_start)
+ return total_suite_set;
+
+ /* Append init suites and then all other kunit suites */
+ memcpy(total_suite_start, init_suite_set.start, init_num_suites * suite_size);
+ memcpy(total_suite_start + init_num_suites, suite_set.start, num_suites * suite_size);
+
+ /* Set kunit suite set start and end */
+ total_suite_set.start = total_suite_start;
+ total_suite_set.end = total_suite_start + (init_num_suites + num_suites);
+
+ return total_suite_set;
+}
+
#if IS_BUILTIN(CONFIG_KUNIT)
static char *kunit_shutdown;
@@ -313,21 +342,41 @@ static void kunit_handle_shutdown(void)
int kunit_run_all_tests(void)
{
- struct kunit_suite_set suite_set = {
+ struct kunit_suite_set suite_set = {NULL, NULL};
+ struct kunit_suite_set filtered_suite_set = {NULL, NULL};
+ struct kunit_suite_set init_suite_set = {
+ __kunit_init_suites_start, __kunit_init_suites_end,
+ };
+ struct kunit_suite_set normal_suite_set = {
__kunit_suites_start, __kunit_suites_end,
};
+ size_t init_num_suites = init_suite_set.end - init_suite_set.start;
int err = 0;
+
+ if (init_num_suites > 0) {
+ suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
+ if (!suite_set.start)
+ goto out;
+ } else
+ suite_set = normal_suite_set;
+
if (!kunit_enabled()) {
pr_info("kunit: disabled\n");
- goto out;
+ goto free_out;
}
if (filter_glob_param || filter_param) {
- suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
+ filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
filter_param, filter_action_param, &err);
+
+ /* Free original suite set before using filtered suite set */
+ if (init_num_suites > 0)
+ kfree(suite_set.start);
+ suite_set = filtered_suite_set;
+
if (err) {
pr_err("kunit executor: error filtering suites: %d\n", err);
- goto out;
+ goto free_out;
}
}
@@ -340,9 +389,12 @@ int kunit_run_all_tests(void)
else
pr_err("kunit executor: unknown action '%s'\n", action_param);
- if (filter_glob_param || filter_param) { /* a copy was made of each suite */
+free_out:
+ if (filter_glob_param || filter_param)
kunit_free_suite_set(suite_set);
- }
+ else if (init_num_suites > 0)
+ /* Don't use kunit_free_suite_set because suites aren't individually allocated */
+ kfree(suite_set.start);
out:
kunit_handle_shutdown();
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index f2eb71f1a66c..8bae6e2bc6a0 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -704,28 +704,40 @@ EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
#ifdef CONFIG_MODULES
static void kunit_module_init(struct module *mod)
{
- struct kunit_suite_set suite_set = {
+ struct kunit_suite_set suite_set, filtered_set;
+ struct kunit_suite_set normal_suite_set = {
mod->kunit_suites, mod->kunit_suites + mod->num_kunit_suites,
};
+ struct kunit_suite_set init_suite_set = {
+ mod->kunit_init_suites, mod->kunit_init_suites + mod->num_kunit_init_suites,
+ };
const char *action = kunit_action();
int err = 0;
- suite_set = kunit_filter_suites(&suite_set,
+ if (mod->num_kunit_init_suites > 0)
+ suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
+ else
+ suite_set = normal_suite_set;
+
+ filtered_set = kunit_filter_suites(&suite_set,
kunit_filter_glob() ?: "*.*",
kunit_filter(), kunit_filter_action(),
&err);
if (err)
pr_err("kunit module: error filtering suites: %d\n", err);
- mod->kunit_suites = (struct kunit_suite **)suite_set.start;
- mod->num_kunit_suites = suite_set.end - suite_set.start;
+ mod->kunit_suites = (struct kunit_suite **)filtered_set.start;
+ mod->num_kunit_suites = filtered_set.end - filtered_set.start;
+
+ if (mod->num_kunit_init_suites > 0)
+ kfree(suite_set.start);
if (!action)
- kunit_exec_run_tests(&suite_set, false);
+ kunit_exec_run_tests(&filtered_set, false);
else if (!strcmp(action, "list"))
- kunit_exec_list_tests(&suite_set, false);
+ kunit_exec_list_tests(&filtered_set, false);
else if (!strcmp(action, "list_attr"))
- kunit_exec_list_tests(&suite_set, true);
+ kunit_exec_list_tests(&filtered_set, true);
else
pr_err("kunit: unknown action '%s'\n", action);
}
--
2.43.0.rc2.451.g8631bc7472-goog
Add example_init_test_suite to allow for testing the feature of running
test suites marked as init to indicate they use init data and/or
functions.
This suite should always pass and uses a simple init function.
This suite can also be used to test the is_init attribute introduced in
the next patch.
Signed-off-by: Rae Moar <[email protected]>
---
lib/kunit/kunit-example-test.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index 6bb5c2ef6696..262a68a59800 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -287,4 +287,33 @@ static struct kunit_suite example_test_suite = {
*/
kunit_test_suites(&example_test_suite);
+static int __init init_add(int x, int y)
+{
+ return (x + y);
+}
+
+/*
+ * This test should always pass. Can be used to test init suites.
+ */
+static void example_init_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, init_add(1, 1), 2);
+}
+
+static struct kunit_case example_init_test_cases[] = {
+ KUNIT_CASE(example_init_test),
+ {}
+};
+
+static struct kunit_suite example_init_test_suite = {
+ .name = "example_init",
+ .test_cases = example_init_test_cases,
+};
+
+/*
+ * This registers the test suite and marks the suite as using init data
+ * and/or functions.
+ */
+kunit_test_init_section_suites(&example_init_test_suite);
+
MODULE_LICENSE("GPL v2");
--
2.43.0.rc2.451.g8631bc7472-goog
Add is_init test attribute of type bool. Add to_string, get, and filter
methods to lib/kunit/attributes.c.
Mark each of the tests in the init section with the is_init=true attribute.
Add is_init to the attributes documentation.
Signed-off-by: Rae Moar <[email protected]>
---
.../dev-tools/kunit/running_tips.rst | 7 +++
include/kunit/test.h | 3 +
lib/kunit/attributes.c | 60 +++++++++++++++++++
lib/kunit/executor.c | 6 +-
4 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/Documentation/dev-tools/kunit/running_tips.rst b/Documentation/dev-tools/kunit/running_tips.rst
index 766f9cdea0fa..024e9ad1d1e9 100644
--- a/Documentation/dev-tools/kunit/running_tips.rst
+++ b/Documentation/dev-tools/kunit/running_tips.rst
@@ -428,3 +428,10 @@ This attribute indicates the name of the module associated with the test.
This attribute is automatically saved as a string and is printed for each suite.
Tests can also be filtered using this attribute.
+
+``is_init``
+
+This attribute indicates whether the test uses init data or functions.
+
+This attribute is automatically saved as a boolean and tests can also be
+filtered using this attribute.
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 06e826a0b894..65583782903d 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -82,6 +82,9 @@ enum kunit_speed {
/* Holds attributes for each test case and suite */
struct kunit_attributes {
enum kunit_speed speed;
+
+ /* private: internal use only */
+ bool is_init;
};
/**
diff --git a/lib/kunit/attributes.c b/lib/kunit/attributes.c
index 1b512f7e1838..ddacec6a3337 100644
--- a/lib/kunit/attributes.c
+++ b/lib/kunit/attributes.c
@@ -58,6 +58,16 @@ static const char *attr_enum_to_string(void *attr, const char * const str_list[]
return str_list[val];
}
+static const char *attr_bool_to_string(void *attr, bool *to_free)
+{
+ bool val = (bool)attr;
+
+ *to_free = false;
+ if (val)
+ return "true";
+ return "false";
+}
+
static const char *attr_speed_to_string(void *attr, bool *to_free)
{
return attr_enum_to_string(attr, speed_str_list, to_free);
@@ -166,6 +176,37 @@ static int attr_string_filter(void *attr, const char *input, int *err)
return false;
}
+static int attr_bool_filter(void *attr, const char *input, int *err)
+{
+ int i, input_int = -1;
+ long val = (long)attr;
+ const char *input_str = NULL;
+
+ for (i = 0; input[i]; i++) {
+ if (!strchr(op_list, input[i])) {
+ input_str = input + i;
+ break;
+ }
+ }
+
+ if (!input_str) {
+ *err = -EINVAL;
+ pr_err("kunit executor: filter value not found: %s\n", input);
+ return false;
+ }
+
+ if (!strcmp(input_str, "true"))
+ input_int = (int)true;
+ else if (!strcmp(input_str, "false"))
+ input_int = (int)false;
+ else {
+ *err = -EINVAL;
+ pr_err("kunit executor: invalid filter input: %s\n", input);
+ return false;
+ }
+
+ return int_filter(val, input, input_int, err);
+}
/* Get Attribute Methods */
@@ -194,6 +235,17 @@ static void *attr_module_get(void *test_or_suite, bool is_test)
return (void *) "";
}
+static void *attr_is_init_get(void *test_or_suite, bool is_test)
+{
+ struct kunit_suite *suite = is_test ? NULL : test_or_suite;
+ struct kunit_case *test = is_test ? test_or_suite : NULL;
+
+ if (test)
+ return ((void *) test->attr.is_init);
+ else
+ return ((void *) suite->attr.is_init);
+}
+
/* List of all Test Attributes */
static struct kunit_attr kunit_attr_list[] = {
@@ -212,6 +264,14 @@ static struct kunit_attr kunit_attr_list[] = {
.filter = attr_string_filter,
.attr_default = (void *)"",
.print = PRINT_SUITE,
+ },
+ {
+ .name = "is_init",
+ .get_attr = attr_is_init_get,
+ .to_string = attr_bool_to_string,
+ .filter = attr_bool_filter,
+ .attr_default = (void *)false,
+ .print = PRINT_SUITE,
}
};
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 847329c51e91..594de994161a 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -300,6 +300,7 @@ struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_
struct kunit_suite_set total_suite_set = {NULL, NULL};
struct kunit_suite **total_suite_start = NULL;
size_t init_num_suites, num_suites, suite_size;
+ int i = 0;
init_num_suites = init_suite_set.end - init_suite_set.start;
num_suites = suite_set.end - suite_set.start;
@@ -310,8 +311,11 @@ struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_
if (!total_suite_start)
return total_suite_set;
- /* Append init suites and then all other kunit suites */
+ /* Append and mark init suites and then append all other kunit suites */
memcpy(total_suite_start, init_suite_set.start, init_num_suites * suite_size);
+ for (i = 0; i < init_num_suites; i++)
+ total_suite_start[i]->attr.is_init = true;
+
memcpy(total_suite_start + init_num_suites, suite_set.start, num_suites * suite_size);
/* Set kunit suite set start and end */
--
2.43.0.rc2.451.g8631bc7472-goog
Expand the documentation on the KUnit debugfs filesystem on the
run_manual.rst page.
Add section describing how to access results using debugfs.
Add section describing how to run tests after boot using debugfs.
Signed-off-by: Rae Moar <[email protected]>
---
Changes since v2:
- Add info to documentation about cleaning up data, init tests, and
running tests concurrently
Documentation/dev-tools/kunit/run_manual.rst | 49 ++++++++++++++++++--
1 file changed, 45 insertions(+), 4 deletions(-)
diff --git a/Documentation/dev-tools/kunit/run_manual.rst b/Documentation/dev-tools/kunit/run_manual.rst
index e7b46421f247..aebb52ba9605 100644
--- a/Documentation/dev-tools/kunit/run_manual.rst
+++ b/Documentation/dev-tools/kunit/run_manual.rst
@@ -49,9 +49,50 @@ loaded.
The results will appear in TAP format in ``dmesg``.
+debugfs
+=======
+
+``debugfs`` is a file system that enables user interaction with the files to
+make kernel information available to user space (See more information at
+Documentation/filesystems/debugfs.html)
+
+By default, only the root user has access to the debugfs directory.
+
+If ``CONFIG_KUNIT_DEBUGFS`` is enabled, you can use KUnit debugfs
+filesystem to perform the following actions.
+
+Retrieve Test Results
+=====================
+
+You can use debugfs to retrieve KUnit test results. The test results are
+accessible from the debugfs filesystem in the following read-only file:
+
+.. code-block :: bash
+
+ /sys/kernel/debug/kunit/<test_suite>/results
+
+The test results are available in KTAP format.
+
+Run Tests After Kernel Has Booted
+=================================
+
+You can use the debugfs filesystem to trigger built-in tests to run after
+boot. To run the test suite, you can use the following command to write to
+the ``/sys/kernel/debug/kunit/<test_suite>/run`` file:
+
+.. code-block :: bash
+
+ echo "any string" > /sys/kernel/debugfs/kunit/<test_suite>/run
+
+As a result, the test suite runs and the results are printed to the kernel
+log.
+
+However, this feature is not available with KUnit tests that use init data.
+
+Also, you cannot use this feature to run tests concurrently as there is a
+mutex lock around running KUnit tests at the same time.
+
.. note ::
- If ``CONFIG_KUNIT_DEBUGFS`` is enabled, KUnit test results will
- be accessible from the ``debugfs`` filesystem (if mounted).
- They will be in ``/sys/kernel/debug/kunit/<test_suite>/results``, in
- TAP format.
+ For test authors, to use this feature, tests will need to correctly initialise
+ and/or clean up any data, so the test runs correctly a second time.
--
2.43.0.rc2.451.g8631bc7472-goog
Add functionality to run built-in tests after boot by writing to a
debugfs file.
Add a new debugfs file labeled "run" for each test suite to use for
this purpose.
As an example, write to the file using the following:
echo "any string" > /sys/kernel/debugfs/kunit/<testsuite>/run
This will trigger the test suite to run and will print results to the
kernel log.
To guard against running tests concurrently with this feature, add a
mutex lock around running kunit. This supports the current practice of
not allowing tests to be run concurrently on the same kernel.
This new functionality could be used to design a parameter
injection feature in the future.
Signed-off-by: Rae Moar <[email protected]>
---
Changes since v2:
- Move resetting the log to test.c
- Add is_init attribute and patches to change linker sections to avoid
re-running tests that use init data and functions
lib/kunit/debugfs.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
lib/kunit/test.c | 10 +++++++
2 files changed, 78 insertions(+)
diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
index 270d185737e6..2e0a92a0c461 100644
--- a/lib/kunit/debugfs.c
+++ b/lib/kunit/debugfs.c
@@ -8,12 +8,14 @@
#include <linux/module.h>
#include <kunit/test.h>
+#include <kunit/test-bug.h>
#include "string-stream.h"
#include "debugfs.h"
#define KUNIT_DEBUGFS_ROOT "kunit"
#define KUNIT_DEBUGFS_RESULTS "results"
+#define KUNIT_DEBUGFS_RUN "run"
/*
* Create a debugfs representation of test suites:
@@ -21,6 +23,8 @@
* Path Semantics
* /sys/kernel/debug/kunit/<testsuite>/results Show results of last run for
* testsuite
+ * /sys/kernel/debug/kunit/<testsuite>/run Write to this file to trigger
+ * testsuite to run
*
*/
@@ -99,6 +103,51 @@ static int debugfs_results_open(struct inode *inode, struct file *file)
return single_open(file, debugfs_print_results, suite);
}
+/*
+ * Print a usage message to the debugfs "run" file
+ * (/sys/kernel/debug/kunit/<testsuite>/run) if opened.
+ */
+static int debugfs_print_run(struct seq_file *seq, void *v)
+{
+ struct kunit_suite *suite = (struct kunit_suite *)seq->private;
+
+ seq_puts(seq, "Write to this file to trigger the test suite to run.\n");
+ seq_printf(seq, "usage: echo \"any string\" > /sys/kernel/debugfs/kunit/%s/run\n",
+ suite->name);
+ return 0;
+}
+
+/*
+ * The debugfs "run" file (/sys/kernel/debug/kunit/<testsuite>/run)
+ * contains no information. Write to the file to trigger the test suite
+ * to run.
+ */
+static int debugfs_run_open(struct inode *inode, struct file *file)
+{
+ struct kunit_suite *suite;
+
+ suite = (struct kunit_suite *)inode->i_private;
+
+ return single_open(file, debugfs_print_run, suite);
+}
+
+/*
+ * Trigger a test suite to run by writing to the suite's "run" debugfs
+ * file found at: /sys/kernel/debug/kunit/<testsuite>/run
+ *
+ * Note: what is written to this file will not be saved.
+ */
+static ssize_t debugfs_run(struct file *file,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ struct inode *f_inode = file->f_inode;
+ struct kunit_suite *suite = (struct kunit_suite *) f_inode->i_private;
+
+ __kunit_test_suites_init(&suite, 1);
+
+ return count;
+}
+
static const struct file_operations debugfs_results_fops = {
.open = debugfs_results_open,
.read = seq_read,
@@ -106,10 +155,22 @@ static const struct file_operations debugfs_results_fops = {
.release = debugfs_release,
};
+static const struct file_operations debugfs_run_fops = {
+ .open = debugfs_run_open,
+ .read = seq_read,
+ .write = debugfs_run,
+ .llseek = seq_lseek,
+ .release = debugfs_release,
+};
+
void kunit_debugfs_create_suite(struct kunit_suite *suite)
{
struct kunit_case *test_case;
+ /* If suite log already allocated, do not create new debugfs files. */
+ if (suite->log)
+ return;
+
/* Allocate logs before creating debugfs representation. */
suite->log = alloc_string_stream(GFP_KERNEL);
string_stream_set_append_newlines(suite->log, true);
@@ -124,6 +185,13 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite)
debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444,
suite->debugfs,
suite, &debugfs_results_fops);
+
+ /* Do not create file to re-run test if test runs on init */
+ if (!suite->attr.is_init) {
+ debugfs_create_file(KUNIT_DEBUGFS_RUN, S_IFREG | 0644,
+ suite->debugfs,
+ suite, &debugfs_run_fops);
+ }
}
void kunit_debugfs_destroy_suite(struct kunit_suite *suite)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 8bae6e2bc6a0..58e46bb3b4c4 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -13,6 +13,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
+#include <linux/mutex.h>
#include <linux/panic.h>
#include <linux/sched/debug.h>
#include <linux/sched.h>
@@ -22,6 +23,8 @@
#include "string-stream.h"
#include "try-catch-impl.h"
+static DEFINE_MUTEX(kunit_run_lock);
+
/*
* Hook to fail the current test and print an error message to the log.
*/
@@ -654,6 +657,7 @@ static void kunit_init_suite(struct kunit_suite *suite)
kunit_debugfs_create_suite(suite);
suite->status_comment[0] = '\0';
suite->suite_init_err = 0;
+ string_stream_clear(suite->log);
}
bool kunit_enabled(void)
@@ -670,6 +674,11 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
return 0;
}
+ /* Use mutex lock to guard against running tests concurrently. */
+ if (mutex_lock_interruptible(&kunit_run_lock)) {
+ pr_err("kunit: test interrupted\n");
+ return -EINTR;
+ }
static_branch_inc(&kunit_running);
for (i = 0; i < num_suites; i++) {
@@ -678,6 +687,7 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
}
static_branch_dec(&kunit_running);
+ mutex_unlock(&kunit_run_lock);
return 0;
}
EXPORT_SYMBOL_GPL(__kunit_test_suites_init);
--
2.43.0.rc2.451.g8631bc7472-goog
On Tue, 5 Dec 2023 at 06:19, Rae Moar <[email protected]> wrote:
>
> Alter the linker section of KUNIT_TABLE to move it out of INIT_DATA and
> into DATA_DATA.
>
> Data for KUnit tests does not need to be in the init section.
>
> In order to run tests again after boot the KUnit data cannot be labeled as
> init data as the kernel could write over it.
>
> Add a KUNIT_INIT_TABLE in the next patch for KUnit tests that test init
> data/functions.
>
> Signed-off-by: Rae Moar <[email protected]>
> ---
I think this actually fixes a potential bug, as we loop through the
list of suites after init has ended in the debugfs logic.
So maybe this is:
Fixes: 90a025a859a3 ("vmlinux.lds.h: add linker section for KUnit test suites")
Regardless, I'd love to get this in, even if we don't manage to get
the rest of the series in soon.
Reviewed-by: David Gow <[email protected]>
Cheers,
-- David
> include/asm-generic/vmlinux.lds.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index bae0fe4d499b..1107905d37fc 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -370,7 +370,8 @@
> BRANCH_PROFILE() \
> TRACE_PRINTKS() \
> BPF_RAW_TP() \
> - TRACEPOINT_STR()
> + TRACEPOINT_STR() \
> + KUNIT_TABLE()
>
> /*
> * Data section helpers
> @@ -699,8 +700,7 @@
> THERMAL_TABLE(governor) \
> EARLYCON_TABLE() \
> LSM_TABLE() \
> - EARLY_LSM_TABLE() \
> - KUNIT_TABLE()
> + EARLY_LSM_TABLE()
>
> #define INIT_TEXT \
> *(.init.text .init.text.*) \
>
> base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
> --
> 2.43.0.rc2.451.g8631bc7472-goog
>
On Tue, 5 Dec 2023 at 06:19, Rae Moar <[email protected]> wrote:
>
> Add KUNIT_INIT_TABLE to the INIT_DATA linker section.
>
> Alter the KUnit macros to create init tests:
> kunit_test_init_section_suites
>
> Update lib/kunit/executor.c to run both the suites in KUNIT_TABLE and
> KUNIT_INIT_TABLE.
>
> Signed-off-by: Rae Moar <[email protected]>
> ---
This works well here.
I'm still a little bit conflicted around the idea of merging suite
sets at runtime -- I think there could be more efficient ways of
handling that -- though the more I think about it, the less worried
I'm getting (since we'll need to keep init suites around somewhere for
debugfs, anyway, right?).
In fact, that's something we probably need to work out -- is it legal
for the actual kunit_test_suite struct to be __initdata? I'd thought
so, but if we need to loop over these later in debugfs to keep their
logs, then probably not. Unless you wanted to make a copy of the
kunit_suite itself, not just the pointers to it (though that seems
excessive).
If we're settled on that (the suite itself can't be __initdata), then this is:
Reviewed-by: David Gow <[email protected]>
-- David
> include/asm-generic/vmlinux.lds.h | 9 ++++-
> include/kunit/test.h | 10 ++++-
> include/linux/module.h | 2 +
> kernel/module/main.c | 3 ++
> lib/kunit/executor.c | 64 ++++++++++++++++++++++++++++---
> lib/kunit/test.c | 26 +++++++++----
> 6 files changed, 99 insertions(+), 15 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 1107905d37fc..5dd3a61d673d 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -700,7 +700,8 @@
> THERMAL_TABLE(governor) \
> EARLYCON_TABLE() \
> LSM_TABLE() \
> - EARLY_LSM_TABLE()
> + EARLY_LSM_TABLE() \
> + KUNIT_INIT_TABLE()
>
> #define INIT_TEXT \
> *(.init.text .init.text.*) \
> @@ -926,6 +927,12 @@
> . = ALIGN(8); \
> BOUNDED_SECTION_POST_LABEL(.kunit_test_suites, __kunit_suites, _start, _end)
>
> +/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
> +#define KUNIT_INIT_TABLE() \
> + . = ALIGN(8); \
I still hate that we hardcode '8' here, but I guess we've got no
choice in a linker script.
> + BOUNDED_SECTION_POST_LABEL(.kunit_init_test_suites, \
> + __kunit_init_suites, _start, _end)
> +
> #ifdef CONFIG_BLK_DEV_INITRD
> #define INIT_RAM_FS \
> . = ALIGN(4); \
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 20ed9f9275c9..06e826a0b894 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -337,6 +337,9 @@ void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites);
> void kunit_exec_run_tests(struct kunit_suite_set *suite_set, bool builtin);
> void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr);
>
> +struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_set,
> + struct kunit_suite_set suite_set);
> +
> #if IS_BUILTIN(CONFIG_KUNIT)
> int kunit_run_all_tests(void);
> #else
> @@ -371,6 +374,11 @@ static inline int kunit_run_all_tests(void)
>
> #define kunit_test_suite(suite) kunit_test_suites(&suite)
>
> +#define __kunit_init_test_suites(unique_array, ...) \
> + static struct kunit_suite *unique_array[] \
> + __aligned(sizeof(struct kunit_suite *)) \
> + __used __section(".kunit_init_test_suites") = { __VA_ARGS__ }
> +
> /**
> * kunit_test_init_section_suites() - used to register one or more &struct
> * kunit_suite containing init functions or
> @@ -392,7 +400,7 @@ static inline int kunit_run_all_tests(void)
> * this manner.
> */
> #define kunit_test_init_section_suites(__suites...) \
> - __kunit_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe), \
> + __kunit_init_test_suites(__UNIQUE_ID(array), \
> ##__suites)
>
> #define kunit_test_init_section_suite(suite) \
> diff --git a/include/linux/module.h b/include/linux/module.h
> index a98e188cf37b..9cd0009bd050 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -540,6 +540,8 @@ struct module {
> struct static_call_site *static_call_sites;
> #endif
> #if IS_ENABLED(CONFIG_KUNIT)
> + int num_kunit_init_suites;
> + struct kunit_suite **kunit_init_suites;
> int num_kunit_suites;
> struct kunit_suite **kunit_suites;
> #endif
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 98fedfdb8db5..36681911c05a 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2199,6 +2199,9 @@ static int find_module_sections(struct module *mod, struct load_info *info)
> mod->kunit_suites = section_objs(info, ".kunit_test_suites",
> sizeof(*mod->kunit_suites),
> &mod->num_kunit_suites);
> + mod->kunit_init_suites = section_objs(info, ".kunit_init_test_suites",
> + sizeof(*mod->kunit_init_suites),
> + &mod->num_kunit_init_suites);
> #endif
>
> mod->extable = section_objs(info, "__ex_table",
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 1236b3cd2fbb..847329c51e91 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -12,6 +12,8 @@
> */
> extern struct kunit_suite * const __kunit_suites_start[];
> extern struct kunit_suite * const __kunit_suites_end[];
> +extern struct kunit_suite * const __kunit_init_suites_start[];
> +extern struct kunit_suite * const __kunit_init_suites_end[];
>
> static char *action_param;
>
> @@ -292,6 +294,33 @@ void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr)
> }
> }
>
> +struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_set,
> + struct kunit_suite_set suite_set)
> +{
> + struct kunit_suite_set total_suite_set = {NULL, NULL};
> + struct kunit_suite **total_suite_start = NULL;
> + size_t init_num_suites, num_suites, suite_size;
> +
> + init_num_suites = init_suite_set.end - init_suite_set.start;
> + num_suites = suite_set.end - suite_set.start;
> + suite_size = sizeof(suite_set.start);
> +
> + /* Allocate memory for array of all kunit suites */
> + total_suite_start = kmalloc_array(init_num_suites + num_suites, suite_size, GFP_KERNEL);
> + if (!total_suite_start)
> + return total_suite_set;
> +
> + /* Append init suites and then all other kunit suites */
> + memcpy(total_suite_start, init_suite_set.start, init_num_suites * suite_size);
> + memcpy(total_suite_start + init_num_suites, suite_set.start, num_suites * suite_size);
> +
> + /* Set kunit suite set start and end */
> + total_suite_set.start = total_suite_start;
> + total_suite_set.end = total_suite_start + (init_num_suites + num_suites);
> +
> + return total_suite_set;
> +}
> +
> #if IS_BUILTIN(CONFIG_KUNIT)
>
> static char *kunit_shutdown;
> @@ -313,21 +342,41 @@ static void kunit_handle_shutdown(void)
>
> int kunit_run_all_tests(void)
> {
> - struct kunit_suite_set suite_set = {
> + struct kunit_suite_set suite_set = {NULL, NULL};
> + struct kunit_suite_set filtered_suite_set = {NULL, NULL};
> + struct kunit_suite_set init_suite_set = {
> + __kunit_init_suites_start, __kunit_init_suites_end,
> + };
> + struct kunit_suite_set normal_suite_set = {
> __kunit_suites_start, __kunit_suites_end,
> };
> + size_t init_num_suites = init_suite_set.end - init_suite_set.start;
> int err = 0;
> +
> + if (init_num_suites > 0) {
> + suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
> + if (!suite_set.start)
> + goto out;
> + } else
> + suite_set = normal_suite_set;
> +
> if (!kunit_enabled()) {
> pr_info("kunit: disabled\n");
> - goto out;
> + goto free_out;
> }
>
> if (filter_glob_param || filter_param) {
> - suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
> + filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
> filter_param, filter_action_param, &err);
> +
> + /* Free original suite set before using filtered suite set */
> + if (init_num_suites > 0)
> + kfree(suite_set.start);
> + suite_set = filtered_suite_set;
> +
> if (err) {
> pr_err("kunit executor: error filtering suites: %d\n", err);
> - goto out;
> + goto free_out;
> }
> }
>
> @@ -340,9 +389,12 @@ int kunit_run_all_tests(void)
> else
> pr_err("kunit executor: unknown action '%s'\n", action_param);
>
> - if (filter_glob_param || filter_param) { /* a copy was made of each suite */
> +free_out:
> + if (filter_glob_param || filter_param)
> kunit_free_suite_set(suite_set);
> - }
> + else if (init_num_suites > 0)
> + /* Don't use kunit_free_suite_set because suites aren't individually allocated */
> + kfree(suite_set.start);
>
> out:
> kunit_handle_shutdown();
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index f2eb71f1a66c..8bae6e2bc6a0 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -704,28 +704,40 @@ EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
> #ifdef CONFIG_MODULES
> static void kunit_module_init(struct module *mod)
> {
> - struct kunit_suite_set suite_set = {
> + struct kunit_suite_set suite_set, filtered_set;
> + struct kunit_suite_set normal_suite_set = {
> mod->kunit_suites, mod->kunit_suites + mod->num_kunit_suites,
> };
> + struct kunit_suite_set init_suite_set = {
> + mod->kunit_init_suites, mod->kunit_init_suites + mod->num_kunit_init_suites,
> + };
> const char *action = kunit_action();
> int err = 0;
>
> - suite_set = kunit_filter_suites(&suite_set,
> + if (mod->num_kunit_init_suites > 0)
> + suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
> + else
> + suite_set = normal_suite_set;
> +
> + filtered_set = kunit_filter_suites(&suite_set,
> kunit_filter_glob() ?: "*.*",
> kunit_filter(), kunit_filter_action(),
> &err);
> if (err)
> pr_err("kunit module: error filtering suites: %d\n", err);
>
> - mod->kunit_suites = (struct kunit_suite **)suite_set.start;
> - mod->num_kunit_suites = suite_set.end - suite_set.start;
> + mod->kunit_suites = (struct kunit_suite **)filtered_set.start;
> + mod->num_kunit_suites = filtered_set.end - filtered_set.start;
> +
> + if (mod->num_kunit_init_suites > 0)
> + kfree(suite_set.start);
>
> if (!action)
> - kunit_exec_run_tests(&suite_set, false);
> + kunit_exec_run_tests(&filtered_set, false);
> else if (!strcmp(action, "list"))
> - kunit_exec_list_tests(&suite_set, false);
> + kunit_exec_list_tests(&filtered_set, false);
> else if (!strcmp(action, "list_attr"))
> - kunit_exec_list_tests(&suite_set, true);
> + kunit_exec_list_tests(&filtered_set, true);
> else
> pr_err("kunit: unknown action '%s'\n", action);
> }
> --
> 2.43.0.rc2.451.g8631bc7472-goog
>
On Tue, 5 Dec 2023 at 06:19, Rae Moar <[email protected]> wrote:
>
> Add example_init_test_suite to allow for testing the feature of running
> test suites marked as init to indicate they use init data and/or
> functions.
>
> This suite should always pass and uses a simple init function.
>
> This suite can also be used to test the is_init attribute introduced in
> the next patch.
>
> Signed-off-by: Rae Moar <[email protected]>
> ---
Can we make the actual test function __init as well? I don't think it
should be _necessary_ for this to work, but I'd like us to have the
option of entire tests being marked __init, and so discarded after
boot.
This seems to work here if the test is marked __init, and the
example_init_test_cases and example_init_test_suite are marked
__initdata.
But, as mentioned in the previous patch, that might not work if we
need to ensure example_init_test_suite and other metadata still
exists. So maybe we can document that here?
Otherwise, looks good.
Reviewed-by: David Gow <[email protected]>
Cheers,
-- David
> lib/kunit/kunit-example-test.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
> index 6bb5c2ef6696..262a68a59800 100644
> --- a/lib/kunit/kunit-example-test.c
> +++ b/lib/kunit/kunit-example-test.c
> @@ -287,4 +287,33 @@ static struct kunit_suite example_test_suite = {
> */
> kunit_test_suites(&example_test_suite);
>
> +static int __init init_add(int x, int y)
> +{
> + return (x + y);
> +}
> +
> +/*
> + * This test should always pass. Can be used to test init suites.
> + */
> +static void example_init_test(struct kunit *test)
> +{
> + KUNIT_EXPECT_EQ(test, init_add(1, 1), 2);
> +}
> +
> +static struct kunit_case example_init_test_cases[] = {
> + KUNIT_CASE(example_init_test),
> + {}
> +};
> +
> +static struct kunit_suite example_init_test_suite = {
> + .name = "example_init",
> + .test_cases = example_init_test_cases,
> +};
> +
> +/*
> + * This registers the test suite and marks the suite as using init data
> + * and/or functions.
> + */
> +kunit_test_init_section_suites(&example_init_test_suite);
> +
> MODULE_LICENSE("GPL v2");
> --
> 2.43.0.rc2.451.g8631bc7472-goog
>
On Tue, 5 Dec 2023 at 06:19, Rae Moar <[email protected]> wrote:
>
> Add is_init test attribute of type bool. Add to_string, get, and filter
> methods to lib/kunit/attributes.c.
>
> Mark each of the tests in the init section with the is_init=true attribute.
>
> Add is_init to the attributes documentation.
>
> Signed-off-by: Rae Moar <[email protected]>
> ---
Would it be possible to not have this in kunit_attributes? I know it's
required for the run-after-boot stuff later, but I'd love this to be
(a) just generated at runtime, or (b) stored only at a suite or
suite-set level. It seems like a bit of a waste to store this
per-test-case, and to have it potentially accessible or overwritable
by users.
Otherwise, this looks good (and I appreciate the automatic setting of
this when merging the suite sets.
Maybe if we always kept the init suites in a separate set, we could
just use pointer comparisons to generate this; otherwise let's make
this a suite-level-only attribute (inherited by tests).
-- David
On Tue, 5 Dec 2023 at 06:19, Rae Moar <[email protected]> wrote:
>
> Add functionality to run built-in tests after boot by writing to a
> debugfs file.
>
> Add a new debugfs file labeled "run" for each test suite to use for
> this purpose.
>
> As an example, write to the file using the following:
>
> echo "any string" > /sys/kernel/debugfs/kunit/<testsuite>/run
>
> This will trigger the test suite to run and will print results to the
> kernel log.
>
> To guard against running tests concurrently with this feature, add a
> mutex lock around running kunit. This supports the current practice of
> not allowing tests to be run concurrently on the same kernel.
>
> This new functionality could be used to design a parameter
> injection feature in the future.
>
> Signed-off-by: Rae Moar <[email protected]>
> ---
This looks good to me.
A future feature which may be useful would be to support other kunit
actions here (like list, list_attr), but that's probably worth leaving
as a follow-up.
Reviewed-by: David Gow <[email protected]>
Cheers,
-- David
>
> Changes since v2:
> - Move resetting the log to test.c
> - Add is_init attribute and patches to change linker sections to avoid
> re-running tests that use init data and functions
>
> lib/kunit/debugfs.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
> lib/kunit/test.c | 10 +++++++
> 2 files changed, 78 insertions(+)
>
> diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
> index 270d185737e6..2e0a92a0c461 100644
> --- a/lib/kunit/debugfs.c
> +++ b/lib/kunit/debugfs.c
> @@ -8,12 +8,14 @@
> #include <linux/module.h>
>
> #include <kunit/test.h>
> +#include <kunit/test-bug.h>
>
> #include "string-stream.h"
> #include "debugfs.h"
>
> #define KUNIT_DEBUGFS_ROOT "kunit"
> #define KUNIT_DEBUGFS_RESULTS "results"
> +#define KUNIT_DEBUGFS_RUN "run"
>
> /*
> * Create a debugfs representation of test suites:
> @@ -21,6 +23,8 @@
> * Path Semantics
> * /sys/kernel/debug/kunit/<testsuite>/results Show results of last run for
> * testsuite
> + * /sys/kernel/debug/kunit/<testsuite>/run Write to this file to trigger
> + * testsuite to run
> *
> */
>
> @@ -99,6 +103,51 @@ static int debugfs_results_open(struct inode *inode, struct file *file)
> return single_open(file, debugfs_print_results, suite);
> }
>
> +/*
> + * Print a usage message to the debugfs "run" file
> + * (/sys/kernel/debug/kunit/<testsuite>/run) if opened.
> + */
> +static int debugfs_print_run(struct seq_file *seq, void *v)
> +{
> + struct kunit_suite *suite = (struct kunit_suite *)seq->private;
> +
> + seq_puts(seq, "Write to this file to trigger the test suite to run.\n");
> + seq_printf(seq, "usage: echo \"any string\" > /sys/kernel/debugfs/kunit/%s/run\n",
> + suite->name);
> + return 0;
> +}
> +
> +/*
> + * The debugfs "run" file (/sys/kernel/debug/kunit/<testsuite>/run)
> + * contains no information. Write to the file to trigger the test suite
> + * to run.
> + */
> +static int debugfs_run_open(struct inode *inode, struct file *file)
> +{
> + struct kunit_suite *suite;
> +
> + suite = (struct kunit_suite *)inode->i_private;
> +
> + return single_open(file, debugfs_print_run, suite);
> +}
> +
> +/*
> + * Trigger a test suite to run by writing to the suite's "run" debugfs
> + * file found at: /sys/kernel/debug/kunit/<testsuite>/run
> + *
> + * Note: what is written to this file will not be saved.
> + */
> +static ssize_t debugfs_run(struct file *file,
> + const char __user *buf, size_t count, loff_t *ppos)
> +{
> + struct inode *f_inode = file->f_inode;
> + struct kunit_suite *suite = (struct kunit_suite *) f_inode->i_private;
> +
> + __kunit_test_suites_init(&suite, 1);
> +
> + return count;
> +}
> +
> static const struct file_operations debugfs_results_fops = {
> .open = debugfs_results_open,
> .read = seq_read,
> @@ -106,10 +155,22 @@ static const struct file_operations debugfs_results_fops = {
> .release = debugfs_release,
> };
>
> +static const struct file_operations debugfs_run_fops = {
> + .open = debugfs_run_open,
> + .read = seq_read,
> + .write = debugfs_run,
> + .llseek = seq_lseek,
> + .release = debugfs_release,
> +};
> +
> void kunit_debugfs_create_suite(struct kunit_suite *suite)
> {
> struct kunit_case *test_case;
>
> + /* If suite log already allocated, do not create new debugfs files. */
> + if (suite->log)
> + return;
> +
> /* Allocate logs before creating debugfs representation. */
> suite->log = alloc_string_stream(GFP_KERNEL);
> string_stream_set_append_newlines(suite->log, true);
> @@ -124,6 +185,13 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite)
> debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444,
> suite->debugfs,
> suite, &debugfs_results_fops);
> +
> + /* Do not create file to re-run test if test runs on init */
> + if (!suite->attr.is_init) {
> + debugfs_create_file(KUNIT_DEBUGFS_RUN, S_IFREG | 0644,
> + suite->debugfs,
> + suite, &debugfs_run_fops);
> + }
> }
>
> void kunit_debugfs_destroy_suite(struct kunit_suite *suite)
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 8bae6e2bc6a0..58e46bb3b4c4 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -13,6 +13,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> +#include <linux/mutex.h>
> #include <linux/panic.h>
> #include <linux/sched/debug.h>
> #include <linux/sched.h>
> @@ -22,6 +23,8 @@
> #include "string-stream.h"
> #include "try-catch-impl.h"
>
> +static DEFINE_MUTEX(kunit_run_lock);
> +
> /*
> * Hook to fail the current test and print an error message to the log.
> */
> @@ -654,6 +657,7 @@ static void kunit_init_suite(struct kunit_suite *suite)
> kunit_debugfs_create_suite(suite);
> suite->status_comment[0] = '\0';
> suite->suite_init_err = 0;
> + string_stream_clear(suite->log);
> }
>
> bool kunit_enabled(void)
> @@ -670,6 +674,11 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
> return 0;
> }
>
> + /* Use mutex lock to guard against running tests concurrently. */
> + if (mutex_lock_interruptible(&kunit_run_lock)) {
> + pr_err("kunit: test interrupted\n");
> + return -EINTR;
> + }
> static_branch_inc(&kunit_running);
>
> for (i = 0; i < num_suites; i++) {
> @@ -678,6 +687,7 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
> }
>
> static_branch_dec(&kunit_running);
> + mutex_unlock(&kunit_run_lock);
> return 0;
> }
> EXPORT_SYMBOL_GPL(__kunit_test_suites_init);
> --
> 2.43.0.rc2.451.g8631bc7472-goog
>
On Tue, 5 Dec 2023 at 06:19, Rae Moar <[email protected]> wrote:
>
> Expand the documentation on the KUnit debugfs filesystem on the
> run_manual.rst page.
>
> Add section describing how to access results using debugfs.
>
> Add section describing how to run tests after boot using debugfs.
>
> Signed-off-by: Rae Moar <[email protected]>
> ---
Looks good to me, some nitpicks below.
The other thing I'd really want to add is some documentation on
writing init-section suites, which covers the pitfalls better (as
mentioned in the previous emails). Though that could be a separate
patch if you want to keep this one debugfs-specific.
Otherwise,
Reviewed-by: David Gow <[email protected]>
Cheers,
-- David
>
> Changes since v2:
> - Add info to documentation about cleaning up data, init tests, and
> running tests concurrently
>
> Documentation/dev-tools/kunit/run_manual.rst | 49 ++++++++++++++++++--
> 1 file changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/run_manual.rst b/Documentation/dev-tools/kunit/run_manual.rst
> index e7b46421f247..aebb52ba9605 100644
> --- a/Documentation/dev-tools/kunit/run_manual.rst
> +++ b/Documentation/dev-tools/kunit/run_manual.rst
> @@ -49,9 +49,50 @@ loaded.
>
> The results will appear in TAP format in ``dmesg``.
>
> +debugfs
> +=======
> +
> +``debugfs`` is a file system that enables user interaction with the files to
> +make kernel information available to user space (See more information at
> +Documentation/filesystems/debugfs.html)
Nit: reference debugfs.rst here, not debugfs.html -- sphinx ought to
update the link automatically.
Also, maybe we can make this introduction a _little_ bit more
KUnit-specific. I'd personally start by saying that KUnit can be
accessed from userspace via the debugfs filesystem (link), usually
mounted in /sys/kernel/debug/kunit, etc, if CONFIG_KUNIT_DEBUGFS is
enabled.
> +
> +By default, only the root user has access to the debugfs directory.
> +
> +If ``CONFIG_KUNIT_DEBUGFS`` is enabled, you can use KUnit debugfs
> +filesystem to perform the following actions.
> +
> +Retrieve Test Results
> +=====================
> +
> +You can use debugfs to retrieve KUnit test results. The test results are
> +accessible from the debugfs filesystem in the following read-only file:
> +
> +.. code-block :: bash
> +
> + /sys/kernel/debug/kunit/<test_suite>/results
> +
> +The test results are available in KTAP format.
Do we want to note that this is a separate KTAP document, and so may
have different suite numbering from the dmesg log?
> +
> +Run Tests After Kernel Has Booted
> +=================================
> +
> +You can use the debugfs filesystem to trigger built-in tests to run after
> +boot. To run the test suite, you can use the following command to write to
> +the ``/sys/kernel/debug/kunit/<test_suite>/run`` file:
> +
> +.. code-block :: bash
> +
> + echo "any string" > /sys/kernel/debugfs/kunit/<test_suite>/run
> +
> +As a result, the test suite runs and the results are printed to the kernel
> +log.
> +
> +However, this feature is not available with KUnit tests that use init data.
Let's expand this slightly, and mention that this is because the data
may have already been discarded, and that you can find such tests by
either looking for the kunit_test_init_section_suites() macro or the
is_init attribute.
> +
> +Also, you cannot use this feature to run tests concurrently as there is a
> +mutex lock around running KUnit tests at the same time.
> +
Instead of mentioning the mutex, which is an implementation detail,
just mention that tests will either wait for other tests to complete,
or fail having timed out.
> .. note ::
>
> - If ``CONFIG_KUNIT_DEBUGFS`` is enabled, KUnit test results will
> - be accessible from the ``debugfs`` filesystem (if mounted).
> - They will be in ``/sys/kernel/debug/kunit/<test_suite>/results``, in
> - TAP format.
> + For test authors, to use this feature, tests will need to correctly initialise
> + and/or clean up any data, so the test runs correctly a second time.
> --
> 2.43.0.rc2.451.g8631bc7472-goog
>
On Sat, Dec 9, 2023 at 2:48 AM David Gow <[email protected]> wrote:
>
> On Tue, 5 Dec 2023 at 06:19, Rae Moar <[email protected]> wrote:
> >
> > Alter the linker section of KUNIT_TABLE to move it out of INIT_DATA and
> > into DATA_DATA.
> >
> > Data for KUnit tests does not need to be in the init section.
> >
> > In order to run tests again after boot the KUnit data cannot be labeled as
> > init data as the kernel could write over it.
> >
> > Add a KUNIT_INIT_TABLE in the next patch for KUnit tests that test init
> > data/functions.
> >
> > Signed-off-by: Rae Moar <[email protected]>
> > ---
>
> I think this actually fixes a potential bug, as we loop through the
> list of suites after init has ended in the debugfs logic.
>
> So maybe this is:
> Fixes: 90a025a859a3 ("vmlinux.lds.h: add linker section for KUnit test suites")
>
> Regardless, I'd love to get this in, even if we don't manage to get
> the rest of the series in soon.
>
> Reviewed-by: David Gow <[email protected]>
>
> Cheers,
> -- David
Hello!
Thanks for reviewing! I will be adding this fixes tag. Should I make
this a separate patch for the next version?
Thanks!
-Rae
>
> > include/asm-generic/vmlinux.lds.h | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index bae0fe4d499b..1107905d37fc 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -370,7 +370,8 @@
> > BRANCH_PROFILE() \
> > TRACE_PRINTKS() \
> > BPF_RAW_TP() \
> > - TRACEPOINT_STR()
> > + TRACEPOINT_STR() \
> > + KUNIT_TABLE()
> >
> > /*
> > * Data section helpers
> > @@ -699,8 +700,7 @@
> > THERMAL_TABLE(governor) \
> > EARLYCON_TABLE() \
> > LSM_TABLE() \
> > - EARLY_LSM_TABLE() \
> > - KUNIT_TABLE()
> > + EARLY_LSM_TABLE()
> >
> > #define INIT_TEXT \
> > *(.init.text .init.text.*) \
> >
> > base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
> > --
> > 2.43.0.rc2.451.g8631bc7472-goog
> >
On Sat, Dec 9, 2023 at 2:58 AM David Gow <[email protected]> wrote:
>
> On Tue, 5 Dec 2023 at 06:19, Rae Moar <[email protected]> wrote:
> >
> > Expand the documentation on the KUnit debugfs filesystem on the
> > run_manual.rst page.
> >
> > Add section describing how to access results using debugfs.
> >
> > Add section describing how to run tests after boot using debugfs.
> >
> > Signed-off-by: Rae Moar <[email protected]>
> > ---
>
> Looks good to me, some nitpicks below.
>
> The other thing I'd really want to add is some documentation on
> writing init-section suites, which covers the pitfalls better (as
> mentioned in the previous emails). Though that could be a separate
> patch if you want to keep this one debugfs-specific.
>
> Otherwise,
> Reviewed-by: David Gow <[email protected]>
>
> Cheers,
> -- David
Hello!
I have responded to your comments below. I would also be happy to add
documentation to the init-section suites either in this patch series
or in a future one.
Thanks!
-Rae
>
> >
> > Changes since v2:
> > - Add info to documentation about cleaning up data, init tests, and
> > running tests concurrently
> >
> > Documentation/dev-tools/kunit/run_manual.rst | 49 ++++++++++++++++++--
> > 1 file changed, 45 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/dev-tools/kunit/run_manual.rst b/Documentation/dev-tools/kunit/run_manual.rst
> > index e7b46421f247..aebb52ba9605 100644
> > --- a/Documentation/dev-tools/kunit/run_manual.rst
> > +++ b/Documentation/dev-tools/kunit/run_manual.rst
> > @@ -49,9 +49,50 @@ loaded.
> >
> > The results will appear in TAP format in ``dmesg``.
> >
> > +debugfs
> > +=======
> > +
> > +``debugfs`` is a file system that enables user interaction with the files to
> > +make kernel information available to user space (See more information at
> > +Documentation/filesystems/debugfs.html)
>
> Nit: reference debugfs.rst here, not debugfs.html -- sphinx ought to
> update the link automatically.
Thanks for catching this! I didn't realize Sphinx would update it.
>
> Also, maybe we can make this introduction a _little_ bit more
> KUnit-specific. I'd personally start by saying that KUnit can be
> accessed from userspace via the debugfs filesystem (link), usually
> mounted in /sys/kernel/debug/kunit, etc, if CONFIG_KUNIT_DEBUGFS is
> enabled.
Ok I will add this for the next version.
>
> > +
> > +By default, only the root user has access to the debugfs directory.
> > +
> > +If ``CONFIG_KUNIT_DEBUGFS`` is enabled, you can use KUnit debugfs
> > +filesystem to perform the following actions.
> > +
> > +Retrieve Test Results
> > +=====================
> > +
> > +You can use debugfs to retrieve KUnit test results. The test results are
> > +accessible from the debugfs filesystem in the following read-only file:
> > +
> > +.. code-block :: bash
> > +
> > + /sys/kernel/debug/kunit/<test_suite>/results
> > +
> > +The test results are available in KTAP format.
>
> Do we want to note that this is a separate KTAP document, and so may
> have different suite numbering from the dmesg log?
Sure! I will add this for the next version.
>
> > +
> > +Run Tests After Kernel Has Booted
> > +=================================
> > +
> > +You can use the debugfs filesystem to trigger built-in tests to run after
> > +boot. To run the test suite, you can use the following command to write to
> > +the ``/sys/kernel/debug/kunit/<test_suite>/run`` file:
> > +
> > +.. code-block :: bash
> > +
> > + echo "any string" > /sys/kernel/debugfs/kunit/<test_suite>/run
> > +
> > +As a result, the test suite runs and the results are printed to the kernel
> > +log.
> > +
> > +However, this feature is not available with KUnit tests that use init data.
>
> Let's expand this slightly, and mention that this is because the data
> may have already been discarded, and that you can find such tests by
> either looking for the kunit_test_init_section_suites() macro or the
> is_init attribute.
Got it. I will definitely expand this more.
>
> > +
> > +Also, you cannot use this feature to run tests concurrently as there is a
> > +mutex lock around running KUnit tests at the same time.
> > +
>
> Instead of mentioning the mutex, which is an implementation detail,
> just mention that tests will either wait for other tests to complete,
> or fail having timed out.
>
I will definitely switch this out in the next version.
>
>
> > .. note ::
> >
> > - If ``CONFIG_KUNIT_DEBUGFS`` is enabled, KUnit test results will
> > - be accessible from the ``debugfs`` filesystem (if mounted).
> > - They will be in ``/sys/kernel/debug/kunit/<test_suite>/results``, in
> > - TAP format.
> > + For test authors, to use this feature, tests will need to correctly initialise
> > + and/or clean up any data, so the test runs correctly a second time.
> > --
> > 2.43.0.rc2.451.g8631bc7472-goog
> >
On Sat, Dec 9, 2023 at 2:57 AM David Gow <[email protected]> wrote:
>
> On Tue, 5 Dec 2023 at 06:19, Rae Moar <[email protected]> wrote:
> >
> > Add is_init test attribute of type bool. Add to_string, get, and filter
> > methods to lib/kunit/attributes.c.
> >
> > Mark each of the tests in the init section with the is_init=true attribute.
> >
> > Add is_init to the attributes documentation.
> >
> > Signed-off-by: Rae Moar <[email protected]>
> > ---
>
> Would it be possible to not have this in kunit_attributes? I know it's
> required for the run-after-boot stuff later, but I'd love this to be
> (a) just generated at runtime, or (b) stored only at a suite or
> suite-set level. It seems like a bit of a waste to store this
> per-test-case, and to have it potentially accessible or overwritable
> by users.
>
> Otherwise, this looks good (and I appreciate the automatic setting of
> this when merging the suite sets.
>
> Maybe if we always kept the init suites in a separate set, we could
> just use pointer comparisons to generate this; otherwise let's make
> this a suite-level-only attribute (inherited by tests).
>
>
> -- David
Hello!
I did consider making is_init a field within a suite object instead
and then pointing to that in the kunit_attributes framework during
filtering, printing, etc... I will change that out in the next
version.
Thanks!
-Rae