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.
Note that what you "write" to the debugfs file will not be saved.
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 functionality may not work for all tests.
This new functionality could be used to design a parameter injection feature in the future.
Signed-off-by: Rae Moar rmoar@google.com ---
Changes since v1: - Removed second patch as this problem has been fixed - Added Documentation patch - Made changes to work with new dynamically-extending log feature
Note that these patches now rely on (and are rebased on) the patch series: https://lore.kernel.org/all/20230828104111.2394344-1-rf@opensource.cirrus.co...
lib/kunit/debugfs.c | 66 +++++++++++++++++++++++++++++++++++++++++++++ lib/kunit/test.c | 13 +++++++++ 2 files changed, 79 insertions(+)
diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c index 270d185737e6..8c0a970321ce 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,23 @@ 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) { + /* Clear the suite log that's leftover from a previous run. */ + string_stream_clear(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 +186,10 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite) debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444, suite->debugfs, suite, &debugfs_results_fops); + + 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 651cbda9f250..d376b886d72d 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 struct mutex kunit_run_lock; + /* * Hook to fail the current test and print an error message to the log. */ @@ -668,6 +671,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++) { @@ -676,6 +684,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); @@ -836,6 +845,10 @@ static int __init kunit_init(void) kunit_install_hooks();
kunit_debugfs_init(); + + /* Initialize lock to guard against running tests concurrently. */ + mutex_init(&kunit_run_lock); + #ifdef CONFIG_MODULES return register_module_notifier(&kunit_mod_nb); #else
base-commit: b754593274e04fc840482a658b29791bc8f8b933
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 rmoar@google.com Co-developed-by: Sadiya Kazi sadiyakazi@google.com Signed-off-by: Sadiya Kazi sadiyakazi@google.com --- Documentation/dev-tools/kunit/run_manual.rst | 45 ++++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-)
diff --git a/Documentation/dev-tools/kunit/run_manual.rst b/Documentation/dev-tools/kunit/run_manual.rst index e7b46421f247..613385c5ba5b 100644 --- a/Documentation/dev-tools/kunit/run_manual.rst +++ b/Documentation/dev-tools/kunit/run_manual.rst @@ -49,9 +49,46 @@ 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. A user can interact with +the debugfs filesystem using a variety of file operations, such as open, +read, and write. + +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. + .. 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. + The contents written to the debugfs file + ``/sys/kernel/debug/kunit/<test_suite>/run`` are not saved.
On Sat, 9 Sept 2023 at 05:32, Rae Moar rmoar@google.com 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 rmoar@google.com Co-developed-by: Sadiya Kazi sadiyakazi@google.com Signed-off-by: Sadiya Kazi sadiyakazi@google.com
Looks good to me, a few nitpicks, and the fact that we'll probably need to add something about init section suites when those are implemented.
(Also, since you sent the email, your sign off should be at the bottom of the list above.)
Documentation/dev-tools/kunit/run_manual.rst | 45 ++++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-)
diff --git a/Documentation/dev-tools/kunit/run_manual.rst b/Documentation/dev-tools/kunit/run_manual.rst index e7b46421f247..613385c5ba5b 100644 --- a/Documentation/dev-tools/kunit/run_manual.rst +++ b/Documentation/dev-tools/kunit/run_manual.rst @@ -49,9 +49,46 @@ 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. A user can interact with +the debugfs filesystem using a variety of file operations, such as open, +read, and write.
+By default, only the root user has access to the debugfs directory.
These two paragraphs are probably a bit excessive: we want to focus on what KUnit can do with debugfs, not describing what debugfs is as a whole (which is best left to pages like Documentation/filesystems/debugfs.rst )
+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.
We _could_ mention that this is a separate KTAP document (i.e., the numbering starts at 1), though it may be obvious.
+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.
.. note ::
If ``CONFIG_KUNIT_DEBUGFS`` is enabled, KUnit test results willbe accessible from the ``debugfs`` filesystem (if mounted).They will be in ``/sys/kernel/debug/kunit/<test_suite>/results``, inTAP format.
The contents written to the debugfs file``/sys/kernel/debug/kunit/<test_suite>/run`` are not saved.
This is possibly a bit obvious. Maybe it'd be more useful with a bit more context, e.g., "The contents written to the file ... are discarded; it is the act of writing which triggers the test, not the specific contents written."?
It might be worth having a note that tests cannot run concurrently, so this may block or fail.
Equally, it may be worth having a note for test authors, that their tests will need to correctly initialise and/or clean up any data, so the test runs correctly a second time.
-- 2.42.0.283.g2d96d420d3-goog
On Thu, Sep 14, 2023 at 5:06 AM David Gow davidgow@google.com wrote:
On Sat, 9 Sept 2023 at 05:32, Rae Moar rmoar@google.com 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 rmoar@google.com Co-developed-by: Sadiya Kazi sadiyakazi@google.com Signed-off-by: Sadiya Kazi sadiyakazi@google.com
Looks good to me, a few nitpicks, and the fact that we'll probably need to add something about init section suites when those are implemented.
(Also, since you sent the email, your sign off should be at the bottom of the list above.)
Hello!
Thanks for the comments! Sorry about the Signed-off order. I will change that for next time.
Documentation/dev-tools/kunit/run_manual.rst | 45 ++++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-)
diff --git a/Documentation/dev-tools/kunit/run_manual.rst b/Documentation/dev-tools/kunit/run_manual.rst index e7b46421f247..613385c5ba5b 100644 --- a/Documentation/dev-tools/kunit/run_manual.rst +++ b/Documentation/dev-tools/kunit/run_manual.rst @@ -49,9 +49,46 @@ 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. A user can interact with +the debugfs filesystem using a variety of file operations, such as open, +read, and write.
+By default, only the root user has access to the debugfs directory.
These two paragraphs are probably a bit excessive: we want to focus on what KUnit can do with debugfs, not describing what debugfs is as a whole (which is best left to pages like Documentation/filesystems/debugfs.rst )
Got it. Maybe I should just leave the first sentence and then link to ../debugfs.rst.
+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.
We _could_ mention that this is a separate KTAP document (i.e., the numbering starts at 1), though it may be obvious.
+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.
.. note ::
If ``CONFIG_KUNIT_DEBUGFS`` is enabled, KUnit test results willbe accessible from the ``debugfs`` filesystem (if mounted).They will be in ``/sys/kernel/debug/kunit/<test_suite>/results``, inTAP format.
The contents written to the debugfs file``/sys/kernel/debug/kunit/<test_suite>/run`` are not saved.This is possibly a bit obvious. Maybe it'd be more useful with a bit more context, e.g., "The contents written to the file ... are discarded; it is the act of writing which triggers the test, not the specific contents written."?
I will try to add more context here in the next version.
It might be worth having a note that tests cannot run concurrently, so this may block or fail.
Equally, it may be worth having a note for test authors, that their tests will need to correctly initialise and/or clean up any data, so the test runs correctly a second time.
Yes these are two good points. I will add notes on tests not being able to run concurrently, cleaning up data, and also init tests.
-- 2.42.0.283.g2d96d420d3-goog
On Sat, 9 Sept 2023 at 05:31, Rae Moar rmoar@google.com 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.
Note that what you "write" to the debugfs file will not be saved.
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 functionality may not work for all tests.
This new functionality could be used to design a parameter injection feature in the future.
Signed-off-by: Rae Moar rmoar@google.com
This is looking pretty good, but I have a few nitpicks below and one big issue.
The big issue is that this doesn't seem to exclude test suites created with kunit_test_init_section_suite{,s}(). The init section versions of the suite declarations, by definition, won't work if run after the kernel has finished booting. At the moment, these macros just pass through to the normal versions (because we've not been able to run after boot until now), but we'll need to implement it (maybe as a separate linker section, maybe as an attribute, etc) now. I expect that the correct solution here would be to not create the 'run' debugfs file for these tests. But I could be convinced to have it exist, but to just say "this test cannot be run after boot" if you've got a good argument. In any case, grep 'test.h' for "NOTE TO KUNIT DEVS" and you'll see the details.
My one other not-totally-related thought (and this extends to module loading, too, so is possibly more useful as a separate patch) is that we're continually incrementing the test number still. This doesn't matter if we read the results from debugfs though, and it may make more sense to have this continue to increment (and thus treat all of dmesg as one long KTAP document). We could always add a reset option to debugfs in a follow-up patch if we want. But that's not something I'd hold this up with.
Changes since v1:
- Removed second patch as this problem has been fixed
- Added Documentation patch
- Made changes to work with new dynamically-extending log feature
Note that these patches now rely on (and are rebased on) the patch series: https://lore.kernel.org/all/20230828104111.2394344-1-rf@opensource.cirrus.co...
lib/kunit/debugfs.c | 66 +++++++++++++++++++++++++++++++++++++++++++++ lib/kunit/test.c | 13 +++++++++ 2 files changed, 79 insertions(+)
diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c index 270d185737e6..8c0a970321ce 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,23 @@ 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) {/* Clear the suite log that's leftover from a previous run. */string_stream_clear(suite->log);return;}
Can we just move this to kunit_init_suite() in test.c. It doesn't feel quite debugfs-y enough, and the return here tripped me up for a little too long.
Ideally, we'd then split up kunit_init_suite() into a one-time initialisation (which calls kunit_debugfs_create_suite()), and a reset function (which resets the state of the suite back to the beginning). We then only call init once, but reset on every execution.
/* Allocate logs before creating debugfs representation. */ suite->log = alloc_string_stream(GFP_KERNEL); string_stream_set_append_newlines(suite->log, true);@@ -124,6 +186,10 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite) debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444, suite->debugfs, suite, &debugfs_results_fops);
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 651cbda9f250..d376b886d72d 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 struct mutex kunit_run_lock;
Should we use DEFINE_MUTEX() here rather than initialising it at runtime?
/*
- Hook to fail the current test and print an error message to the log.
*/ @@ -668,6 +671,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++) {@@ -676,6 +684,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); @@ -836,6 +845,10 @@ static int __init kunit_init(void) kunit_install_hooks();
kunit_debugfs_init();
/* Initialize lock to guard against running tests concurrently. */mutex_init(&kunit_run_lock);
As I understand it, we can just use DEFINE_MUTEX() above.
#ifdef CONFIG_MODULES return register_module_notifier(&kunit_mod_nb); #else
base-commit: b754593274e04fc840482a658b29791bc8f8b933
2.42.0.283.g2d96d420d3-goog
On Thu, Sep 14, 2023 at 5:06 AM David Gow davidgow@google.com wrote:
On Sat, 9 Sept 2023 at 05:31, Rae Moar rmoar@google.com 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.
Note that what you "write" to the debugfs file will not be saved.
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 functionality may not work for all tests.
This new functionality could be used to design a parameter injection feature in the future.
Signed-off-by: Rae Moar rmoar@google.com
This is looking pretty good, but I have a few nitpicks below and one big issue.
The big issue is that this doesn't seem to exclude test suites created with kunit_test_init_section_suite{,s}(). The init section versions of the suite declarations, by definition, won't work if run after the kernel has finished booting. At the moment, these macros just pass through to the normal versions (because we've not been able to run after boot until now), but we'll need to implement it (maybe as a separate linker section, maybe as an attribute, etc) now. I expect that the correct solution here would be to not create the 'run' debugfs file for these tests. But I could be convinced to have it exist, but to just say "this test cannot be run after boot" if you've got a good argument. In any case, grep 'test.h' for "NOTE TO KUNIT DEVS" and you'll see the details.
My one other not-totally-related thought (and this extends to module loading, too, so is possibly more useful as a separate patch) is that we're continually incrementing the test number still. This doesn't matter if we read the results from debugfs though, and it may make more sense to have this continue to increment (and thus treat all of dmesg as one long KTAP document). We could always add a reset option to debugfs in a follow-up patch if we want. But that's not something I'd hold this up with.
Hello!
Sorry for the delay in this response. I was working on other items but I have started working on the next version of this patch.
Thanks for bringing my attention to the init tests. I am currently working on a draft to remove the run files for these tests. However, if that does not work, I will resort to outputting the message you detailed above: "this test cannot be run after boot".
I am currently fine with the test number incrementing. However, I would also be fine to implement a reset to ensure all the re-run results have the test number of 1.
Changes since v1:
- Removed second patch as this problem has been fixed
- Added Documentation patch
- Made changes to work with new dynamically-extending log feature
Note that these patches now rely on (and are rebased on) the patch series: https://lore.kernel.org/all/20230828104111.2394344-1-rf@opensource.cirrus.co...
lib/kunit/debugfs.c | 66 +++++++++++++++++++++++++++++++++++++++++++++ lib/kunit/test.c | 13 +++++++++ 2 files changed, 79 insertions(+)
diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c index 270d185737e6..8c0a970321ce 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,23 @@ 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) {/* Clear the suite log that's leftover from a previous run. */string_stream_clear(suite->log);return;}Can we just move this to kunit_init_suite() in test.c. It doesn't feel quite debugfs-y enough, and the return here tripped me up for a little too long.
Ideally, we'd then split up kunit_init_suite() into a one-time initialisation (which calls kunit_debugfs_create_suite()), and a reset function (which resets the state of the suite back to the beginning). We then only call init once, but reset on every execution.
I definitely think you are right here to move this into test.c. I will try to put this into a reset function.
/* Allocate logs before creating debugfs representation. */ suite->log = alloc_string_stream(GFP_KERNEL); string_stream_set_append_newlines(suite->log, true);@@ -124,6 +186,10 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite) debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444, suite->debugfs, suite, &debugfs_results_fops);
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 651cbda9f250..d376b886d72d 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 struct mutex kunit_run_lock;
Should we use DEFINE_MUTEX() here rather than initialising it at runtime?
I will try to implement this using DEFINE_MUTEX.
/*
- Hook to fail the current test and print an error message to the log.
*/ @@ -668,6 +671,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++) {@@ -676,6 +684,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); @@ -836,6 +845,10 @@ static int __init kunit_init(void) kunit_install_hooks();
kunit_debugfs_init();
/* Initialize lock to guard against running tests concurrently. */mutex_init(&kunit_run_lock);As I understand it, we can just use DEFINE_MUTEX() above.
#ifdef CONFIG_MODULES return register_module_notifier(&kunit_mod_nb); #else
base-commit: b754593274e04fc840482a658b29791bc8f8b933
2.42.0.283.g2d96d420d3-goog
linux-kselftest-mirror@lists.linaro.org
-
David Gow -
Rae Moar