[PATCH kunit 0/3] kunit: add debugfs representation to show results/run tests

[PATCH kunit 0/3] kunit: add debugfs representation to show results/run tests

[Alan Maguire]

When kunit tests are run on native (i.e. non-UML) environments, the results
of test execution are often intermixed with dmesg output.  This patch
series attempts to solve this by providing a debugfs representation
of the results of the last test run, available as

/sys/kernel/debug/kunit/<testsuite>/results

In addition, we provide a way to re-run the tests and show results via

/sys/kernel/debug/kunit/<testsuite>/run

Alan Maguire (3):
  kunit: add debugfs /sys/kernel/debug/kunit/<suite>/results display
  kunit: add "run" debugfs file to run suites, display results
  kunit: update documentation to describe debugfs representation

 Documentation/dev-tools/kunit/usage.rst |  20 +++++
 include/kunit/test.h                    |  21 +++--
 lib/kunit/Makefile                      |   3 +-
 lib/kunit/debugfs.c                     | 145 ++++++++++++++++++++++++++++++++
 lib/kunit/debugfs.h                     |  11 +++
 lib/kunit/test.c                        |  88 ++++++++++++++-----
 6 files changed, 260 insertions(+), 28 deletions(-)
 create mode 100644 lib/kunit/debugfs.c
 create mode 100644 lib/kunit/debugfs.h

-- 
1.8.3.1

[PATCH kunit 1/3] kunit: add debugfs /sys/kernel/debug/kunit/<suite>/results display

[Alan Maguire]

add debugfs support for displaying kunit test suite results; this is
especially useful for module-loaded tests to allow disentangling of
test result display from other dmesg events.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 include/kunit/test.h |  21 +++++++---
 lib/kunit/Makefile   |   3 +-
 lib/kunit/debugfs.c  | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/kunit/debugfs.h  |  11 +++++
 lib/kunit/test.c     |  88 ++++++++++++++++++++++++++++++----------
 5 files changed, 206 insertions(+), 28 deletions(-)
 create mode 100644 lib/kunit/debugfs.c
 create mode 100644 lib/kunit/debugfs.h

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 2dfb550..37219b8a 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -125,6 +125,8 @@ struct kunit_case {
 	bool success;
 };
 
+#define	kunit_status2str(status)	(status ? "ok" : "not ok")
+
 /**
  * KUNIT_CASE - A helper for creating a &struct kunit_case
  *
@@ -157,6 +159,9 @@ struct kunit_suite {
 	int (*init)(struct kunit *test);
 	void (*exit)(struct kunit *test);
 	struct kunit_case *test_cases;
+
+	/* private - internal use only */
+	struct dentry *debugfs;
 };
 
 /**
@@ -197,6 +202,15 @@ struct kunit {
 
 int kunit_run_tests(struct kunit_suite *suite);
 
+size_t kunit_suite_num_test_cases(struct kunit_suite *suite);
+
+unsigned int kunit_test_case_num(struct kunit_suite *suite,
+				 struct kunit_case *test_case);
+
+int __kunit_test_suites_init(struct kunit_suite **suites);
+
+void __kunit_test_suites_exit(struct kunit_suite **suites);
+
 /**
  * kunit_test_suites() - used to register one or more &struct kunit_suite
  *			 with KUnit.
@@ -226,15 +240,12 @@ struct kunit {
 	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;						\
+		return __kunit_test_suites_init(suites);		\
 	}								\
 	late_initcall(kunit_test_suites_init);				\
 	static void __exit kunit_test_suites_exit(void)			\
 	{								\
-		return;							\
+		return __kunit_test_suites_exit(suites);		\
 	}								\
 	module_exit(kunit_test_suites_exit)
 
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index fab5564..869aab0 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -3,7 +3,8 @@ obj-$(CONFIG_KUNIT) +=			kunit.o
 kunit-objs +=				test.o \
 					string-stream.o \
 					assert.o \
-					try-catch.o
+					try-catch.o \
+					debugfs.o
 
 obj-$(CONFIG_KUNIT_TEST) +=		kunit-test.o
 
diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
new file mode 100644
index 0000000..5994f32
--- /dev/null
+++ b/lib/kunit/debugfs.c
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates.
+ *    Author: Alan Maguire <alan.maguire@oracle.com>
+ */
+#include <asm/unistd.h>
+#include <linux/debugfs.h>
+#include <linux/module.h>
+#include <linux/time.h>
+
+#include <uapi/linux/limits.h>
+#include <kunit/test.h>
+
+#include "string-stream.h"
+
+#define KUNIT_DEBUGFS_ROOT             "kunit"
+#define KUNIT_DEBUGFS_RESULTS          "results"
+
+/*
+ * Create a debugfs representation of test suites:
+ *
+ * Path						Semantics
+ * /sys/kernel/debug/kunit/<testsuite>/results	Show results of last run for
+ *						testsuite
+ *
+ */
+
+static struct dentry *debugfs_rootdir;
+
+void debugfs_cleanup(void)
+{
+	debugfs_remove(debugfs_rootdir);
+}
+
+int debugfs_init(void)
+{
+	if (!debugfs_rootdir)
+		debugfs_rootdir = debugfs_create_dir(KUNIT_DEBUGFS_ROOT, NULL);
+	if (IS_ERR(debugfs_rootdir))
+		return PTR_ERR(debugfs_rootdir);
+	return 0;
+}
+
+static void debugfs_print_result(struct seq_file *seq,
+				 struct kunit_suite *suite,
+				 struct kunit_case *test_case)
+{
+	if (!test_case)
+		return;
+
+	seq_printf(seq, "\t%s %d - %s\n", kunit_status2str(test_case->success),
+		   kunit_test_case_num(suite, test_case), test_case->name);
+}
+
+/*
+ * /sys/kernel/debug/kunit/<testsuite>/results shows all results for testsuite.
+ */
+static int debugfs_print_results(struct seq_file *seq, void *v)
+{
+	struct kunit_suite *suite = (struct kunit_suite *)seq->private;
+	struct kunit_case *test_case;
+
+	if (!suite)
+		return 0;
+
+	seq_printf(seq, "\t# Subtest: %s\n", suite->name);
+	seq_printf(seq, "\t1..%zd\n", kunit_suite_num_test_cases(suite));
+
+	for (test_case = suite->test_cases; test_case->run_case; test_case++)
+		debugfs_print_result(seq, suite, test_case);
+
+	return 0;
+}
+
+static int debugfs_release(struct inode *inode, struct file *file)
+{
+	return single_release(inode, file);
+}
+
+static int debugfs_results_open(struct inode *inode, struct file *file)
+{
+	struct kunit_suite *suite;
+
+	suite = (struct kunit_suite *)inode->i_private;
+
+	return single_open(file, debugfs_print_results, suite);
+}
+
+static const struct file_operations debugfs_results_fops = {
+	.open = debugfs_results_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = debugfs_release,
+};
+
+void debugfs_create_suite(struct kunit_suite *suite)
+{
+	/* First add /sys/kernel/debug/kunit/<testsuite> */
+	suite->debugfs = debugfs_create_dir(suite->name, debugfs_rootdir);
+	if (IS_ERR(suite->debugfs))
+		return;
+
+	debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444,
+			    suite->debugfs,
+			    suite, &debugfs_results_fops);
+}
+
+void debugfs_destroy_suite(struct kunit_suite *suite)
+{
+	debugfs_remove_recursive(suite->debugfs);
+}
diff --git a/lib/kunit/debugfs.h b/lib/kunit/debugfs.h
new file mode 100644
index 0000000..d74e8a4
--- /dev/null
+++ b/lib/kunit/debugfs.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020, Oracle and/or its affiliates.
+ */
+
+#include <kunit/test.h>
+
+void debugfs_create_suite(struct kunit_suite *suite);
+void debugfs_destroy_suite(struct kunit_suite *suite);
+int debugfs_init(void);
+void debugfs_cleanup(void);
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 9242f93..efb05ba 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -10,6 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/sched/debug.h>
 
+#include "debugfs.h"
 #include "string-stream.h"
 #include "try-catch-impl.h"
 
@@ -28,22 +29,24 @@ static void kunit_print_tap_version(void)
 	}
 }
 
-static size_t kunit_test_cases_len(struct kunit_case *test_cases)
+size_t kunit_suite_num_test_cases(struct kunit_suite *suite)
 {
-	struct kunit_case *test_case;
+	struct kunit_case *test_case, *test_cases = suite->test_cases;
 	size_t len = 0;
 
-	for (test_case = test_cases; test_case->run_case; test_case++)
+	for (test_case = test_cases; test_case && test_case->run_case;
+	     test_case++)
 		len++;
 
 	return len;
 }
+EXPORT_SYMBOL_GPL(kunit_suite_num_test_cases);
 
 static void kunit_print_subtest_start(struct kunit_suite *suite)
 {
 	kunit_print_tap_version();
 	pr_info("\t# Subtest: %s\n", suite->name);
-	pr_info("\t1..%zd\n", kunit_test_cases_len(suite->test_cases));
+	pr_info("\t1..%zd\n", kunit_suite_num_test_cases(suite));
 }
 
 static void kunit_print_ok_not_ok(bool should_indent,
@@ -51,19 +54,15 @@ static void kunit_print_ok_not_ok(bool should_indent,
 				  size_t test_number,
 				  const char *description)
 {
-	const char *indent, *ok_not_ok;
+	const char *indent;
 
 	if (should_indent)
 		indent = "\t";
 	else
 		indent = "";
 
-	if (is_ok)
-		ok_not_ok = "ok";
-	else
-		ok_not_ok = "not ok";
-
-	pr_info("%s%s %zd - %s\n", indent, ok_not_ok, test_number, description);
+	pr_info("%s%s %zd - %s\n", indent, kunit_status2str(is_ok),
+		test_number, description);
 }
 
 static bool kunit_suite_has_succeeded(struct kunit_suite *suite)
@@ -87,14 +86,20 @@ static void kunit_print_subtest_end(struct kunit_suite *suite)
 			      suite->name);
 }
 
-static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case,
-					    size_t test_number)
+unsigned int kunit_test_case_num(struct kunit_suite *suite,
+				 struct kunit_case *test_case)
 {
-	kunit_print_ok_not_ok(true,
-			      test_case->success,
-			      test_number,
-			      test_case->name);
+	struct kunit_case *tc;
+	unsigned int i;
+
+	for (i = 1, tc = suite->test_cases; tc->run_case; tc++, i++) {
+		if (tc == test_case)
+			return i;
+	}
+
+	return 0;
 }
+EXPORT_SYMBOL_GPL(kunit_test_case_num);
 
 static void kunit_print_string_stream(struct kunit *test,
 				      struct string_stream *stream)
@@ -102,6 +107,9 @@ static void kunit_print_string_stream(struct kunit *test,
 	struct string_stream_fragment *fragment;
 	char *buf;
 
+	if (string_stream_is_empty(stream))
+		return;
+
 	buf = string_stream_get_string(stream);
 	if (!buf) {
 		kunit_err(test,
@@ -303,19 +311,20 @@ static void kunit_run_case_catch_errors(struct kunit_suite *suite,
 	kunit_try_catch_run(try_catch, &context);
 
 	test_case->success = test.success;
+
+	kunit_print_ok_not_ok(true, test_case->success,
+			      kunit_test_case_num(suite, test_case),
+			      test_case->name);
 }
 
 int kunit_run_tests(struct kunit_suite *suite)
 {
 	struct kunit_case *test_case;
-	size_t test_case_count = 1;
 
 	kunit_print_subtest_start(suite);
 
-	for (test_case = suite->test_cases; test_case->run_case; test_case++) {
+	for (test_case = suite->test_cases; test_case->run_case; test_case++)
 		kunit_run_case_catch_errors(suite, test_case);
-		kunit_print_test_case_ok_not_ok(test_case, test_case_count++);
-	}
 
 	kunit_print_subtest_end(suite);
 
@@ -323,6 +332,37 @@ int kunit_run_tests(struct kunit_suite *suite)
 }
 EXPORT_SYMBOL_GPL(kunit_run_tests);
 
+static void kunit_init_suite(struct kunit_suite *suite)
+{
+	debugfs_create_suite(suite);
+}
+
+int __kunit_test_suites_init(struct kunit_suite **suites)
+{
+	unsigned int i;
+
+	for (i = 0; suites[i] != NULL; i++) {
+		kunit_init_suite(suites[i]);
+		kunit_run_tests(suites[i]);
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__kunit_test_suites_init);
+
+static void kunit_exit_suite(struct kunit_suite *suite)
+{
+	debugfs_destroy_suite(suite);
+}
+
+void __kunit_test_suites_exit(struct kunit_suite **suites)
+{
+	unsigned int i;
+
+	for (i = 0; suites[i] != NULL; i++)
+		kunit_exit_suite(suites[i]);
+}
+EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
+
 struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test,
 						    kunit_resource_init_t init,
 						    kunit_resource_free_t free,
@@ -398,6 +438,9 @@ int kunit_resource_destroy(struct kunit *test,
 {
 	struct kunit_resource *resource;
 
+	if (!test)
+		kfree(match_data);
+
 	resource = kunit_resource_remove(test, match, free, match_data);
 
 	if (!resource)
@@ -489,12 +532,13 @@ void kunit_cleanup(struct kunit *test)
 
 static int __init kunit_init(void)
 {
-	return 0;
+	return debugfs_init();
 }
 late_initcall(kunit_init);
 
 static void __exit kunit_exit(void)
 {
+	debugfs_cleanup();
 }
 module_exit(kunit_exit);
 
-- 
1.8.3.1

[PATCH kunit 2/3] kunit: add "run" debugfs file to run suites, display results

[Alan Maguire]

Add /sys/kernel/debug/kunit/<suite>/run file which will run the
specified suite and show results.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 lib/kunit/debugfs.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
index 5994f32..4881261 100644
--- a/lib/kunit/debugfs.c
+++ b/lib/kunit/debugfs.c
@@ -15,6 +15,7 @@
 
 #define KUNIT_DEBUGFS_ROOT             "kunit"
 #define KUNIT_DEBUGFS_RESULTS          "results"
+#define KUNIT_DEBUGFS_RUN		"run"
 
 /*
  * Create a debugfs representation of test suites:
@@ -23,6 +24,8 @@
  * /sys/kernel/debug/kunit/<testsuite>/results	Show results of last run for
  *						testsuite
  *
+ * /sys/kernel/debug/kunit/<testsuite>/run	Run testsuite and show results
+ *
  */
 
 static struct dentry *debugfs_rootdir;
@@ -72,6 +75,18 @@ static int debugfs_print_results(struct seq_file *seq, void *v)
 	return 0;
 }
 
+/*
+ * /sys/kernel/debug/kunit/<testsuite>/run (re)runs suite and shows all results.
+ */
+static int debugfs_run_print_results(struct seq_file *seq, void *v)
+{
+	struct kunit_suite *suite = (struct kunit_suite *)seq->private;
+
+	kunit_run_tests(suite);
+
+	return debugfs_print_results(seq, v);
+}
+
 static int debugfs_release(struct inode *inode, struct file *file)
 {
 	return single_release(inode, file);
@@ -93,6 +108,22 @@ static int debugfs_results_open(struct inode *inode, struct file *file)
 	.release = debugfs_release,
 };
 
+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_run_print_results, suite);
+}
+
+static const struct file_operations debugfs_run_fops = {
+	.open = debugfs_run_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = debugfs_release,
+};
+
 void debugfs_create_suite(struct kunit_suite *suite)
 {
 	/* First add /sys/kernel/debug/kunit/<testsuite> */
@@ -103,6 +134,9 @@ void 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 | 0444,
+			    suite->debugfs,
+			    suite, &debugfs_run_fops);
 }
 
 void debugfs_destroy_suite(struct kunit_suite *suite)
-- 
1.8.3.1

[PATCH kunit 3/3] kunit: update documentation to describe debugfs representation

[Alan Maguire]

Documentation should describe debugfs layout and semantics.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 Documentation/dev-tools/kunit/usage.rst | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index 7cd56a1..b1ebf9f 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -590,3 +590,23 @@ able to run one test case per invocation.
 
 .. TODO(brendanhiggins@google.com): Add an actual example of an architecture
    dependent KUnit test.
+
+KUnit debugfs representation
+============================
+When kunit test suites are initialized, they create an associated directory
+in /sys/kernel/debug/kunit/<test-suite>.  The directory contains two files
+
+- results: "cat results" displays results of last test run
+- run: "cat run" runs the test suite and displays the results
+
+Thus to re-run all currently loaded suites and display results, we can do this:
+
+```
+$ cat /sys/kernel/debug/kunit/*/run
+```
+
+The debugfs representation is primarily of use when kunit test suites are
+run in a native environment, either as modules or builtin.  Having a way
+to display results like this is valuable as otherwise results can be
+intermixed with other events in dmesg output.
+
-- 
1.8.3.1

Re: [PATCH kunit 1/3] kunit: add debugfs /sys/kernel/debug/kunit/<suite>/results display

[Greg KH]

On Thu, Jan 23, 2020 at 02:47:18PM +0000, Alan Maguire wrote:
> add debugfs support for displaying kunit test suite results; this is
> especially useful for module-loaded tests to allow disentangling of
> test result display from other dmesg events.
> 
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  include/kunit/test.h |  21 +++++++---
>  lib/kunit/Makefile   |   3 +-
>  lib/kunit/debugfs.c  | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/kunit/debugfs.h  |  11 +++++
>  lib/kunit/test.c     |  88 ++++++++++++++++++++++++++++++----------
>  5 files changed, 206 insertions(+), 28 deletions(-)
>  create mode 100644 lib/kunit/debugfs.c
>  create mode 100644 lib/kunit/debugfs.h
> 
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 2dfb550..37219b8a 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -125,6 +125,8 @@ struct kunit_case {
>  	bool success;
>  };
>  
> +#define	kunit_status2str(status)	(status ? "ok" : "not ok")
> +
>  /**
>   * KUNIT_CASE - A helper for creating a &struct kunit_case
>   *
> @@ -157,6 +159,9 @@ struct kunit_suite {
>  	int (*init)(struct kunit *test);
>  	void (*exit)(struct kunit *test);
>  	struct kunit_case *test_cases;
> +
> +	/* private - internal use only */
> +	struct dentry *debugfs;
>  };
>  
>  /**
> @@ -197,6 +202,15 @@ struct kunit {
>  
>  int kunit_run_tests(struct kunit_suite *suite);
>  
> +size_t kunit_suite_num_test_cases(struct kunit_suite *suite);
> +
> +unsigned int kunit_test_case_num(struct kunit_suite *suite,
> +				 struct kunit_case *test_case);
> +
> +int __kunit_test_suites_init(struct kunit_suite **suites);
> +
> +void __kunit_test_suites_exit(struct kunit_suite **suites);
> +
>  /**
>   * kunit_test_suites() - used to register one or more &struct kunit_suite
>   *			 with KUnit.
> @@ -226,15 +240,12 @@ struct kunit {
>  	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;						\
> +		return __kunit_test_suites_init(suites);		\
>  	}								\
>  	late_initcall(kunit_test_suites_init);				\
>  	static void __exit kunit_test_suites_exit(void)			\
>  	{								\
> -		return;							\
> +		return __kunit_test_suites_exit(suites);		\
>  	}								\
>  	module_exit(kunit_test_suites_exit)
>  
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index fab5564..869aab0 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -3,7 +3,8 @@ obj-$(CONFIG_KUNIT) +=			kunit.o
>  kunit-objs +=				test.o \
>  					string-stream.o \
>  					assert.o \
> -					try-catch.o
> +					try-catch.o \
> +					debugfs.o
>  
>  obj-$(CONFIG_KUNIT_TEST) +=		kunit-test.o
>  
> diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
> new file mode 100644
> index 0000000..5994f32
> --- /dev/null
> +++ b/lib/kunit/debugfs.c
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020, Oracle and/or its affiliates.
> + *    Author: Alan Maguire <alan.maguire@oracle.com>
> + */
> +#include <asm/unistd.h>

Why do you need this asm include file?

> +#include <linux/debugfs.h>
> +#include <linux/module.h>
> +#include <linux/time.h>
> +
> +#include <uapi/linux/limits.h>
> +#include <kunit/test.h>
> +
> +#include "string-stream.h"
> +
> +#define KUNIT_DEBUGFS_ROOT             "kunit"
> +#define KUNIT_DEBUGFS_RESULTS          "results"
> +
> +/*
> + * Create a debugfs representation of test suites:
> + *
> + * Path						Semantics
> + * /sys/kernel/debug/kunit/<testsuite>/results	Show results of last run for
> + *						testsuite
> + *
> + */
> +
> +static struct dentry *debugfs_rootdir;
> +
> +void debugfs_cleanup(void)

Can you prefix all of your global symbols here with "kunit_debugfs"
instead of just "debugfs" to show that this really is not the core
debugfs kernel code?

thanks,

greg k-h

Re: [PATCH kunit 1/3] kunit: add debugfs /sys/kernel/debug/kunit/<suite>/results display

[Greg KH]

On Thu, Jan 23, 2020 at 02:47:18PM +0000, Alan Maguire wrote:
> +int debugfs_init(void)
> +{
> +	if (!debugfs_rootdir)
> +		debugfs_rootdir = debugfs_create_dir(KUNIT_DEBUGFS_ROOT, NULL);
> +	if (IS_ERR(debugfs_rootdir))
> +		return PTR_ERR(debugfs_rootdir);
> +	return 0;

No, you never care if a debugfs call works or not, just call it and move
on.  So just create the directory, and then pass it into whatever
debugfs call you want, never test it or do anything about it.  This
function can be void as well.

> +void debugfs_create_suite(struct kunit_suite *suite)
> +{
> +	/* First add /sys/kernel/debug/kunit/<testsuite> */
> +	suite->debugfs = debugfs_create_dir(suite->name, debugfs_rootdir);
> +	if (IS_ERR(suite->debugfs))
> +		return;

Same here, don't test, just call and move on.

thanks,

greg k-h