* [PATCH v7 09/18] kunit: test: add support for test abort
From: Brendan Higgins @ 2019-07-09 6:30 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190709063023.251446-1-brendanhiggins@google.com>
Add support for aborting/bailing out of test cases, which is needed for
implementing assertions.
An assertion is like an expectation, but bails out of the test case
early if the assertion is not met. The idea with assertions is that you
use them to state all the preconditions for your test. Logically
speaking, these are the premises of the test case, so if a premise isn't
true, there is no point in continuing the test case because there are no
conclusions that can be drawn without the premises. Whereas, the
expectation is the thing you are trying to prove.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
include/kunit/test.h | 16 ++++
include/kunit/try-catch.h | 69 +++++++++++++++
kunit/Makefile | 3 +-
kunit/test.c | 176 +++++++++++++++++++++++++++++++++++---
kunit/try-catch.c | 95 ++++++++++++++++++++
5 files changed, 344 insertions(+), 15 deletions(-)
create mode 100644 include/kunit/try-catch.h
create mode 100644 kunit/try-catch.c
diff --git a/include/kunit/test.h b/include/kunit/test.h
index df18765e6ea25..01440f3569847 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -13,6 +13,7 @@
#include <linux/types.h>
#include <linux/slab.h>
#include <kunit/kunit-stream.h>
+#include <kunit/try-catch.h>
struct kunit_resource;
@@ -167,6 +168,7 @@ struct kunit {
/* private: internal use only. */
const char *name; /* Read only after initialization! */
+ struct kunit_try_catch try_catch;
/*
* success starts as true, and may only be set to false during a test
* case; thus, it is safe to update this across multiple threads using
@@ -176,6 +178,11 @@ struct kunit {
*/
bool success; /* Read only after test_case finishes! */
struct mutex lock; /* Gaurds all mutable test state. */
+ /*
+ * death_test may be both set and unset from multiple threads in a test
+ * case.
+ */
+ bool death_test; /* Protected by lock. */
/*
* Because resources is a list that may be updated multiple times (with
* new resources) from any thread associated with a test case, we must
@@ -184,10 +191,19 @@ struct kunit {
struct list_head resources; /* Protected by lock. */
};
+static inline void kunit_set_death_test(struct kunit *test, bool death_test)
+{
+ mutex_lock(&test->lock);
+ test->death_test = death_test;
+ mutex_unlock(&test->lock);
+}
+
void kunit_init_test(struct kunit *test, const char *name);
void kunit_fail(struct kunit *test, struct kunit_stream *stream);
+void kunit_abort(struct kunit *test);
+
int kunit_run_tests(struct kunit_suite *suite);
/**
diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h
new file mode 100644
index 0000000000000..8a414a9af0b64
--- /dev/null
+++ b/include/kunit/try-catch.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * An API to allow a function, that may fail, to be executed, and recover in a
+ * controlled manner.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#ifndef _KUNIT_TRY_CATCH_H
+#define _KUNIT_TRY_CATCH_H
+
+#include <linux/types.h>
+
+typedef void (*kunit_try_catch_func_t)(void *);
+
+struct kunit;
+
+/*
+ * struct kunit_try_catch - provides a generic way to run code which might fail.
+ * @context: used to pass user data to the try and catch functions.
+ *
+ * kunit_try_catch provides a generic, architecture independent way to execute
+ * an arbitrary function of type kunit_try_catch_func_t which may bail out by
+ * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called, @try
+ * is stopped at the site of invocation and @catch is catch is called.
+ *
+ * struct kunit_try_catch provides a generic interface for the functionality
+ * needed to implement kunit->abort() which in turn is needed for implementing
+ * assertions. Assertions allow stating a precondition for a test simplifying
+ * how test cases are written and presented.
+ *
+ * Assertions are like expectations, except they abort (call
+ * kunit_try_catch_throw()) when the specified condition is not met. This is
+ * useful when you look at a test case as a logical statement about some piece
+ * of code, where assertions are the premises for the test case, and the
+ * conclusion is a set of predicates, rather expectations, that must all be
+ * true. If your premises are violated, it does not makes sense to continue.
+ */
+struct kunit_try_catch {
+ /* private: internal use only. */
+ struct kunit *test;
+ struct completion *try_completion;
+ int try_result;
+ kunit_try_catch_func_t try;
+ kunit_try_catch_func_t catch;
+ void *context;
+};
+
+void kunit_try_catch_init(struct kunit_try_catch *try_catch,
+ struct kunit *test,
+ kunit_try_catch_func_t try,
+ kunit_try_catch_func_t catch);
+
+void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context);
+
+void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch);
+
+static inline int kunit_try_catch_get_result(struct kunit_try_catch *try_catch)
+{
+ return try_catch->try_result;
+}
+
+/*
+ * Exposed for testing only.
+ */
+void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch);
+
+#endif /* _KUNIT_TRY_CATCH_H */
diff --git a/kunit/Makefile b/kunit/Makefile
index 60a9ea6cb4697..1f7680cfa11ad 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1,6 +1,7 @@
obj-$(CONFIG_KUNIT) += test.o \
string-stream.o \
- kunit-stream.o
+ kunit-stream.o \
+ try-catch.o
obj-$(CONFIG_KUNIT_TEST) += string-stream-test.o
diff --git a/kunit/test.c b/kunit/test.c
index 8c745fd0c82d3..731f3c9eecf20 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -7,13 +7,26 @@
*/
#include <linux/kernel.h>
+#include <linux/sched/debug.h>
#include <kunit/test.h>
+#include <kunit/try-catch.h>
static void kunit_set_failure(struct kunit *test)
{
WRITE_ONCE(test->success, false);
}
+static bool kunit_get_death_test(struct kunit *test)
+{
+ bool death_test;
+
+ mutex_lock(&test->lock);
+ death_test = test->death_test;
+ mutex_unlock(&test->lock);
+
+ return death_test;
+}
+
static int kunit_vprintk_emit(int level, const char *fmt, va_list args)
{
return vprintk_emit(0, level, NULL, 0, fmt, args);
@@ -126,40 +139,175 @@ void kunit_fail(struct kunit *test, struct kunit_stream *stream)
kunit_stream_commit(stream);
}
+void __noreturn kunit_abort(struct kunit *test)
+{
+ kunit_set_death_test(test, true);
+
+ kunit_try_catch_throw(&test->try_catch);
+
+ /*
+ * Throw could not abort from test.
+ *
+ * XXX: we should never reach this line! As kunit_try_catch_throw is
+ * marked __noreturn.
+ */
+ WARN_ONCE(true, "Throw could not abort from test!\n");
+}
+
void kunit_init_test(struct kunit *test, const char *name)
{
mutex_init(&test->lock);
INIT_LIST_HEAD(&test->resources);
test->name = name;
test->success = true;
+ test->death_test = false;
}
/*
- * Performs all logic to run a test case.
+ * Initializes and runs test case. Does not clean up or do post validations.
*/
-static void kunit_run_case(struct kunit_suite *suite,
- struct kunit_case *test_case)
+static void kunit_run_case_internal(struct kunit *test,
+ struct kunit_suite *suite,
+ struct kunit_case *test_case)
{
- struct kunit test;
- int ret = 0;
-
- kunit_init_test(&test, test_case->name);
+ int ret;
if (suite->init) {
- ret = suite->init(&test);
+ ret = suite->init(test);
if (ret) {
- kunit_err(&test, "failed to initialize: %d\n", ret);
- kunit_set_failure(&test);
+ kunit_err(test, "failed to initialize: %d\n", ret);
+ kunit_set_failure(test);
return;
}
}
- test_case->run_case(&test);
+ test_case->run_case(test);
+}
+
+static void kunit_case_internal_cleanup(struct kunit *test)
+{
+ kunit_cleanup(test);
+}
+/*
+ * Performs post validations and cleanup after a test case was run.
+ * XXX: Should ONLY BE CALLED AFTER kunit_run_case_internal!
+ */
+static void kunit_run_case_cleanup(struct kunit *test,
+ struct kunit_suite *suite)
+{
if (suite->exit)
- suite->exit(&test);
+ suite->exit(test);
+
+ kunit_case_internal_cleanup(test);
+}
+
+/*
+ * Handles an unexpected crash in a test case.
+ */
+static void kunit_handle_test_crash(struct kunit *test,
+ struct kunit_suite *suite,
+ struct kunit_case *test_case)
+{
+ kunit_err(test, "kunit test case crashed!");
+ /*
+ * TODO(brendanhiggins@google.com): This prints the stack trace up
+ * through this frame, not up to the frame that caused the crash.
+ */
+ show_stack(NULL, NULL);
+
+ kunit_case_internal_cleanup(test);
+}
+
+struct kunit_try_catch_context {
+ struct kunit *test;
+ struct kunit_suite *suite;
+ struct kunit_case *test_case;
+};
+
+static void kunit_try_run_case(void *data)
+{
+ struct kunit_try_catch_context *ctx = data;
+ struct kunit *test = ctx->test;
+ struct kunit_suite *suite = ctx->suite;
+ struct kunit_case *test_case = ctx->test_case;
+
+ /*
+ * kunit_run_case_internal may encounter a fatal error; if it does,
+ * abort will be called, this thread will exit, and finally the parent
+ * thread will resume control and handle any necessary clean up.
+ */
+ kunit_run_case_internal(test, suite, test_case);
+ /* This line may never be reached. */
+ kunit_run_case_cleanup(test, suite);
+}
+
+static void kunit_catch_run_case(void *data)
+{
+ struct kunit_try_catch_context *ctx = data;
+ struct kunit *test = ctx->test;
+ struct kunit_suite *suite = ctx->suite;
+ struct kunit_case *test_case = ctx->test_case;
+ int try_exit_code = kunit_try_catch_get_result(&test->try_catch);
+
+ if (try_exit_code) {
+ kunit_set_failure(test);
+ /*
+ * Test case could not finish, we have no idea what state it is
+ * in, so don't do clean up.
+ */
+ if (try_exit_code == -ETIMEDOUT)
+ kunit_err(test, "test case timed out\n");
+ /*
+ * Unknown internal error occurred preventing test case from
+ * running, so there is nothing to clean up.
+ */
+ else
+ kunit_err(test, "internal error occurred preventing test case from running: %d\n",
+ try_exit_code);
+ return;
+ }
+
+ if (kunit_get_death_test(test)) {
+ /*
+ * EXPECTED DEATH: kunit_run_case_internal encountered
+ * anticipated fatal error. Everything should be in a safe
+ * state.
+ */
+ kunit_run_case_cleanup(test, suite);
+ } else {
+ /*
+ * UNEXPECTED DEATH: kunit_run_case_internal encountered an
+ * unanticipated fatal error. We have no idea what the state of
+ * the test case is in.
+ */
+ kunit_handle_test_crash(test, suite, test_case);
+ kunit_set_failure(test);
+ }
+}
+
+/*
+ * Performs all logic to run a test case. It also catches most errors that
+ * occurs in a test case and reports them as failures.
+ */
+static void kunit_run_case_catch_errors(struct kunit_suite *suite,
+ struct kunit_case *test_case)
+{
+ struct kunit_try_catch_context context;
+ struct kunit_try_catch *try_catch;
+ struct kunit test;
+
+ kunit_init_test(&test, test_case->name);
+ try_catch = &test.try_catch;
- kunit_cleanup(&test);
+ kunit_try_catch_init(try_catch,
+ &test,
+ kunit_try_run_case,
+ kunit_catch_run_case);
+ context.test = &test;
+ context.suite = suite;
+ context.test_case = test_case;
+ kunit_try_catch_run(try_catch, &context);
test_case->success = test.success;
}
@@ -172,7 +320,7 @@ int kunit_run_tests(struct kunit_suite *suite)
kunit_print_subtest_start(suite);
for (test_case = suite->test_cases; test_case->run_case; test_case++) {
- kunit_run_case(suite, test_case);
+ kunit_run_case_catch_errors(suite, test_case);
kunit_print_test_case_ok_not_ok(test_case, test_case_count++);
}
diff --git a/kunit/try-catch.c b/kunit/try-catch.c
new file mode 100644
index 0000000000000..de580f074387b
--- /dev/null
+++ b/kunit/try-catch.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * An API to allow a function, that may fail, to be executed, and recover in a
+ * controlled manner.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <kunit/try-catch.h>
+#include <kunit/test.h>
+#include <linux/completion.h>
+#include <linux/kthread.h>
+
+void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch)
+{
+ try_catch->try_result = -EFAULT;
+ complete_and_exit(try_catch->try_completion, -EFAULT);
+}
+
+static int kunit_generic_run_threadfn_adapter(void *data)
+{
+ struct kunit_try_catch *try_catch = data;
+
+ try_catch->try(try_catch->context);
+
+ complete_and_exit(try_catch->try_completion, 0);
+}
+
+void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
+{
+ DECLARE_COMPLETION_ONSTACK(try_completion);
+ struct kunit *test = try_catch->test;
+ struct task_struct *task_struct;
+ int exit_code, status;
+
+ try_catch->context = context;
+ try_catch->try_completion = &try_completion;
+ try_catch->try_result = 0;
+ task_struct = kthread_run(kunit_generic_run_threadfn_adapter,
+ try_catch,
+ "kunit_try_catch_thread");
+ if (IS_ERR(task_struct)) {
+ try_catch->catch(try_catch->context);
+ return;
+ }
+
+ /*
+ * TODO(brendanhiggins@google.com): We should probably have some type of
+ * variable timeout here. The only question is what that timeout value
+ * should be.
+ *
+ * The intention has always been, at some point, to be able to label
+ * tests with some type of size bucket (unit/small, integration/medium,
+ * large/system/end-to-end, etc), where each size bucket would get a
+ * default timeout value kind of like what Bazel does:
+ * https://docs.bazel.build/versions/master/be/common-definitions.html#test.size
+ * There is still some debate to be had on exactly how we do this. (For
+ * one, we probably want to have some sort of test runner level
+ * timeout.)
+ *
+ * For more background on this topic, see:
+ * https://mike-bland.com/2011/11/01/small-medium-large.html
+ */
+ status = wait_for_completion_timeout(&try_completion,
+ 300 * MSEC_PER_SEC); /* 5 min */
+ if (status < 0) {
+ kunit_err(test, "try timed out\n");
+ try_catch->try_result = -ETIMEDOUT;
+ }
+
+ exit_code = try_catch->try_result;
+
+ if (!exit_code)
+ return;
+
+ if (exit_code == -EFAULT)
+ try_catch->try_result = 0;
+ else if (exit_code == -EINTR)
+ kunit_err(test, "wake_up_process() was never called\n");
+ else if (exit_code)
+ kunit_err(test, "Unknown error: %d\n", exit_code);
+
+ try_catch->catch(try_catch->context);
+}
+
+void kunit_try_catch_init(struct kunit_try_catch *try_catch,
+ struct kunit *test,
+ kunit_try_catch_func_t try,
+ kunit_try_catch_func_t catch)
+{
+ try_catch->test = test;
+ try_catch->try = try;
+ try_catch->catch = catch;
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v7 08/18] objtool: add kunit_try_catch_throw to the noreturn list
From: Brendan Higgins @ 2019-07-09 6:30 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins, kbuild test robot
In-Reply-To: <20190709063023.251446-1-brendanhiggins@google.com>
Fix the following warning seen on GCC 7.3:
kunit/test-test.o: warning: objtool: kunit_test_unsuccessful_try() falls through to next function kunit_test_catch()
kunit_try_catch_throw is a function added in the following patch in this
series; it allows KUnit, a unit testing framework for the kernel, to
bail out of a broken test. As a consequence, it is a new __noreturn
function that objtool thinks is broken (as seen above). So fix this
warning by adding kunit_try_catch_throw to objtool's noreturn list.
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Link: https://www.spinics.net/lists/linux-kbuild/msg21708.html
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
tools/objtool/check.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 172f991957269..98db5fe85c797 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -134,6 +134,7 @@ static int __dead_end_function(struct objtool_file *file, struct symbol *func,
"usercopy_abort",
"machine_real_restart",
"rewind_stack_do_exit",
+ "kunit_try_catch_throw",
};
if (func->bind == STB_WEAK)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v7 07/18] kunit: test: add initial tests
From: Brendan Higgins @ 2019-07-09 6:30 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190709063023.251446-1-brendanhiggins@google.com>
Add a test for string stream along with a simpler example.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
kunit/Kconfig | 21 +++++++++
kunit/Makefile | 4 ++
kunit/example-test.c | 88 ++++++++++++++++++++++++++++++++++++++
kunit/string-stream-test.c | 75 ++++++++++++++++++++++++++++++++
4 files changed, 188 insertions(+)
create mode 100644 kunit/example-test.c
create mode 100644 kunit/string-stream-test.c
diff --git a/kunit/Kconfig b/kunit/Kconfig
index 330ae83527c23..8541ef95b65ad 100644
--- a/kunit/Kconfig
+++ b/kunit/Kconfig
@@ -14,4 +14,25 @@ config KUNIT
architectures. For more information, please see
Documentation/dev-tools/kunit/.
+config KUNIT_TEST
+ bool "KUnit test for KUnit"
+ depends on KUNIT
+ help
+ Enables the unit tests for the KUnit test framework. These tests test
+ the KUnit test framework itself; the tests are both written using
+ KUnit and test KUnit. This option should only be enabled for testing
+ purposes by developers interested in testing that KUnit works as
+ expected.
+
+config KUNIT_EXAMPLE_TEST
+ bool "Example test for KUnit"
+ depends on KUNIT
+ help
+ Enables an example unit test that illustrates some of the basic
+ features of KUnit. This test only exists to help new users understand
+ what KUnit is and how it is used. Please refer to the example test
+ itself, kunit/example-test.c, for more information. This option is
+ intended for curious hackers who would like to understand how to use
+ KUnit for kernel development.
+
endmenu
diff --git a/kunit/Makefile b/kunit/Makefile
index 6ddc622ee6b1c..60a9ea6cb4697 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1,3 +1,7 @@
obj-$(CONFIG_KUNIT) += test.o \
string-stream.o \
kunit-stream.o
+
+obj-$(CONFIG_KUNIT_TEST) += string-stream-test.o
+
+obj-$(CONFIG_KUNIT_EXAMPLE_TEST) += example-test.o
diff --git a/kunit/example-test.c b/kunit/example-test.c
new file mode 100644
index 0000000000000..f64a829aa441f
--- /dev/null
+++ b/kunit/example-test.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Example KUnit test to show how to use KUnit.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <kunit/test.h>
+
+/*
+ * This is the most fundamental element of KUnit, the test case. A test case
+ * makes a set EXPECTATIONs and ASSERTIONs about the behavior of some code; if
+ * any expectations or assertions are not met, the test fails; otherwise, the
+ * test passes.
+ *
+ * In KUnit, a test case is just a function with the signature
+ * `void (*)(struct kunit *)`. `struct kunit` is a context object that stores
+ * information about the current test.
+ */
+static void example_simple_test(struct kunit *test)
+{
+ /*
+ * This is an EXPECTATION; it is how KUnit tests things. When you want
+ * to test a piece of code, you set some expectations about what the
+ * code should do. KUnit then runs the test and verifies that the code's
+ * behavior matched what was expected.
+ */
+ KUNIT_EXPECT_EQ(test, 1 + 1, 2);
+}
+
+/*
+ * This is run once before each test case, see the comment on
+ * example_test_suite for more information.
+ */
+static int example_test_init(struct kunit *test)
+{
+ kunit_info(test, "initializing\n");
+
+ return 0;
+}
+
+/*
+ * Here we make a list of all the test cases we want to add to the test suite
+ * below.
+ */
+static struct kunit_case example_test_cases[] = {
+ /*
+ * This is a helper to create a test case object from a test case
+ * function; its exact function is not important to understand how to
+ * use KUnit, just know that this is how you associate test cases with a
+ * test suite.
+ */
+ KUNIT_CASE(example_simple_test),
+ {}
+};
+
+/*
+ * This defines a suite or grouping of tests.
+ *
+ * Test cases are defined as belonging to the suite by adding them to
+ * `kunit_cases`.
+ *
+ * Often it is desirable to run some function which will set up things which
+ * will be used by every test; this is accomplished with an `init` function
+ * which runs before each test case is invoked. Similarly, an `exit` function
+ * may be specified which runs after every test case and can be used to for
+ * cleanup. For clarity, running tests in a test suite would behave as follows:
+ *
+ * suite.init(test);
+ * suite.test_case[0](test);
+ * suite.exit(test);
+ * suite.init(test);
+ * suite.test_case[1](test);
+ * suite.exit(test);
+ * ...;
+ */
+static struct kunit_suite example_test_suite = {
+ .name = "example",
+ .init = example_test_init,
+ .test_cases = example_test_cases,
+};
+
+/*
+ * This registers the above test suite telling KUnit that this is a suite of
+ * tests that need to be run.
+ */
+kunit_test_suite(example_test_suite);
diff --git a/kunit/string-stream-test.c b/kunit/string-stream-test.c
new file mode 100644
index 0000000000000..b5641b078b8f6
--- /dev/null
+++ b/kunit/string-stream-test.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for struct string_stream.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <linux/slab.h>
+#include <kunit/test.h>
+#include <kunit/string-stream.h>
+
+static void string_stream_test_empty_on_creation(struct kunit *test)
+{
+ struct string_stream *stream = alloc_string_stream(test);
+
+ KUNIT_EXPECT_TRUE(test, string_stream_is_empty(stream));
+}
+
+static void string_stream_test_not_empty_after_add(struct kunit *test)
+{
+ struct string_stream *stream = alloc_string_stream(test);
+
+ string_stream_add(stream, "Foo");
+
+ KUNIT_EXPECT_FALSE(test, string_stream_is_empty(stream));
+}
+static void string_stream_test_get_string(struct kunit *test)
+{
+ struct string_stream *stream = alloc_string_stream(test);
+ char *output;
+
+ string_stream_add(stream, "Foo");
+ string_stream_add(stream, " %s", "bar");
+
+ output = string_stream_get_string(stream);
+ KUNIT_EXPECT_STREQ(test, output, "Foo bar");
+ kfree(output);
+}
+
+static void string_stream_test_add_and_clear(struct kunit *test)
+{
+ struct string_stream *stream = alloc_string_stream(test);
+ char *output;
+ int i;
+
+ for (i = 0; i < 10; i++)
+ string_stream_add(stream, "A");
+
+ output = string_stream_get_string(stream);
+ KUNIT_EXPECT_STREQ(test, output, "AAAAAAAAAA");
+ KUNIT_EXPECT_EQ(test, stream->length, (size_t)10);
+ KUNIT_EXPECT_FALSE(test, string_stream_is_empty(stream));
+ kfree(output);
+
+ string_stream_clear(stream);
+
+ output = string_stream_get_string(stream);
+ KUNIT_EXPECT_STREQ(test, output, "");
+ KUNIT_EXPECT_TRUE(test, string_stream_is_empty(stream));
+}
+
+static struct kunit_case string_stream_test_cases[] = {
+ KUNIT_CASE(string_stream_test_empty_on_creation),
+ KUNIT_CASE(string_stream_test_not_empty_after_add),
+ KUNIT_CASE(string_stream_test_get_string),
+ KUNIT_CASE(string_stream_test_add_and_clear),
+ {}
+};
+
+static struct kunit_suite string_stream_test_suite = {
+ .name = "string-stream-test",
+ .test_cases = string_stream_test_cases
+};
+kunit_test_suite(string_stream_test_suite);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v7 06/18] kbuild: enable building KUnit
From: Brendan Higgins @ 2019-07-09 6:30 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins, Michal Marek
In-Reply-To: <20190709063023.251446-1-brendanhiggins@google.com>
KUnit is a new unit testing framework for the kernel and when used is
built into the kernel as a part of it. Add KUnit to the root Kconfig and
Makefile to allow it to be actually built.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <michal.lkml@markovi.net>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Kconfig | 2 ++
Makefile | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/Kconfig b/Kconfig
index 48a80beab6853..10428501edb78 100644
--- a/Kconfig
+++ b/Kconfig
@@ -30,3 +30,5 @@ source "crypto/Kconfig"
source "lib/Kconfig"
source "lib/Kconfig.debug"
+
+source "kunit/Kconfig"
diff --git a/Makefile b/Makefile
index 3e4868a6498b2..60cf4f0813e0d 100644
--- a/Makefile
+++ b/Makefile
@@ -991,7 +991,7 @@ endif
PHONY += prepare0
ifeq ($(KBUILD_EXTMOD),)
-core-y += kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
+core-y += kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/ kunit/
vmlinux-dirs := $(patsubst %/,%,$(filter %/, $(init-y) $(init-m) \
$(core-y) $(core-m) $(drivers-y) $(drivers-m) \
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v7 05/18] kunit: test: add the concept of expectations
From: Brendan Higgins @ 2019-07-09 6:30 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190709063023.251446-1-brendanhiggins@google.com>
Add support for expectations, which allow properties to be specified and
then verified in tests.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
include/kunit/test.h | 525 +++++++++++++++++++++++++++++++++++++++++++
kunit/test.c | 66 ++++++
2 files changed, 591 insertions(+)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index bc7dbdcf8abab..df18765e6ea25 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -9,6 +9,7 @@
#ifndef _KUNIT_TEST_H
#define _KUNIT_TEST_H
+#include <linux/kernel.h>
#include <linux/types.h>
#include <linux/slab.h>
#include <kunit/kunit-stream.h>
@@ -295,4 +296,528 @@ void __printf(3, 4) kunit_printk(const char *level,
#define kunit_err(test, fmt, ...) \
kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__)
+/*
+ * Generates a compile-time warning in case of comparing incompatible types.
+ */
+#define __kunit_typecheck(lhs, rhs) \
+ ((void) __typecheck(lhs, rhs))
+
+static inline struct kunit_stream *kunit_expect_start(struct kunit *test,
+ const char *file,
+ const char *line)
+{
+ struct kunit_stream *stream = alloc_kunit_stream(test, KERN_ERR);
+
+ kunit_stream_add(stream, "EXPECTATION FAILED at %s:%s\n\t", file, line);
+
+ return stream;
+}
+
+static inline void kunit_expect_end(struct kunit *test,
+ bool success,
+ struct kunit_stream *stream)
+{
+ if (!success)
+ kunit_fail(test, stream);
+ else
+ kunit_stream_clear(stream);
+}
+
+#define KUNIT_EXPECT_START(test) \
+ kunit_expect_start(test, __FILE__, __stringify(__LINE__))
+
+#define KUNIT_EXPECT_END(test, success, stream) \
+ kunit_expect_end(test, success, stream)
+
+#define KUNIT_EXPECT_MSG(test, success, message, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ \
+ kunit_stream_add(__stream, message); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ KUNIT_EXPECT_END(test, success, __stream); \
+} while (0)
+
+#define KUNIT_EXPECT(test, success, message) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ \
+ kunit_stream_add(__stream, message); \
+ KUNIT_EXPECT_END(test, success, __stream); \
+} while (0)
+
+/**
+ * KUNIT_SUCCEED() - A no-op expectation. Only exists for code clarity.
+ * @test: The test context object.
+ *
+ * The opposite of KUNIT_FAIL(), it is an expectation that cannot fail. In other
+ * words, it does nothing and only exists for code clarity. See
+ * KUNIT_EXPECT_TRUE() for more information.
+ */
+#define KUNIT_SUCCEED(test) do {} while (0)
+
+/**
+ * KUNIT_FAIL() - Always causes a test to fail when evaluated.
+ * @test: The test context object.
+ * @fmt: an informational message to be printed when the assertion is made.
+ * @...: string format arguments.
+ *
+ * The opposite of KUNIT_SUCCEED(), it is an expectation that always fails. In
+ * other words, it always results in a failed expectation, and consequently
+ * always causes the test case to fail when evaluated. See KUNIT_EXPECT_TRUE()
+ * for more information.
+ */
+#define KUNIT_FAIL(test, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ KUNIT_EXPECT_END(test, false, __stream); \
+} while (0)
+
+/**
+ * KUNIT_EXPECT_TRUE() - Causes a test failure when the expression is not true.
+ * @test: The test context object.
+ * @condition: an arbitrary boolean expression. The test fails when this does
+ * not evaluate to true.
+ *
+ * This and expectations of the form `KUNIT_EXPECT_*` will cause the test case
+ * to fail when the specified condition is not met; however, it will not prevent
+ * the test case from continuing to run; this is otherwise known as an
+ * *expectation failure*.
+ */
+#define KUNIT_EXPECT_TRUE(test, condition) \
+ KUNIT_EXPECT(test, (condition), \
+ "Expected " #condition " is true, but is false\n")
+
+#define KUNIT_EXPECT_TRUE_MSG(test, condition, fmt, ...) \
+ KUNIT_EXPECT_MSG(test, (condition), \
+ "Expected " #condition " is true, but is false\n",\
+ fmt, ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_FALSE() - Makes a test failure when the expression is not false.
+ * @test: The test context object.
+ * @condition: an arbitrary boolean expression. The test fails when this does
+ * not evaluate to false.
+ *
+ * Sets an expectation that @condition evaluates to false. See
+ * KUNIT_EXPECT_TRUE() for more information.
+ */
+#define KUNIT_EXPECT_FALSE(test, condition) \
+ KUNIT_EXPECT(test, !(condition), \
+ "Expected " #condition " is false, but is true\n")
+
+#define KUNIT_EXPECT_FALSE_MSG(test, condition, fmt, ...) \
+ KUNIT_EXPECT_MSG(test, !(condition), \
+ "Expected " #condition " is false, but is true\n",\
+ fmt, ##__VA_ARGS__)
+
+void kunit_expect_binary_msg(struct kunit *test,
+ long long left, const char *left_name,
+ long long right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...);
+
+static inline void kunit_expect_binary(struct kunit *test,
+ long long left, const char *left_name,
+ long long right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line)
+{
+ kunit_expect_binary_msg(test,
+ left, left_name,
+ right, right_name,
+ compare_result,
+ compare_name,
+ file,
+ line,
+ NULL);
+}
+
+void kunit_expect_ptr_binary_msg(struct kunit *test,
+ void *left, const char *left_name,
+ void *right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...);
+
+static inline void kunit_expect_ptr_binary(struct kunit *test,
+ void *left, const char *left_name,
+ void *right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line)
+{
+ kunit_expect_ptr_binary_msg(test,
+ left, left_name,
+ right, right_name,
+ compare_result,
+ compare_name,
+ file,
+ line,
+ NULL);
+}
+
+/*
+ * A factory macro for defining the expectations for the basic comparisons
+ * defined for the built in types.
+ *
+ * Unfortunately, there is no common type that all types can be promoted to for
+ * which all the binary operators behave the same way as for the actual types
+ * (for example, there is no type that long long and unsigned long long can
+ * both be cast to where the comparison result is preserved for all values). So
+ * the best we can do is do the comparison in the original types and then coerce
+ * everything to long long for printing; this way, the comparison behaves
+ * correctly and the printed out value usually makes sense without
+ * interpretation, but can always be interpretted to figure out the actual
+ * value.
+ */
+#define KUNIT_EXPECT_BINARY(test, left, condition, right) do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_expect_binary(test, \
+ (long long) __left, #left, \
+ (long long) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__)); \
+} while (0)
+
+#define KUNIT_EXPECT_BINARY_MSG(test, left, condition, right, fmt, ...) do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_expect_binary_msg(test, \
+ (long long) __left, #left, \
+ (long long) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__), \
+ fmt, ##__VA_ARGS__); \
+} while (0)
+
+/*
+ * Just like KUNIT_EXPECT_BINARY, but for comparing pointer types.
+ */
+#define KUNIT_EXPECT_PTR_BINARY(test, left, condition, right) do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_expect_ptr_binary(test, \
+ (void *) __left, #left, \
+ (void *) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__)); \
+} while (0)
+
+#define KUNIT_EXPECT_PTR_BINARY_MSG(test, left, condition, right, fmt, ...) \
+do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_expect_ptr_binary_msg(test, \
+ (void *) __left, #left, \
+ (void *) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__), \
+ fmt, ##__VA_ARGS__); \
+} while (0)
+
+/**
+ * KUNIT_EXPECT_EQ() - Sets an expectation that @left and @right are equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) == (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_EQ(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, ==, right)
+
+#define KUNIT_EXPECT_EQ_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ ==, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_PTR_EQ() - Expects that pointers @left and @right are equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a pointer.
+ * @right: an arbitrary expression that evaluates to a pointer.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) == (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_PTR_EQ(test, left, right) \
+ KUNIT_EXPECT_PTR_BINARY(test, left, ==, right)
+
+#define KUNIT_EXPECT_PTR_EQ_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_PTR_BINARY_MSG(test, \
+ left, \
+ ==, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_NE() - An expectation that @left and @right are not equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are not
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) != (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_NE(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, !=, right)
+
+#define KUNIT_EXPECT_NE_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ !=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_PTR_NE() - Expects that pointers @left and @right are not equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a pointer.
+ * @right: an arbitrary expression that evaluates to a pointer.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are not
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) != (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_PTR_NE(test, left, right) \
+ KUNIT_EXPECT_PTR_BINARY(test, left, !=, right)
+
+#define KUNIT_EXPECT_PTR_NE_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_PTR_BINARY_MSG(test, \
+ left, \
+ !=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_LT() - An expectation that @left is less than @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the value that @left evaluates to is less than the
+ * value that @right evaluates to. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) < (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_LT(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, <, right)
+
+#define KUNIT_EXPECT_LT_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ <, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_LE() - Expects that @left is less than or equal to @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the value that @left evaluates to is less than or
+ * equal to the value that @right evaluates to. Semantically this is equivalent
+ * to KUNIT_EXPECT_TRUE(@test, (@left) <= (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_LE(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, <=, right)
+
+#define KUNIT_EXPECT_LE_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ <=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_GT() - An expectation that @left is greater than @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the value that @left evaluates to is greater than
+ * the value that @right evaluates to. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) > (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_GT(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, >, right)
+
+#define KUNIT_EXPECT_GT_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ >, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_GE() - Expects that @left is greater than or equal to @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the value that @left evaluates to is greater than
+ * the value that @right evaluates to. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) >= (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_GE(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, >=, right)
+
+#define KUNIT_EXPECT_GE_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ >=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_STREQ() - Expects that strings @left and @right are equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a null terminated string.
+ * @right: an arbitrary expression that evaluates to a null terminated string.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, !strcmp((@left), (@right))). See KUNIT_EXPECT_TRUE()
+ * for more information.
+ */
+#define KUNIT_EXPECT_STREQ(test, left, right) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Expected " #left " == " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ \
+ KUNIT_EXPECT_END(test, !strcmp(left, right), __stream); \
+} while (0)
+
+#define KUNIT_EXPECT_STREQ_MSG(test, left, right, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Expected " #left " == " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ \
+ KUNIT_EXPECT_END(test, !strcmp(left, right), __stream); \
+} while (0)
+
+/**
+ * KUNIT_EXPECT_STRNEQ() - Expects that strings @left and @right are not equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a null terminated string.
+ * @right: an arbitrary expression that evaluates to a null terminated string.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * not equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, strcmp((@left), (@right))). See KUNIT_EXPECT_TRUE()
+ * for more information.
+ */
+#define KUNIT_EXPECT_STRNEQ(test, left, right) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Expected " #left " != " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ \
+ KUNIT_EXPECT_END(test, strcmp(left, right), __stream); \
+} while (0)
+
+#define KUNIT_EXPECT_STRNEQ_MSG(test, left, right, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Expected " #left " != " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ \
+ KUNIT_EXPECT_END(test, strcmp(left, right), __stream); \
+} while (0)
+
+/**
+ * KUNIT_EXPECT_NOT_ERR_OR_NULL() - Expects that @ptr is not null and not err.
+ * @test: The test context object.
+ * @ptr: an arbitrary pointer.
+ *
+ * Sets an expectation that the value that @ptr evaluates to is not null and not
+ * an errno stored in a pointer. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, !IS_ERR_OR_NULL(@ptr)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_NOT_ERR_OR_NULL(test, ptr) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(ptr) __ptr = (ptr); \
+ \
+ if (!__ptr) \
+ kunit_stream_add(__stream, \
+ "Expected " #ptr " is not null, but is\n"); \
+ if (IS_ERR(__ptr)) \
+ kunit_stream_add(__stream, \
+ "Expected " #ptr " is not error, but is: %ld", \
+ PTR_ERR(__ptr)); \
+ \
+ KUNIT_EXPECT_END(test, !IS_ERR_OR_NULL(__ptr), __stream); \
+} while (0)
+
+#define KUNIT_EXPECT_NOT_ERR_OR_NULL_MSG(test, ptr, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(ptr) __ptr = (ptr); \
+ \
+ if (!__ptr) { \
+ kunit_stream_add(__stream, \
+ "Expected " #ptr " is not null, but is\n"); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ } \
+ if (IS_ERR(__ptr)) { \
+ kunit_stream_add(__stream, \
+ "Expected " #ptr " is not error, but is: %ld", \
+ PTR_ERR(__ptr)); \
+ \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ } \
+ KUNIT_EXPECT_END(test, !IS_ERR_OR_NULL(__ptr), __stream); \
+} while (0)
+
#endif /* _KUNIT_TEST_H */
diff --git a/kunit/test.c b/kunit/test.c
index 29edf34a89a37..8c745fd0c82d3 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -287,3 +287,69 @@ void kunit_printk(const char *level,
va_end(args);
}
+
+void kunit_expect_binary_msg(struct kunit *test,
+ long long left, const char *left_name,
+ long long right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...)
+{
+ struct kunit_stream *stream = kunit_expect_start(test, file, line);
+ struct va_format vaf;
+ va_list args;
+
+ kunit_stream_add(stream,
+ "Expected %s %s %s, but\n",
+ left_name, compare_name, right_name);
+ kunit_stream_add(stream, "\t\t%s == %lld\n", left_name, left);
+ kunit_stream_add(stream, "\t\t%s == %lld", right_name, right);
+
+ if (fmt) {
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ kunit_stream_add(stream, "\n%pV", &vaf);
+
+ va_end(args);
+ }
+
+ kunit_expect_end(test, compare_result, stream);
+}
+
+void kunit_expect_ptr_binary_msg(struct kunit *test,
+ void *left, const char *left_name,
+ void *right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...)
+{
+ struct kunit_stream *stream = kunit_expect_start(test, file, line);
+ struct va_format vaf;
+ va_list args;
+
+ kunit_stream_add(stream,
+ "Expected %s %s %s, but\n",
+ left_name, compare_name, right_name);
+ kunit_stream_add(stream, "\t\t%s == %pK\n", left_name, left);
+ kunit_stream_add(stream, "\t\t%s == %pK", right_name, right);
+
+ if (fmt) {
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ kunit_stream_add(stream, "\n%pV", &vaf);
+
+ va_end(args);
+ }
+
+ kunit_expect_end(test, compare_result, stream);
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v7 03/18] kunit: test: add string_stream a std::stream like string builder
From: Brendan Higgins @ 2019-07-09 6:30 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190709063023.251446-1-brendanhiggins@google.com>
A number of test features need to do pretty complicated string printing
where it may not be possible to rely on a single preallocated string
with parameters.
So provide a library for constructing the string as you go similar to
C++'s std::string.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
include/kunit/string-stream.h | 49 ++++++++++++
kunit/Makefile | 3 +-
kunit/string-stream.c | 147 ++++++++++++++++++++++++++++++++++
3 files changed, 198 insertions(+), 1 deletion(-)
create mode 100644 include/kunit/string-stream.h
create mode 100644 kunit/string-stream.c
diff --git a/include/kunit/string-stream.h b/include/kunit/string-stream.h
new file mode 100644
index 0000000000000..0552a05781afe
--- /dev/null
+++ b/include/kunit/string-stream.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * C++ stream style string builder used in KUnit for building messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#ifndef _KUNIT_STRING_STREAM_H
+#define _KUNIT_STRING_STREAM_H
+
+#include <linux/types.h>
+#include <linux/spinlock.h>
+#include <linux/kref.h>
+#include <stdarg.h>
+
+struct string_stream_fragment {
+ struct list_head node;
+ char *fragment;
+};
+
+struct string_stream {
+ size_t length;
+ struct list_head fragments;
+ /* length and fragments are protected by this lock */
+ spinlock_t lock;
+};
+
+struct kunit;
+
+struct string_stream *alloc_string_stream(struct kunit *test);
+
+void string_stream_get(struct string_stream *stream);
+
+int string_stream_put(struct string_stream *stream);
+
+int string_stream_add(struct string_stream *stream, const char *fmt, ...);
+
+int string_stream_vadd(struct string_stream *stream,
+ const char *fmt,
+ va_list args);
+
+char *string_stream_get_string(struct string_stream *stream);
+
+void string_stream_clear(struct string_stream *stream);
+
+bool string_stream_is_empty(struct string_stream *stream);
+
+#endif /* _KUNIT_STRING_STREAM_H */
diff --git a/kunit/Makefile b/kunit/Makefile
index 5efdc4dea2c08..275b565a0e81f 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1 +1,2 @@
-obj-$(CONFIG_KUNIT) += test.o
+obj-$(CONFIG_KUNIT) += test.o \
+ string-stream.o
diff --git a/kunit/string-stream.c b/kunit/string-stream.c
new file mode 100644
index 0000000000000..0463a92dad74b
--- /dev/null
+++ b/kunit/string-stream.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * C++ stream style string builder used in KUnit for building messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <kunit/string-stream.h>
+#include <kunit/test.h>
+
+int string_stream_vadd(struct string_stream *stream,
+ const char *fmt,
+ va_list args)
+{
+ struct string_stream_fragment *frag_container;
+ int len;
+ va_list args_for_counting;
+ unsigned long flags;
+
+ /* Make a copy because `vsnprintf` could change it */
+ va_copy(args_for_counting, args);
+
+ /* Need space for null byte. */
+ len = vsnprintf(NULL, 0, fmt, args_for_counting) + 1;
+
+ va_end(args_for_counting);
+
+ frag_container = kmalloc(sizeof(*frag_container), GFP_KERNEL);
+ if (!frag_container)
+ return -ENOMEM;
+
+ frag_container->fragment = kmalloc(len, GFP_KERNEL);
+ if (!frag_container->fragment) {
+ kfree(frag_container);
+ return -ENOMEM;
+ }
+
+ len = vsnprintf(frag_container->fragment, len, fmt, args);
+ spin_lock_irqsave(&stream->lock, flags);
+ stream->length += len;
+ list_add_tail(&frag_container->node, &stream->fragments);
+ spin_unlock_irqrestore(&stream->lock, flags);
+
+ return 0;
+}
+
+int string_stream_add(struct string_stream *stream, const char *fmt, ...)
+{
+ va_list args;
+ int result;
+
+ va_start(args, fmt);
+ result = string_stream_vadd(stream, fmt, args);
+ va_end(args);
+
+ return result;
+}
+
+void string_stream_clear(struct string_stream *stream)
+{
+ struct string_stream_fragment *frag_container, *frag_container_safe;
+ unsigned long flags;
+
+ spin_lock_irqsave(&stream->lock, flags);
+ list_for_each_entry_safe(frag_container,
+ frag_container_safe,
+ &stream->fragments,
+ node) {
+ list_del(&frag_container->node);
+ kfree(frag_container->fragment);
+ kfree(frag_container);
+ }
+ stream->length = 0;
+ spin_unlock_irqrestore(&stream->lock, flags);
+}
+
+char *string_stream_get_string(struct string_stream *stream)
+{
+ struct string_stream_fragment *frag_container;
+ size_t buf_len = stream->length + 1; /* +1 for null byte. */
+ char *buf;
+ unsigned long flags;
+
+ buf = kzalloc(buf_len, GFP_KERNEL);
+ if (!buf)
+ return NULL;
+
+ spin_lock_irqsave(&stream->lock, flags);
+ list_for_each_entry(frag_container, &stream->fragments, node)
+ strlcat(buf, frag_container->fragment, buf_len);
+ spin_unlock_irqrestore(&stream->lock, flags);
+
+ return buf;
+}
+
+bool string_stream_is_empty(struct string_stream *stream)
+{
+ bool is_empty;
+ unsigned long flags;
+
+ spin_lock_irqsave(&stream->lock, flags);
+ is_empty = list_empty(&stream->fragments);
+ spin_unlock_irqrestore(&stream->lock, flags);
+
+ return is_empty;
+}
+
+static int string_stream_init(struct kunit_resource *res, void *context)
+{
+ struct string_stream *stream;
+
+ stream = kzalloc(sizeof(*stream), GFP_KERNEL);
+ if (!stream)
+ return -ENOMEM;
+
+ res->allocation = stream;
+ INIT_LIST_HEAD(&stream->fragments);
+ spin_lock_init(&stream->lock);
+
+ return 0;
+}
+
+static void string_stream_free(struct kunit_resource *res)
+{
+ struct string_stream *stream = res->allocation;
+
+ string_stream_clear(stream);
+ kfree(stream);
+}
+
+struct string_stream *alloc_string_stream(struct kunit *test)
+{
+ struct kunit_resource *res;
+
+ res = kunit_alloc_resource(test,
+ string_stream_init,
+ string_stream_free,
+ NULL);
+
+ if (!res)
+ return NULL;
+
+ return res->allocation;
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v7 04/18] kunit: test: add kunit_stream a std::stream like logger
From: Brendan Higgins @ 2019-07-09 6:30 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190709063023.251446-1-brendanhiggins@google.com>
A lot of the expectation and assertion infrastructure prints out fairly
complicated test failure messages, so add a C++ style log library for
for logging test results.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
include/kunit/kunit-stream.h | 81 +++++++++++++++++++++++
include/kunit/test.h | 3 +
kunit/Makefile | 3 +-
kunit/kunit-stream.c | 123 +++++++++++++++++++++++++++++++++++
kunit/test.c | 6 ++
5 files changed, 215 insertions(+), 1 deletion(-)
create mode 100644 include/kunit/kunit-stream.h
create mode 100644 kunit/kunit-stream.c
diff --git a/include/kunit/kunit-stream.h b/include/kunit/kunit-stream.h
new file mode 100644
index 0000000000000..a7b53eabf6be4
--- /dev/null
+++ b/include/kunit/kunit-stream.h
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * C++ stream style string formatter and printer used in KUnit for outputting
+ * KUnit messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#ifndef _KUNIT_KUNIT_STREAM_H
+#define _KUNIT_KUNIT_STREAM_H
+
+#include <linux/types.h>
+#include <kunit/string-stream.h>
+
+struct kunit;
+
+/**
+ * struct kunit_stream - a std::stream style string builder.
+ *
+ * A std::stream style string builder. Allows messages to be built up and
+ * printed all at once.
+ */
+struct kunit_stream {
+ /* private: internal use only. */
+ struct kunit *test;
+ const char *level;
+ struct string_stream *internal_stream;
+};
+
+/**
+ * alloc_kunit_stream() - constructs a new &struct kunit_stream.
+ * @test: The test context object.
+ * @level: The log level at which to print out the message.
+ *
+ * Constructs a new test managed &struct kunit_stream.
+ */
+struct kunit_stream *alloc_kunit_stream(struct kunit *test, const char *level);
+
+/**
+ * kunit_stream_add(): adds the formatted input to the internal buffer.
+ * @kstream: the stream being operated on.
+ * @fmt: printf style format string to append to stream.
+ *
+ * Appends the formatted string, @fmt, to the internal buffer.
+ */
+void __printf(2, 3) kunit_stream_add(struct kunit_stream *kstream,
+ const char *fmt, ...);
+
+/**
+ * kunit_stream_append(): appends the contents of @other to @kstream.
+ * @kstream: the stream to which @other is appended.
+ * @other: the stream whose contents are appended to @kstream.
+ *
+ * Appends the contents of @other to @kstream.
+ */
+void kunit_stream_append(struct kunit_stream *kstream,
+ struct kunit_stream *other);
+
+/**
+ * kunit_stream_commit(): prints out the internal buffer to the user.
+ * @kstream: the stream being operated on.
+ *
+ * Outputs the contents of the internal buffer as a kunit_printk formatted
+ * output. KUNIT_STREAM ONLY OUTPUTS ITS BUFFER TO THE USER IF COMMIT IS
+ * CALLED!!! The reason for this is that it allows us to construct a message
+ * before we know whether we want to print it out; this can be extremely handy
+ * if there is information you might need for a failure message that is easiest
+ * to collect in the steps leading up to the actual check.
+ */
+void kunit_stream_commit(struct kunit_stream *kstream);
+
+/**
+ * kunit_stream_clear(): clears the internal buffer.
+ * @kstream: the stream being operated on.
+ *
+ * Clears the contents of the internal buffer.
+ */
+void kunit_stream_clear(struct kunit_stream *kstream);
+
+#endif /* _KUNIT_KUNIT_STREAM_H */
diff --git a/include/kunit/test.h b/include/kunit/test.h
index bdf41d31c343c..bc7dbdcf8abab 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -11,6 +11,7 @@
#include <linux/types.h>
#include <linux/slab.h>
+#include <kunit/kunit-stream.h>
struct kunit_resource;
@@ -184,6 +185,8 @@ struct kunit {
void kunit_init_test(struct kunit *test, const char *name);
+void kunit_fail(struct kunit *test, struct kunit_stream *stream);
+
int kunit_run_tests(struct kunit_suite *suite);
/**
diff --git a/kunit/Makefile b/kunit/Makefile
index 275b565a0e81f..6ddc622ee6b1c 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_KUNIT) += test.o \
- string-stream.o
+ string-stream.o \
+ kunit-stream.o
diff --git a/kunit/kunit-stream.c b/kunit/kunit-stream.c
new file mode 100644
index 0000000000000..8bea1f22eafb5
--- /dev/null
+++ b/kunit/kunit-stream.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * C++ stream style string formatter and printer used in KUnit for outputting
+ * KUnit messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <kunit/test.h>
+#include <kunit/kunit-stream.h>
+#include <kunit/string-stream.h>
+
+void kunit_stream_add(struct kunit_stream *kstream, const char *fmt, ...)
+{
+ va_list args;
+ struct string_stream *stream = kstream->internal_stream;
+
+ va_start(args, fmt);
+
+ if (string_stream_vadd(stream, fmt, args) < 0)
+ kunit_err(kstream->test,
+ "Failed to allocate fragment: %s\n",
+ fmt);
+
+ va_end(args);
+}
+
+void kunit_stream_append(struct kunit_stream *kstream,
+ struct kunit_stream *other)
+{
+ struct string_stream *other_stream = other->internal_stream;
+ const char *other_content;
+
+ other_content = string_stream_get_string(other_stream);
+
+ if (!other_content) {
+ kunit_err(kstream->test,
+ "Failed to get string from second argument for appending\n");
+ return;
+ }
+
+ kunit_stream_add(kstream, other_content);
+}
+
+void kunit_stream_clear(struct kunit_stream *kstream)
+{
+ string_stream_clear(kstream->internal_stream);
+}
+
+void kunit_stream_commit(struct kunit_stream *kstream)
+{
+ struct string_stream *stream = kstream->internal_stream;
+ struct string_stream_fragment *fragment;
+ struct kunit *test = kstream->test;
+ char *buf;
+
+ buf = string_stream_get_string(stream);
+ if (!buf) {
+ kunit_err(test,
+ "Could not allocate buffer, dumping stream:\n");
+ list_for_each_entry(fragment, &stream->fragments, node) {
+ kunit_err(test, fragment->fragment);
+ }
+ kunit_err(test, "\n");
+ goto cleanup;
+ }
+
+ kunit_printk(kstream->level, test, buf);
+ kfree(buf);
+
+cleanup:
+ kunit_stream_clear(kstream);
+}
+
+static int kunit_stream_init(struct kunit_resource *res, void *context)
+{
+ struct kunit *test = context;
+ struct kunit_stream *stream;
+
+ stream = kzalloc(sizeof(*stream), GFP_KERNEL);
+ if (!stream)
+ return -ENOMEM;
+
+ res->allocation = stream;
+ stream->test = test;
+ stream->internal_stream = alloc_string_stream(test);
+
+ if (!stream->internal_stream)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void kunit_stream_free(struct kunit_resource *res)
+{
+ struct kunit_stream *stream = res->allocation;
+
+ if (!string_stream_is_empty(stream->internal_stream)) {
+ kunit_err(stream->test,
+ "End of test case reached with uncommitted stream entries\n");
+ kunit_stream_commit(stream);
+ }
+}
+
+struct kunit_stream *alloc_kunit_stream(struct kunit *test, const char *level)
+{
+ struct kunit_stream *kstream;
+ struct kunit_resource *res;
+
+ res = kunit_alloc_resource(test,
+ kunit_stream_init,
+ kunit_stream_free,
+ test);
+
+ if (!res)
+ return NULL;
+
+ kstream = res->allocation;
+ kstream->level = level;
+
+ return kstream;
+}
diff --git a/kunit/test.c b/kunit/test.c
index f165c9d8e10b0..29edf34a89a37 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -120,6 +120,12 @@ static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case,
test_case->name);
}
+void kunit_fail(struct kunit *test, struct kunit_stream *stream)
+{
+ kunit_set_failure(test);
+ kunit_stream_commit(stream);
+}
+
void kunit_init_test(struct kunit *test, const char *name)
{
mutex_init(&test->lock);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v7 02/18] kunit: test: add test resource management API
From: Brendan Higgins @ 2019-07-09 6:30 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190709063023.251446-1-brendanhiggins@google.com>
Create a common API for test managed resources like memory and test
objects. A lot of times a test will want to set up infrastructure to be
used in test cases; this could be anything from just wanting to allocate
some memory to setting up a driver stack; this defines facilities for
creating "test resources" which are managed by the test infrastructure
and are automatically cleaned up at the conclusion of the test.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
include/kunit/test.h | 116 +++++++++++++++++++++++++++++++++++++++++++
kunit/test.c | 94 +++++++++++++++++++++++++++++++++++
2 files changed, 210 insertions(+)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index e0b34acb9ee4e..bdf41d31c343c 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -10,6 +10,70 @@
#define _KUNIT_TEST_H
#include <linux/types.h>
+#include <linux/slab.h>
+
+struct kunit_resource;
+
+typedef int (*kunit_resource_init_t)(struct kunit_resource *, void *);
+typedef void (*kunit_resource_free_t)(struct kunit_resource *);
+
+/**
+ * struct kunit_resource - represents a *test managed resource*
+ * @allocation: for the user to store arbitrary data.
+ * @free: a user supplied function to free the resource. Populated by
+ * kunit_alloc_resource().
+ *
+ * Represents a *test managed resource*, a resource which will automatically be
+ * cleaned up at the end of a test case.
+ *
+ * Example:
+ *
+ * .. code-block:: c
+ *
+ * struct kunit_kmalloc_params {
+ * size_t size;
+ * gfp_t gfp;
+ * };
+ *
+ * static int kunit_kmalloc_init(struct kunit_resource *res, void *context)
+ * {
+ * struct kunit_kmalloc_params *params = context;
+ * res->allocation = kmalloc(params->size, params->gfp);
+ *
+ * if (!res->allocation)
+ * return -ENOMEM;
+ *
+ * return 0;
+ * }
+ *
+ * static void kunit_kmalloc_free(struct kunit_resource *res)
+ * {
+ * kfree(res->allocation);
+ * }
+ *
+ * void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp)
+ * {
+ * struct kunit_kmalloc_params params;
+ * struct kunit_resource *res;
+ *
+ * params.size = size;
+ * params.gfp = gfp;
+ *
+ * res = kunit_alloc_resource(test, kunit_kmalloc_init,
+ * kunit_kmalloc_free, ¶ms);
+ * if (res)
+ * return res->allocation;
+ *
+ * return NULL;
+ * }
+ */
+struct kunit_resource {
+ void *allocation;
+ kunit_resource_free_t free;
+
+ /* private: internal use only. */
+ struct list_head node;
+};
struct kunit;
@@ -109,6 +173,13 @@ struct kunit {
* have terminated.
*/
bool success; /* Read only after test_case finishes! */
+ struct mutex lock; /* Gaurds all mutable test state. */
+ /*
+ * Because resources is a list that may be updated multiple times (with
+ * new resources) from any thread associated with a test case, we must
+ * protect it with some type of lock.
+ */
+ struct list_head resources; /* Protected by lock. */
};
void kunit_init_test(struct kunit *test, const char *name);
@@ -141,6 +212,51 @@ int kunit_run_tests(struct kunit_suite *suite);
} \
late_initcall(kunit_suite_init##suite)
+/**
+ * kunit_alloc_resource() - Allocates a *test managed resource*.
+ * @test: The test context object.
+ * @init: a user supplied function to initialize the resource.
+ * @free: a user supplied function to free the resource.
+ * @context: for the user to pass in arbitrary data to the init function.
+ *
+ * Allocates a *test managed resource*, a resource which will automatically be
+ * cleaned up at the end of a test case. See &struct kunit_resource for an
+ * example.
+ */
+struct kunit_resource *kunit_alloc_resource(struct kunit *test,
+ kunit_resource_init_t init,
+ kunit_resource_free_t free,
+ void *context);
+
+void kunit_free_resource(struct kunit *test, struct kunit_resource *res);
+
+/**
+ * kunit_kmalloc() - Like kmalloc() except the allocation is *test managed*.
+ * @test: The test context object.
+ * @size: The size in bytes of the desired memory.
+ * @gfp: flags passed to underlying kmalloc().
+ *
+ * Just like `kmalloc(...)`, except the allocation is managed by the test case
+ * and is automatically cleaned up after the test case concludes. See &struct
+ * kunit_resource for more information.
+ */
+void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp);
+
+/**
+ * kunit_kzalloc() - Just like kunit_kmalloc(), but zeroes the allocation.
+ * @test: The test context object.
+ * @size: The size in bytes of the desired memory.
+ * @gfp: flags passed to underlying kmalloc().
+ *
+ * See kzalloc() and kunit_kmalloc() for more information.
+ */
+static inline void *kunit_kzalloc(struct kunit *test, size_t size, gfp_t gfp)
+{
+ return kunit_kmalloc(test, size, gfp | __GFP_ZERO);
+}
+
+void kunit_cleanup(struct kunit *test);
+
void __printf(3, 4) kunit_printk(const char *level,
const struct kunit *test,
const char *fmt, ...);
diff --git a/kunit/test.c b/kunit/test.c
index 571e4c65deb5c..f165c9d8e10b0 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -122,6 +122,8 @@ static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case,
void kunit_init_test(struct kunit *test, const char *name)
{
+ mutex_init(&test->lock);
+ INIT_LIST_HEAD(&test->resources);
test->name = name;
test->success = true;
}
@@ -151,6 +153,8 @@ static void kunit_run_case(struct kunit_suite *suite,
if (suite->exit)
suite->exit(&test);
+ kunit_cleanup(&test);
+
test_case->success = test.success;
}
@@ -171,6 +175,96 @@ int kunit_run_tests(struct kunit_suite *suite)
return 0;
}
+struct kunit_resource *kunit_alloc_resource(struct kunit *test,
+ kunit_resource_init_t init,
+ kunit_resource_free_t free,
+ void *context)
+{
+ struct kunit_resource *res;
+ int ret;
+
+ res = kzalloc(sizeof(*res), GFP_KERNEL);
+ if (!res)
+ return NULL;
+
+ ret = init(res, context);
+ if (ret)
+ return NULL;
+
+ res->free = free;
+ mutex_lock(&test->lock);
+ list_add_tail(&res->node, &test->resources);
+ mutex_unlock(&test->lock);
+
+ return res;
+}
+
+void kunit_free_resource(struct kunit *test, struct kunit_resource *res)
+{
+ res->free(res);
+ list_del(&res->node);
+ kfree(res);
+}
+
+struct kunit_kmalloc_params {
+ size_t size;
+ gfp_t gfp;
+};
+
+static int kunit_kmalloc_init(struct kunit_resource *res, void *context)
+{
+ struct kunit_kmalloc_params *params = context;
+
+ res->allocation = kmalloc(params->size, params->gfp);
+ if (!res->allocation)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void kunit_kmalloc_free(struct kunit_resource *res)
+{
+ kfree(res->allocation);
+}
+
+void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp)
+{
+ struct kunit_kmalloc_params params;
+ struct kunit_resource *res;
+
+ params.size = size;
+ params.gfp = gfp;
+
+ res = kunit_alloc_resource(test,
+ kunit_kmalloc_init,
+ kunit_kmalloc_free,
+ ¶ms);
+
+ if (res)
+ return res->allocation;
+
+ return NULL;
+}
+
+void kunit_cleanup(struct kunit *test)
+{
+ struct kunit_resource *resource, *resource_safe;
+
+ mutex_lock(&test->lock);
+ /*
+ * test->resources is a stack - each allocation must be freed in the
+ * reverse order from which it was added since one resource may depend
+ * on another for its entire lifetime.
+ */
+ list_for_each_entry_safe_reverse(resource,
+ resource_safe,
+ &test->resources,
+ node) {
+ kunit_free_resource(test, resource);
+ }
+ mutex_unlock(&test->lock);
+}
+
void kunit_printk(const char *level,
const struct kunit *test,
const char *fmt, ...)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v7 01/18] kunit: test: add KUnit test runner core
From: Brendan Higgins @ 2019-07-09 6:30 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190709063023.251446-1-brendanhiggins@google.com>
Add core facilities for defining unit tests; this provides a common way
to define test cases, functions that execute code which is under test
and determine whether the code under test behaves as expected; this also
provides a way to group together related test cases in test suites (here
we call them test_modules).
Just define test cases and how to execute them for now; setting
expectations on code will be defined later.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
---
include/kunit/test.h | 179 ++++++++++++++++++++++++++++++++++++++++
kunit/Kconfig | 17 ++++
kunit/Makefile | 1 +
kunit/test.c | 189 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 386 insertions(+)
create mode 100644 include/kunit/test.h
create mode 100644 kunit/Kconfig
create mode 100644 kunit/Makefile
create mode 100644 kunit/test.c
diff --git a/include/kunit/test.h b/include/kunit/test.h
new file mode 100644
index 0000000000000..e0b34acb9ee4e
--- /dev/null
+++ b/include/kunit/test.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Base unit test (KUnit) API.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#ifndef _KUNIT_TEST_H
+#define _KUNIT_TEST_H
+
+#include <linux/types.h>
+
+struct kunit;
+
+/**
+ * struct kunit_case - represents an individual test case.
+ * @run_case: the function representing the actual test case.
+ * @name: the name of the test case.
+ *
+ * A test case is a function with the signature, ``void (*)(struct kunit *)``
+ * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
+ * test case is associated with a &struct kunit_suite and will be run after the
+ * suite's init function and followed by the suite's exit function.
+ *
+ * A test case should be static and should only be created with the KUNIT_CASE()
+ * macro; additionally, every array of test cases should be terminated with an
+ * empty test case.
+ *
+ * Example:
+ *
+ * .. code-block:: c
+ *
+ * void add_test_basic(struct kunit *test)
+ * {
+ * KUNIT_EXPECT_EQ(test, 1, add(1, 0));
+ * KUNIT_EXPECT_EQ(test, 2, add(1, 1));
+ * KUNIT_EXPECT_EQ(test, 0, add(-1, 1));
+ * KUNIT_EXPECT_EQ(test, INT_MAX, add(0, INT_MAX));
+ * KUNIT_EXPECT_EQ(test, -1, add(INT_MAX, INT_MIN));
+ * }
+ *
+ * static struct kunit_case example_test_cases[] = {
+ * KUNIT_CASE(add_test_basic),
+ * {}
+ * };
+ *
+ */
+struct kunit_case {
+ void (*run_case)(struct kunit *test);
+ const char *name;
+
+ /* private: internal use only. */
+ bool success;
+};
+
+/**
+ * KUNIT_CASE - A helper for creating a &struct kunit_case
+ * @test_name: a reference to a test case function.
+ *
+ * Takes a symbol for a function representing a test case and creates a
+ * &struct kunit_case object from it. See the documentation for
+ * &struct kunit_case for an example on how to use it.
+ */
+#define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }
+
+/**
+ * struct kunit_suite - describes a related collection of &struct kunit_case s.
+ * @name: the name of the test. Purely informational.
+ * @init: called before every test case.
+ * @exit: called after every test case.
+ * @test_cases: a null terminated array of test cases.
+ *
+ * A kunit_suite is a collection of related &struct kunit_case s, such that
+ * @init is called before every test case and @exit is called after every test
+ * case, similar to the notion of a *test fixture* or a *test class* in other
+ * unit testing frameworks like JUnit or Googletest.
+ *
+ * Every &struct kunit_case must be associated with a kunit_suite for KUnit to
+ * run it.
+ */
+struct kunit_suite {
+ const char name[256];
+ int (*init)(struct kunit *test);
+ void (*exit)(struct kunit *test);
+ struct kunit_case *test_cases;
+};
+
+/**
+ * struct kunit - represents a running instance of a test.
+ * @priv: for user to store arbitrary data. Commonly used to pass data created
+ * in the init function (see &struct kunit_suite).
+ *
+ * Used to store information about the current context under which the test is
+ * running. Most of this data is private and should only be accessed indirectly
+ * via public functions; the one exception is @priv which can be used by the
+ * test writer to store arbitrary data.
+ */
+struct kunit {
+ void *priv;
+
+ /* private: internal use only. */
+ const char *name; /* Read only after initialization! */
+ /*
+ * success starts as true, and may only be set to false during a test
+ * case; thus, it is safe to update this across multiple threads using
+ * WRITE_ONCE; however, as a consequence, it may only be read after the
+ * test case finishes once all threads associated with the test case
+ * have terminated.
+ */
+ bool success; /* Read only after test_case finishes! */
+};
+
+void kunit_init_test(struct kunit *test, const char *name);
+
+int kunit_run_tests(struct kunit_suite *suite);
+
+/**
+ * kunit_test_suite() - used to register a &struct kunit_suite with KUnit.
+ * @suite: a statically allocated &struct kunit_suite.
+ *
+ * Registers @suite with the test framework. See &struct kunit_suite for more
+ * information.
+ *
+ * NOTE: Currently KUnit tests are all run as late_initcalls; this means that
+ * they cannot test anything where tests must run at a different init phase. One
+ * significant restriction resulting from this is that KUnit cannot reliably
+ * test anything that is initialize in the late_init phase; another is that
+ * KUnit is useless to test things that need to be run in an earlier init phase.
+ */
+#define kunit_test_suite(suite) \
+ /*
+ * TODO(brendanhiggins@google.com): Don't run all KUnit tests as
+ * late_initcalls. I have some future work planned to dispatch
+ * all KUnit tests from the same place, and at the very least to
+ * do so after everything else is definitely initialized.
+ */ \
+ static int kunit_suite_init##suite(void) \
+ { \
+ return kunit_run_tests(&suite); \
+ } \
+ late_initcall(kunit_suite_init##suite)
+
+void __printf(3, 4) kunit_printk(const char *level,
+ const struct kunit *test,
+ const char *fmt, ...);
+
+/**
+ * kunit_info() - Prints an INFO level message associated with the current test.
+ * @test: The test context object.
+ * @fmt: A printk() style format string.
+ *
+ * Prints an info level message associated with the test suite being run. Takes
+ * a variable number of format parameters just like printk().
+ */
+#define kunit_info(test, fmt, ...) \
+ kunit_printk(KERN_INFO, test, fmt, ##__VA_ARGS__)
+
+/**
+ * kunit_warn() - Prints a WARN level message associated with the current test.
+ * @test: The test context object.
+ * @fmt: A printk() style format string.
+ *
+ * Prints a warning level message.
+ */
+#define kunit_warn(test, fmt, ...) \
+ kunit_printk(KERN_WARNING, test, fmt, ##__VA_ARGS__)
+
+/**
+ * kunit_err() - Prints an ERROR level message associated with the current test.
+ * @test: The test context object.
+ * @fmt: A printk() style format string.
+ *
+ * Prints an error level message.
+ */
+#define kunit_err(test, fmt, ...) \
+ kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__)
+
+#endif /* _KUNIT_TEST_H */
diff --git a/kunit/Kconfig b/kunit/Kconfig
new file mode 100644
index 0000000000000..330ae83527c23
--- /dev/null
+++ b/kunit/Kconfig
@@ -0,0 +1,17 @@
+#
+# KUnit base configuration
+#
+
+menu "KUnit support"
+
+config KUNIT
+ bool "Enable support for unit tests (KUnit)"
+ help
+ Enables support for kernel unit tests (KUnit), a lightweight unit
+ testing and mocking framework for the Linux kernel. These tests are
+ able to be run locally on a developer's workstation without a VM or
+ special hardware when using UML. Can also be used on most other
+ architectures. For more information, please see
+ Documentation/dev-tools/kunit/.
+
+endmenu
diff --git a/kunit/Makefile b/kunit/Makefile
new file mode 100644
index 0000000000000..5efdc4dea2c08
--- /dev/null
+++ b/kunit/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_KUNIT) += test.o
diff --git a/kunit/test.c b/kunit/test.c
new file mode 100644
index 0000000000000..571e4c65deb5c
--- /dev/null
+++ b/kunit/test.c
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Base unit test (KUnit) API.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <linux/kernel.h>
+#include <kunit/test.h>
+
+static void kunit_set_failure(struct kunit *test)
+{
+ WRITE_ONCE(test->success, false);
+}
+
+static int kunit_vprintk_emit(int level, const char *fmt, va_list args)
+{
+ return vprintk_emit(0, level, NULL, 0, fmt, args);
+}
+
+static int kunit_printk_emit(int level, const char *fmt, ...)
+{
+ va_list args;
+ int ret;
+
+ va_start(args, fmt);
+ ret = kunit_vprintk_emit(level, fmt, args);
+ va_end(args);
+
+ return ret;
+}
+
+static void kunit_vprintk(const struct kunit *test,
+ const char *level,
+ struct va_format *vaf)
+{
+ kunit_printk_emit(level[1] - '0', "\t# %s: %pV", test->name, vaf);
+}
+
+static void kunit_print_tap_version(void)
+{
+ static bool kunit_has_printed_tap_version;
+
+ if (!kunit_has_printed_tap_version) {
+ kunit_printk_emit(LOGLEVEL_INFO, "TAP version 14\n");
+ kunit_has_printed_tap_version = true;
+ }
+}
+
+static size_t kunit_test_cases_len(struct kunit_case *test_cases)
+{
+ struct kunit_case *test_case;
+ size_t len = 0;
+
+ for (test_case = test_cases; test_case->run_case; test_case++)
+ len++;
+
+ return len;
+}
+
+static void kunit_print_subtest_start(struct kunit_suite *suite)
+{
+ kunit_print_tap_version();
+ kunit_printk_emit(LOGLEVEL_INFO, "\t# Subtest: %s\n", suite->name);
+ kunit_printk_emit(LOGLEVEL_INFO,
+ "\t1..%zd\n",
+ kunit_test_cases_len(suite->test_cases));
+}
+
+static void kunit_print_ok_not_ok(bool should_indent,
+ bool is_ok,
+ size_t test_number,
+ const char *description)
+{
+ const char *indent, *ok_not_ok;
+
+ if (should_indent)
+ indent = "\t";
+ else
+ indent = "";
+
+ if (is_ok)
+ ok_not_ok = "ok";
+ else
+ ok_not_ok = "not ok";
+
+ kunit_printk_emit(LOGLEVEL_INFO,
+ "%s%s %zd - %s\n",
+ indent, ok_not_ok, test_number, description);
+}
+
+static bool kunit_suite_has_succeeded(struct kunit_suite *suite)
+{
+ const struct kunit_case *test_case;
+
+ for (test_case = suite->test_cases; test_case->run_case; test_case++)
+ if (!test_case->success)
+ return false;
+
+ return true;
+}
+
+static void kunit_print_subtest_end(struct kunit_suite *suite)
+{
+ static size_t kunit_suite_counter = 1;
+
+ kunit_print_ok_not_ok(false,
+ kunit_suite_has_succeeded(suite),
+ kunit_suite_counter++,
+ suite->name);
+}
+
+static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case,
+ size_t test_number)
+{
+ kunit_print_ok_not_ok(true,
+ test_case->success,
+ test_number,
+ test_case->name);
+}
+
+void kunit_init_test(struct kunit *test, const char *name)
+{
+ test->name = name;
+ test->success = true;
+}
+
+/*
+ * Performs all logic to run a test case.
+ */
+static void kunit_run_case(struct kunit_suite *suite,
+ struct kunit_case *test_case)
+{
+ struct kunit test;
+ int ret = 0;
+
+ kunit_init_test(&test, test_case->name);
+
+ if (suite->init) {
+ ret = suite->init(&test);
+ if (ret) {
+ kunit_err(&test, "failed to initialize: %d\n", ret);
+ kunit_set_failure(&test);
+ return;
+ }
+ }
+
+ test_case->run_case(&test);
+
+ if (suite->exit)
+ suite->exit(&test);
+
+ test_case->success = test.success;
+}
+
+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++) {
+ kunit_run_case(suite, test_case);
+ kunit_print_test_case_ok_not_ok(test_case, test_case_count++);
+ }
+
+ kunit_print_subtest_end(suite);
+
+ return 0;
+}
+
+void kunit_printk(const char *level,
+ const struct kunit *test,
+ const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ kunit_vprintk(test, level, &vaf);
+
+ va_end(args);
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v7 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework
From: Brendan Higgins @ 2019-07-09 6:30 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins, Michal Marek, Jonathan Corbet, Iurii Zaikin
## TL;DR
This is a pretty straightforward follow-up to Luis' comments on PATCH
v6: There is nothing that changes any functionality or usage.
As for our current status, we only need reviews/acks on the following
patches:
- [PATCH v7 06/18] kbuild: enable building KUnit
- Need a review or ack from Masahiro Yamada or Michal Marek
- [PATCH v7 08/18] objtool: add kunit_try_catch_throw to the noreturn
list
- Need a review or ack from Josh Poimboeuf or Peter Zijlstra
Other than that, I think we should be good to go.
## Background
This patch set proposes KUnit, a lightweight unit testing and mocking
framework for the Linux kernel.
Unlike Autotest and kselftest, KUnit is a true unit testing framework;
it does not require installing the kernel on a test machine or in a VM
(however, KUnit still allows you to run tests on test machines or in VMs
if you want[1]) and does not require tests to be written in userspace
running on a host kernel. Additionally, KUnit is fast: From invocation
to completion KUnit can run several dozen tests in about a second.
Currently, the entire KUnit test suite for KUnit runs in under a second
from the initial invocation (build time excluded).
KUnit is heavily inspired by JUnit, Python's unittest.mock, and
Googletest/Googlemock for C++. KUnit provides facilities for defining
unit test cases, grouping related test cases into test suites, providing
common infrastructure for running tests, mocking, spying, and much more.
### What's so special about unit testing?
A unit test is supposed to test a single unit of code in isolation,
hence the name. There should be no dependencies outside the control of
the test; this means no external dependencies, which makes tests orders
of magnitudes faster. Likewise, since there are no external dependencies,
there are no hoops to jump through to run the tests. Additionally, this
makes unit tests deterministic: a failing unit test always indicates a
problem. Finally, because unit tests necessarily have finer granularity,
they are able to test all code paths easily solving the classic problem
of difficulty in exercising error handling code.
### Is KUnit trying to replace other testing frameworks for the kernel?
No. Most existing tests for the Linux kernel are end-to-end tests, which
have their place. A well tested system has lots of unit tests, a
reasonable number of integration tests, and some end-to-end tests. KUnit
is just trying to address the unit test space which is currently not
being addressed.
### More information on KUnit
There is a bunch of documentation near the end of this patch set that
describes how to use KUnit and best practices for writing unit tests.
For convenience I am hosting the compiled docs here[2].
Additionally for convenience, I have applied these patches to a
branch[3]. The repo may be cloned with:
git clone https://kunit.googlesource.com/linux
This patchset is on the kunit/rfc/v5.2/v7 branch.
## Changes Since Last Version
Aside from renaming `struct kunit_module` to `struct kunit_suite`, there
isn't really anything in here that changes any functionality:
- Rebased on v5.2
- Added Iurii as a maintainer for PROC SYSCTL, as suggested by Luis.
- Removed some references to spinlock that I failed to remove in the
previous version, as pointed out by Luis.
- Cleaned up some comments, as suggested by Luis.
[1] https://google.github.io/kunit-docs/third_party/kernel/docs/usage.html#kunit-on-non-uml-architectures
[2] https://google.github.io/kunit-docs/third_party/kernel/docs/
[3] https://kunit.googlesource.com/linux/+/kunit/rfc/v5.2/v7
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [RFC 0/7] Introduce TEE based Trusted Keys support
From: Sumit Garg @ 2019-07-09 5:58 UTC (permalink / raw)
To: Jens Wiklander
Cc: corbet, dhowells, jejb, Jarkko Sakkinen, Mimi Zohar, jmorris,
serge, Ard Biesheuvel, Daniel Thompson, linux-doc,
Linux Kernel Mailing List, tee-dev, keyrings,
linux-security-module, linux-integrity
In-Reply-To: <20190708163140.GB28253@jax>
On Mon, 8 Jul 2019 at 22:01, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> Hi Sumit,
>
> On Mon, Jul 08, 2019 at 06:11:39PM +0530, Sumit Garg wrote:
> > Hi Jens,
> >
> > On Thu, 13 Jun 2019 at 16:01, Sumit Garg <sumit.garg@linaro.org> wrote:
> > >
> > > Add support for TEE based trusted keys where TEE provides the functionality
> > > to seal and unseal trusted keys using hardware unique key. Also, this is
> > > an alternative in case platform doesn't possess a TPM device.
> > >
> > > This series also adds some TEE features like:
> > >
> > > Patch #1, #2 enables support for registered kernel shared memory with TEE.
> > >
> >
> > Would you like to pick up Patch #1, #2 separately? I think both these
> > patches add independent functionality and also got reviewed-by tags
> > too.
>
> I think it makes more sense to keep them together in the same patch
> series or could end up with dependencies between trees.
>
I understand your point. Let me keep this patch-set together to avoid
any dependencies.
-Sumit
> If you don't think dependencies will be an issue then I don't mind
> picking them up, in that case they'd likely sit in an arm-soc branch
> until next merge window. However, I think that #3 (support for private
> kernel login method) should be included too and that one isn't ready
> yet.
>
> Thanks,
> Jens
>
> >
> >
> > -Sumit
> >
> > > Patch #3 enables support for private kernel login method required for
> > > cases like trusted keys where we don't wan't user-space to directly access
> > > TEE service to retrieve trusted key contents.
> > >
> > > Rest of the patches from #4 to #7 adds support for TEE based trusted keys.
> > >
> > > This patch-set has been tested with OP-TEE based pseudo TA which can be
> > > found here [1].
> > >
> > > Looking forward to your valuable feedback/suggestions.
> > >
> > > [1] https://github.com/OP-TEE/optee_os/pull/3082
> > >
> > > Sumit Garg (7):
> > > tee: optee: allow kernel pages to register as shm
> > > tee: enable support to register kernel memory
> > > tee: add private login method for kernel clients
> > > KEYS: trusted: Introduce TEE based Trusted Keys
> > > KEYS: encrypted: Allow TEE based trusted master keys
> > > doc: keys: Document usage of TEE based Trusted Keys
> > > MAINTAINERS: Add entry for TEE based Trusted Keys
> > >
> > > Documentation/security/keys/tee-trusted.rst | 93 +++++
> > > MAINTAINERS | 9 +
> > > drivers/tee/optee/call.c | 7 +
> > > drivers/tee/tee_core.c | 6 +
> > > drivers/tee/tee_shm.c | 16 +-
> > > include/keys/tee_trusted.h | 84 ++++
> > > include/keys/trusted-type.h | 1 +
> > > include/linux/tee_drv.h | 1 +
> > > include/uapi/linux/tee.h | 2 +
> > > security/keys/Kconfig | 3 +
> > > security/keys/Makefile | 3 +
> > > security/keys/encrypted-keys/masterkey_trusted.c | 10 +-
> > > security/keys/tee_trusted.c | 506 +++++++++++++++++++++++
> > > 13 files changed, 737 insertions(+), 4 deletions(-)
> > > create mode 100644 Documentation/security/keys/tee-trusted.rst
> > > create mode 100644 include/keys/tee_trusted.h
> > > create mode 100644 security/keys/tee_trusted.c
> > >
> > > --
> > > 2.7.4
> > >
^ permalink raw reply
* Re: [PATCH v2] Added warnings in checkpatch.pl script to :
From: Joe Perches @ 2019-07-09 5:57 UTC (permalink / raw)
To: NitinGote, akpm
Cc: corbet, apw, keescook, linux-doc, linux-kernel, kernel-hardening
In-Reply-To: <20190709054055.21984-1-nitin.r.gote@intel.com>
On Tue, 2019-07-09 at 11:10 +0530, NitinGote wrote:
> From: Nitin Gote <nitin.r.gote@intel.com>
>
> 1. Deprecate strcpy() in favor of strscpy().
> 2. Deprecate strlcpy() in favor of strscpy().
> 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
>
> Updated strncpy() section in Documentation/process/deprecated.rst
> to cover strscpy_pad() case.
>
> Acked-by: Kees Cook <keescook@chromium.org>
Kees, I think the concept is fine, but perhaps your
acked-by here isn't great. There are a few clear
defects in the checkpatch code that you also might
have overlooked.
> Change log:
> v1->v2
> - For string related apis, created different %deprecated_string_api
> and these will get emitted at CHECK Level using command line option
> -f/--file to avoid bad patched from novice script users.
>
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -605,6 +605,21 @@ foreach my $entry (keys %deprecated_apis) {
> }
> $deprecated_apis_search = "(?:${deprecated_apis_search})";
>
> +our %deprecated_string_apis = (
> + "strcpy" => "strscpy",
> + "strlcpy" => "strscpy",
> + "strncpy" => "strscpy, strscpy_pad or for non-NUL-terminated strings,
> + strncpy() can still be used, but destinations should be marked with the __nonstring",
This last strncpy line should not be on multiple lines.
checkpatch output is single line.
[]
> @@ -6446,6 +6461,16 @@ sub process {
> "Deprecated use of '$deprecated_api', prefer '$new_api' instead\n" . $herecurr);
> }
>
> +# check for string deprecated apis
> + if ($line =~ /\b($deprecated_string_apis_search)\b\s*\(/) {
> + my $deprecated_string_api = $1;
> + my $new_api = $deprecated_string_apis{$deprecated_string_api};
> + $check = 1;
> + CHK("DEPRECATED_API",
> + "Deprecated use of '$deprecated_string_api', prefer '$new_api' instead\n" . $herecurr);
> + $check = 0;
nack.
Please use consistent tab indentation and no,
do not set and unset $check.
Please use the same style as the rest of the script
when emitting at different levels for -f uses
my $msg_level = \&WARN;
$msg_level = \&CHK if ($file);
&{$msg_level}("DEPRECATED_API", etc...
^ permalink raw reply
* Re: [RFC 3/7] tee: add private login method for kernel clients
From: Sumit Garg @ 2019-07-09 5:56 UTC (permalink / raw)
To: Jens Wiklander
Cc: keyrings, linux-integrity, linux-security-module, corbet,
dhowells, jejb, Jarkko Sakkinen, Mimi Zohar, jmorris, serge,
Ard Biesheuvel, Daniel Thompson, linux-doc,
Linux Kernel Mailing List, tee-dev
In-Reply-To: <20190708153908.GA28253@jax>
Thanks Jens for your comments.
On Mon, 8 Jul 2019 at 21:09, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> Hi Sumit,
>
> On Thu, Jun 13, 2019 at 04:00:29PM +0530, Sumit Garg wrote:
> > There are use-cases where user-space shouldn't be allowed to communicate
> > directly with a TEE device which is dedicated to provide a specific
> > service for a kernel client. So add a private login method for kernel
> > clients and disallow user-space to open-session using this login method.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> > drivers/tee/tee_core.c | 6 ++++++
> > include/uapi/linux/tee.h | 2 ++
> > 2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> > index 0f16d9f..4581bd1 100644
> > --- a/drivers/tee/tee_core.c
> > +++ b/drivers/tee/tee_core.c
> > @@ -334,6 +334,12 @@ static int tee_ioctl_open_session(struct tee_context *ctx,
> > goto out;
> > }
> >
> > + if (arg.clnt_login == TEE_IOCTL_LOGIN_REE_KERNEL) {
> TEE_IOCTL_LOGIN_REE_KERNEL is defined as 0x80000000 which is in the
> range specified and implementation defined by the GP spec. I wonder if
> we shouldn't filter the entire implementation defined range instead of
> just this value.
Agree. Will rather check for entire implementation defined range:
0x80000000 - 0xFFFFFFFF.
>
> > + pr_err("login method not allowed for user-space client\n");
> pr_debug(), if it's needed at all.
>
Ok will use pr_debug() instead.
> > + rc = -EPERM;
> > + goto out;
> > + }
> > +
> > rc = ctx->teedev->desc->ops->open_session(ctx, &arg, params);
> > if (rc)
> > goto out;
> > diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
> > index 4b9eb06..f33c69c 100644
> > --- a/include/uapi/linux/tee.h
> > +++ b/include/uapi/linux/tee.h
> > @@ -172,6 +172,8 @@ struct tee_ioctl_buf_data {
> > #define TEE_IOCTL_LOGIN_APPLICATION 4
> > #define TEE_IOCTL_LOGIN_USER_APPLICATION 5
> > #define TEE_IOCTL_LOGIN_GROUP_APPLICATION 6
> > +/* Private login method for REE kernel clients */
> It's worth noting that this is filtered by the TEE framework, compared
> to everything else which is treated opaquely.
>
IIUC, you are referring to login filter in optee_os. Change to prevent
filter for this login method is part of this PR [1].
[1] https://github.com/OP-TEE/optee_os/pull/3082
-Sumit
> > +#define TEE_IOCTL_LOGIN_REE_KERNEL 0x80000000
> >
> > /**
> > * struct tee_ioctl_param - parameter
> > --
> > 2.7.4
> >
>
> Thanks,
> Jens
^ permalink raw reply
* [PATCH v2] Added warnings in checkpatch.pl script to :
From: NitinGote @ 2019-07-09 5:40 UTC (permalink / raw)
To: akpm, joe
Cc: corbet, apw, keescook, linux-doc, linux-kernel, kernel-hardening,
Nitin Gote
From: Nitin Gote <nitin.r.gote@intel.com>
1. Deprecate strcpy() in favor of strscpy().
2. Deprecate strlcpy() in favor of strscpy().
3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
Updated strncpy() section in Documentation/process/deprecated.rst
to cover strscpy_pad() case.
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Nitin Gote <nitin.r.gote@intel.com>
---
Change log:
v1->v2
- For string related apis, created different %deprecated_string_api
and these will get emitted at CHECK Level using command line option
-f/--file to avoid bad patched from novice script users.
This patch is already reviewed by mailing list
kernel-hardening@lists.openwall.com. Refer below link
<https://www.openwall.com/lists/kernel-hardening/2019/07/03/4>
Acked-by: Kees Cook <keescook@chromium.org>
Documentation/process/deprecated.rst | 6 +++---
scripts/checkpatch.pl | 25 +++++++++++++++++++++++++
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst
index 49e0f64a3427..f564de3caf76 100644
--- a/Documentation/process/deprecated.rst
+++ b/Documentation/process/deprecated.rst
@@ -93,9 +93,9 @@ will be NUL terminated. This can lead to various linear read overflows
and other misbehavior due to the missing termination. It also NUL-pads the
destination buffer if the source contents are shorter than the destination
buffer size, which may be a needless performance penalty for callers using
-only NUL-terminated strings. The safe replacement is :c:func:`strscpy`.
-(Users of :c:func:`strscpy` still needing NUL-padding will need an
-explicit :c:func:`memset` added.)
+only NUL-terminated strings. In this case, the safe replacement is
+:c:func:`strscpy`. If, however, the destination buffer still needs
+NUL-padding, the safe replacement is :c:func:`strscpy_pad`.
If a caller is using non-NUL-terminated strings, :c:func:`strncpy()` can
still be used, but destinations should be marked with the `__nonstring
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bb28b178d929..10bd72e99dc8 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -605,6 +605,21 @@ foreach my $entry (keys %deprecated_apis) {
}
$deprecated_apis_search = "(?:${deprecated_apis_search})";
+our %deprecated_string_apis = (
+ "strcpy" => "strscpy",
+ "strlcpy" => "strscpy",
+ "strncpy" => "strscpy, strscpy_pad or for non-NUL-terminated strings,
+ strncpy() can still be used, but destinations should be marked with the __nonstring",
+);
+
+#Create a search pattern for all these strings apis to speed up a loop below
+our $deprecated_string_apis_search = "";
+foreach my $entry (keys %deprecated_string_apis) {
+ $deprecated_string_apis_search .= '|' if ($deprecated_string_apis_search ne "");
+ $deprecated_string_apis_search .= $entry;
+}
+$deprecated_string_apis_search = "(?:${deprecated_string_apis_search})";
+
our $mode_perms_world_writable = qr{
S_IWUGO |
S_IWOTH |
@@ -6446,6 +6461,16 @@ sub process {
"Deprecated use of '$deprecated_api', prefer '$new_api' instead\n" . $herecurr);
}
+# check for string deprecated apis
+ if ($line =~ /\b($deprecated_string_apis_search)\b\s*\(/) {
+ my $deprecated_string_api = $1;
+ my $new_api = $deprecated_string_apis{$deprecated_string_api};
+ $check = 1;
+ CHK("DEPRECATED_API",
+ "Deprecated use of '$deprecated_string_api', prefer '$new_api' instead\n" . $herecurr);
+ $check = 0;
+ }
+
# check for various structs that are normally const (ops, kgdb, device_tree)
# and avoid what seem like struct definitions 'struct foo {'
if ($line !~ /\bconst\b/ &&
--
2.17.1
^ permalink raw reply related
* [PATCH 00/11] kbuild: create *.mod with directory path and remove MODVERDIR
From: Masahiro Yamada @ 2019-07-09 4:24 UTC (permalink / raw)
To: linux-kbuild
Cc: Nicolas Pitre, Sam Ravnborg, Masahiro Yamada, linux-scsi,
linux-doc, linux-kernel, Jonathan Corbet, Michal Marek,
Martin K. Petersen, James E.J. Bottomley
This series kills the long standing MODVERDIR.
Since MODVERDIR has a flat structure, it cannot avoid a race
condition when somebody introduces a module name conflict.
Kbuild now reads modules.order to get the list of all modules.
The post-processing/installation stages will be more robust
and simpler.
Masahiro Yamada (11):
kbuild: do not create empty modules.order in the prepare stage
kbuild: get rid of kernel/ prefix from in-tree modules.{order,builtin}
kbuild: remove duplication from modules.order in sub-directories
scsi: remove pointless $(MODVERDIR)/$(obj)/53c700.ver
kbuild: modinst: read modules.order instead of $(MODVERDIR)/*.mod
kbuild: modsign: read modules.order instead of $(MODVERDIR)/*.mod
kbuild: modpost: read modules.order instead of $(MODVERDIR)/*.mod
kbuild: create *.mod with full directory path and remove MODVERDIR
kbuild: remove the first line of *.mod files
kbuild: remove 'prepare1' target
kbuild: split out *.mod out of {single,multi}-used-m rules
.gitignore | 1 +
Documentation/dontdiff | 1 +
Makefile | 33 +++++++++------------------------
drivers/scsi/Makefile | 2 +-
scripts/Makefile.build | 33 ++++++++++++++++-----------------
scripts/Makefile.modbuiltin | 2 +-
scripts/Makefile.modinst | 5 +----
scripts/Makefile.modpost | 17 +++++++++--------
scripts/Makefile.modsign | 3 +--
scripts/adjust_autoksyms.sh | 11 ++++-------
scripts/mod/sumversion.c | 23 ++++-------------------
scripts/modules-check.sh | 2 +-
scripts/package/mkspec | 2 +-
13 files changed, 50 insertions(+), 85 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH 08/11] kbuild: create *.mod with full directory path and remove MODVERDIR
From: Masahiro Yamada @ 2019-07-09 4:24 UTC (permalink / raw)
To: linux-kbuild
Cc: Nicolas Pitre, Sam Ravnborg, Masahiro Yamada, linux-doc,
Jonathan Corbet, linux-kernel, Michal Marek
In-Reply-To: <20190709042416.27554-1-yamada.masahiro@socionext.com>
While descending directories, Kbuild produces objects for modules,
but final *.ko files are linked in the modpost.
To keep track of modules, Kbuild creates a *.mod file in $(MODVERDIR)
for every module it is building. Some post-processing steps read the
necessary information from *.mod files. This avoids descending into
directories again. This mechanism was introduced in 2003 or so.
Later, commit 551559e13af1 ("kbuild: implement modules.order") added
modules.order. So, we can simply read it out to know all the modules
with directory paths. This is easier than parsing the first line of
*.mod files.
$(MODVERDIR) has a flat structure. This is based on the assumption
that the module name is unique. This assumption is really fragile.
Stephen Rothwell reported a race condition caused by a module name
conflict:
https://lkml.org/lkml/2019/5/13/991
In parallel building, two different threads could write to the same
$(MODVERDIR)/*.mod simultaneously.
Non-unique module names are the source of all kind of troubles, hence
commit 3a48a91901c5 ("kbuild: check uniqueness of module names")
introduced a new checker script.
However, it is still fragile in the build system point of view because
this race happens before scripts/modules-check.sh is invoked. If it
happens again, the modpost will emit unclear error messages.
To fix this issue completely, create *.mod in the same directory as
*.ko so that two threads never attempt to write to the same .mod file.
$(MODVERDIR) is no longer needed.
Since modules with directory paths are listed in modules.order, Kbuild
is still able to find *.mod files without additional descending.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
.gitignore | 1 +
Documentation/dontdiff | 1 +
Makefile | 20 +++-----------------
scripts/Makefile.build | 8 ++------
scripts/Makefile.modpost | 4 ++--
scripts/adjust_autoksyms.sh | 11 ++++-------
scripts/mod/sumversion.c | 16 +++-------------
scripts/package/mkspec | 2 +-
8 files changed, 17 insertions(+), 46 deletions(-)
diff --git a/.gitignore b/.gitignore
index 7587ef56b92d..8f5422cba6e2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -30,6 +30,7 @@
*.lz4
*.lzma
*.lzo
+*.mod
*.mod.c
*.o
*.o.*
diff --git a/Documentation/dontdiff b/Documentation/dontdiff
index 5eba889ea84d..9f4392876099 100644
--- a/Documentation/dontdiff
+++ b/Documentation/dontdiff
@@ -30,6 +30,7 @@
*.lzo
*.mo
*.moc
+*.mod
*.mod.c
*.o
*.o.*
diff --git a/Makefile b/Makefile
index 2f40cf704119..958fafa605b9 100644
--- a/Makefile
+++ b/Makefile
@@ -488,11 +488,6 @@ export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE KBUILD_LDFLAGS_MODULE
export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL
export KBUILD_ARFLAGS
-# When compiling out-of-tree modules, put MODVERDIR in the module
-# tree rather than in the kernel tree. The kernel tree might
-# even be read-only.
-export MODVERDIR := $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_versions
-
# Files to ignore in find ... statements
export RCS_FIND_IGNORE := \( -name SCCS -o -name BitKeeper -o -name .svn -o \
@@ -1034,7 +1029,7 @@ vmlinux-deps := $(KBUILD_LDS) $(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)
# Recurse until adjust_autoksyms.sh is satisfied
PHONY += autoksyms_recursive
-autoksyms_recursive: $(vmlinux-deps)
+autoksyms_recursive: $(vmlinux-deps) modules.order
ifdef CONFIG_TRIM_UNUSED_KSYMS
$(Q)$(CONFIG_SHELL) $(srctree)/scripts/adjust_autoksyms.sh \
"$(MAKE) -f $(srctree)/Makefile vmlinux"
@@ -1118,7 +1113,6 @@ endif
prepare1: prepare3 outputmakefile asm-generic $(version_h) $(autoksyms_h) \
include/generated/utsrelease.h
- $(cmd_crmodverdir)
archprepare: archheaders archscripts prepare1 scripts
@@ -1376,7 +1370,7 @@ endif # CONFIG_MODULES
# make distclean Remove editor backup files, patch leftover files and the like
# Directories & files removed with 'make clean'
-CLEAN_DIRS += $(MODVERDIR) include/ksym
+CLEAN_DIRS += include/ksym
CLEAN_FILES += modules.builtin.modinfo
# Directories & files removed with 'make mrproper'
@@ -1637,7 +1631,6 @@ PHONY += $(clean-dirs) clean
$(clean-dirs):
$(Q)$(MAKE) $(clean)=$(patsubst _clean_%,%,$@)
-clean: rm-dirs := $(MODVERDIR)
clean: rm-files := $(KBUILD_EXTMOD)/Module.symvers
PHONY += help
@@ -1651,8 +1644,6 @@ help:
@echo ''
PHONY += prepare
-prepare:
- $(cmd_crmodverdir)
endif # KBUILD_EXTMOD
clean: $(clean-dirs)
@@ -1663,7 +1654,7 @@ clean: $(clean-dirs)
-o -name '*.ko.*' \
-o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \
-o -name '*.dwo' -o -name '*.lst' \
- -o -name '*.su' \
+ -o -name '*.su' -o -name '*.mod' \
-o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \
-o -name '*.lex.c' -o -name '*.tab.[ch]' \
-o -name '*.asn1.[ch]' \
@@ -1792,11 +1783,6 @@ quiet_cmd_depmod = DEPMOD $(KERNELRELEASE)
cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
$(KERNELRELEASE)
-# Create temporary dir for module support files
-# clean it up only when building all modules
-cmd_crmodverdir = $(Q)mkdir -p $(MODVERDIR) \
- $(if $(KBUILD_MODULES),; rm -f $(MODVERDIR)/*)
-
# read saved command lines for existing targets
existing-targets := $(wildcard $(sort $(targets)))
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 1492e37c3927..1d01324455d1 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -69,8 +69,6 @@ modorder-target := $(obj)/modules.order
endif
endif
-# We keep a list of all modules in $(MODVERDIR)
-
__build: $(if $(KBUILD_BUILTIN),$(builtin-target) $(lib-target) $(extra-y)) \
$(if $(KBUILD_MODULES),$(obj-m) $(modorder-target)) \
$(subdir-ym) $(always)
@@ -280,13 +278,11 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
$(call cmd,force_checksrc)
$(call if_changed_rule,cc_o_c)
-# Single-part modules are special since we need to mark them in $(MODVERDIR)
-
$(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
$(call cmd,force_checksrc)
$(call if_changed_rule,cc_o_c)
@{ echo $(@:.o=.ko); echo $@; \
- $(cmd_undef_syms); } > $(MODVERDIR)/$(@F:.o=.mod)
+ $(cmd_undef_syms); } > $(patsubst %.o,%.mod,$@)
quiet_cmd_cc_lst_c = MKLST $@
cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
@@ -468,7 +464,7 @@ cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^) $(cmd_secanalysis
$(multi-used-m): FORCE
$(call if_changed,link_multi-m)
@{ echo $(@:.o=.ko); echo $(filter %.o,$^); \
- $(cmd_undef_syms); } > $(MODVERDIR)/$(@F:.o=.mod)
+ $(cmd_undef_syms); } > $(patsubst %.o,%.mod,$@)
$(call multi_depend, $(multi-used-m), .o, -objs -y -m)
targets += $(multi-used-m)
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 2ab1694a7df3..3e229d4f4d72 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -6,8 +6,8 @@
# Stage one of module building created the following:
# a) The individual .o files used for the module
# b) A <module>.o file which is the .o files above linked together
-# c) A <module>.mod file in $(MODVERDIR)/, listing the name of the
-# the preliminary <module>.o file, plus all .o files
+# c) A <module>.mod file, listing the name of the preliminary <module>.o file,
+# plus all .o files
# d) modules.order, which lists all the modules
# Stage 2 is handled by this file and does the following
diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index aab4e299d7a2..d3561ff4089c 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -47,13 +47,10 @@ cat > "$new_ksyms_file" << EOT
*/
EOT
-[ "$(ls -A "$MODVERDIR")" ] &&
-for mod in "$MODVERDIR"/*.mod; do
- sed -n -e '3{s/ /\n/g;/^$/!p;}' "$mod"
-done | sort -u |
-while read sym; do
- echo "#define __KSYM_${sym} 1"
-done >> "$new_ksyms_file"
+sed 's/ko$/mod/' modules.order |
+xargs -r -n1 sed -n -e '3{s/ /\n/g;/^$/!p;}' |
+sort -u |
+sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$new_ksyms_file"
# Special case for modversions (see modpost.c)
if [ -n "$CONFIG_MODVERSIONS" ]; then
diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
index 0f6dcb4011a8..166f3fa247a9 100644
--- a/scripts/mod/sumversion.c
+++ b/scripts/mod/sumversion.c
@@ -396,21 +396,11 @@ void get_src_version(const char *modname, char sum[], unsigned sumlen)
unsigned long len;
struct md4_ctx md;
char *sources, *end, *fname;
- const char *basename;
char filelist[PATH_MAX + 1];
- char *modverdir = getenv("MODVERDIR");
- if (!modverdir)
- modverdir = ".";
-
- /* Source files for module are in .tmp_versions/modname.mod,
- after the first line. */
- if (strrchr(modname, '/'))
- basename = strrchr(modname, '/') + 1;
- else
- basename = modname;
- snprintf(filelist, sizeof(filelist), "%s/%.*s.mod", modverdir,
- (int) strlen(basename) - 2, basename);
+ /* objects for a module are listed in the second line of *.mod file. */
+ snprintf(filelist, sizeof(filelist), "%.*smod",
+ (int)strlen(modname) - 1, modname);
file = grab_file(filelist, &len);
if (!file)
diff --git a/scripts/package/mkspec b/scripts/package/mkspec
index 2d29df4a0a53..8640c278f1aa 100755
--- a/scripts/package/mkspec
+++ b/scripts/package/mkspec
@@ -29,7 +29,7 @@ fi
PROVIDES="$PROVIDES kernel-$KERNELRELEASE"
__KERNELRELEASE=$(echo $KERNELRELEASE | sed -e "s/-/_/g")
-EXCLUDES="$RCS_TAR_IGNORE --exclude=.tmp_versions --exclude=*vmlinux* \
+EXCLUDES="$RCS_TAR_IGNORE --exclude=*vmlinux* --exclude=*.mod \
--exclude=*.o --exclude=*.ko --exclude=*.cmd --exclude=Documentation \
--exclude=.config.old --exclude=.missing-syscalls.d --exclude=*.s"
--
2.17.1
^ permalink raw reply related
* Re: [PATCH next v2] softirq: enable MAX_SOFTIRQ_TIME tuning with sysctl, max_softirq_time_msecs
From: Zhiqiang Liu @ 2019-07-09 1:32 UTC (permalink / raw)
To: tglx, corbet, Kees Cook, Eric Dumazet
Cc: Andrew Morton, manfred, jwilk, dvyukov, feng.tang, sunilmut,
quentin.perret, linux, alex.popov, linux-doc, linux-kernel,
linux-fsdevel, wangxiaogang (F), Zhoukang (A), Mingfangsen,
tedheadster
In-Reply-To: <64823387-b0f7-a838-01ea-24ae386f2272@huawei.com>
Friendly ping ...
On 2019/6/28 14:52, Zhiqiang Liu wrote:
> Friendly ping ...
>
> On 2019/6/25 11:13, Zhiqiang Liu wrote:
>> From: Zhiqiang liu <liuzhiqiang26@huawei.com>
>>
>> In __do_softirq func, MAX_SOFTIRQ_TIME was set to 2ms via experimentation by
>> commit c10d73671 ("softirq: reduce latencies") in 2013, which was designed
>> to reduce latencies for various network workloads. The key reason is that the
>> maximum number of microseconds in one NAPI polling cycle in net_rx_action func
>> was set to 2 jiffies, so different HZ settting will lead to different latencies.
>>
>> However, commit 7acf8a1e8 ("Replace 2 jiffies with sysctl netdev_budget_usecs
>> to enable softirq tuning") adopts netdev_budget_usecs to tun maximum number of
>> microseconds in one NAPI polling cycle. So the latencies of net_rx_action can be
>> controlled by sysadmins to copy with hardware changes over time.
>>
>> Correspondingly, the MAX_SOFTIRQ_TIME should be able to be tunned by sysadmins,
>> who knows best about hardware performance, for excepted tradeoff between latence
>> and fairness. Here, we add sysctl variable max_softirq_time_msecs to replace
>> MAX_SOFTIRQ_TIME with 2ms default value.
>>
>> Note: max_softirq_time_msecs will be coverted to jiffies, and any budget
>> value will be rounded up to the next jiffies, which relates to CONFIG_HZ.
>> The time accuracy of jiffies will result in a certain difference
>> between the setting jiffies of max_softirq_time_msecs and the actual
>> value, which is in one jiffies range.
>>
>> Signed-off-by: Zhiqiang liu <liuzhiqiang26@huawei.com>
>> ---
>> Documentation/sysctl/kernel.txt | 17 +++++++++++++++++
>> kernel/softirq.c | 8 +++++---
>> kernel/sysctl.c | 9 +++++++++
>> 3 files changed, 31 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
>> index f0c86fbb3b48..23b36393f150 100644
>> --- a/Documentation/sysctl/kernel.txt
>> +++ b/Documentation/sysctl/kernel.txt
>> @@ -44,6 +44,7 @@ show up in /proc/sys/kernel:
>> - kexec_load_disabled
>> - kptr_restrict
>> - l2cr [ PPC only ]
>> +- max_softirq_time_msecs
>> - modprobe ==> Documentation/debugging-modules.txt
>> - modules_disabled
>> - msg_next_id [ sysv ipc ]
>> @@ -445,6 +446,22 @@ This flag controls the L2 cache of G3 processor boards. If
>>
>> ==============================================================
>>
>> +max_softirq_time_msecs:
>> +
>> +Maximum number of milliseconds to break the loop of restarting softirq
>> +processing for at most MAX_SOFTIRQ_RESTART times in __do_softirq().
>> +max_softirq_time_msecs will be coverted to jiffies, and any budget
>> +value will be rounded up to the next jiffies, which relates to CONFIG_HZ.
>> +The time accuracy of jiffies will result in a certain difference
>> +between the setting jiffies of max_softirq_time_msecs and the actual
>> +value, which is in one jiffies range.
>> +
>> +max_softirq_time_msecs is a non-negative integer value, and setting
>> +negative value is meaningless and will return error.
>> +Default: 2
>> +
>> +==============================================================
>> +
>> modules_disabled:
>>
>> A toggle value indicating if modules are allowed to be loaded
>> diff --git a/kernel/softirq.c b/kernel/softirq.c
>> index a6b81c6b6bff..1e456db70093 100644
>> --- a/kernel/softirq.c
>> +++ b/kernel/softirq.c
>> @@ -199,7 +199,8 @@ EXPORT_SYMBOL(__local_bh_enable_ip);
>>
>> /*
>> * We restart softirq processing for at most MAX_SOFTIRQ_RESTART times,
>> - * but break the loop if need_resched() is set or after 2 ms.
>> + * but break the loop if need_resched() is set or after
>> + * max_softirq_time_msecs msecs.
>> * The MAX_SOFTIRQ_TIME provides a nice upper bound in most cases, but in
>> * certain cases, such as stop_machine(), jiffies may cease to
>> * increment and so we need the MAX_SOFTIRQ_RESTART limit as
>> @@ -210,7 +211,7 @@ EXPORT_SYMBOL(__local_bh_enable_ip);
>> * we want to handle softirqs as soon as possible, but they
>> * should not be able to lock up the box.
>> */
>> -#define MAX_SOFTIRQ_TIME msecs_to_jiffies(2)
>> +unsigned int __read_mostly max_softirq_time_msecs = 2;
>> #define MAX_SOFTIRQ_RESTART 10
>>
>> #ifdef CONFIG_TRACE_IRQFLAGS
>> @@ -248,7 +249,8 @@ static inline void lockdep_softirq_end(bool in_hardirq) { }
>>
>> asmlinkage __visible void __softirq_entry __do_softirq(void)
>> {
>> - unsigned long end = jiffies + MAX_SOFTIRQ_TIME;
>> + unsigned long end = jiffies +
>> + msecs_to_jiffies(max_softirq_time_msecs);
>> unsigned long old_flags = current->flags;
>> int max_restart = MAX_SOFTIRQ_RESTART;
>> struct softirq_action *h;
>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>> index 1beca96fb625..96ff292ce7f6 100644
>> --- a/kernel/sysctl.c
>> +++ b/kernel/sysctl.c
>> @@ -118,6 +118,7 @@ extern unsigned int sysctl_nr_open_min, sysctl_nr_open_max;
>> #ifndef CONFIG_MMU
>> extern int sysctl_nr_trim_pages;
>> #endif
>> +extern unsigned int max_softirq_time_msecs;
>>
>> /* Constants used for minimum and maximum */
>> #ifdef CONFIG_LOCKUP_DETECTOR
>> @@ -1276,6 +1277,14 @@ static struct ctl_table kern_table[] = {
>> .extra2 = &one,
>> },
>> #endif
>> + {
>> + .procname = "max_softirq_time_msecs",
>> + .data = &max_softirq_time_msecs,
>> + .maxlen = sizeof(unsigned int),
>> + .mode = 0644,
>> + .proc_handler = proc_dointvec_minmax,
>> + .extra1 = &zero,
>> + },
>> { }
>> };
>>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH next] softirq: enable MAX_SOFTIRQ_TIME tuning with sysctl max_softirq_time_usecs
From: Zhiqiang Liu @ 2019-07-09 1:25 UTC (permalink / raw)
To: Thomas Gleixner
Cc: corbet, mcgrof, Kees Cook, akpm, manfred, jwilk, dvyukov,
feng.tang, sunilmut, quentin.perret, linux, alex.popov, linux-doc,
linux-kernel, linux-fsdevel, wangxiaogang (F), Zhoukang (A),
Mingfangsen, tedheadster, Eric Dumazet
In-Reply-To: <alpine.DEB.2.21.1907081558400.4709@nanos.tec.linutronix.de>
On 2019/7/8 22:14, Thomas Gleixner wrote:
> Zhiqiang,
>
>> If HZ satisfies the condition: HZ <= MSEC_PER_SEC && !(MSEC_PER_SEC %
>> HZ), the return value of _msecs_to_jiffies func with m=0 is different
>> with different HZ setting.
>
>> ------------------------------------
>> | HZ | MSEC_PER_SEC / HZ | return |
>> ------------------------------------
>> |1000| 1 | 0 |
>> |500 | 2 | 1 |
>> |200 | 5 | 1 |
>> |100 | 10 | 1 |
>> ------------------------------------
>>
>> Why only the return value of HZ=1000 is equal to 0 with m=0 ?
>
> I don't know how you tested that, but obviously all four HZ values use
> this variant:
>
>> #if HZ <= MSEC_PER_SEC && !(MSEC_PER_SEC % HZ)
>> static inline unsigned long _msecs_to_jiffies(const unsigned int m)
>> {
>> return (m + (MSEC_PER_SEC / HZ) - 1) / (MSEC_PER_SEC / HZ);
>> }
>
> and for all four HZ values the result is 0. Why?
>
> For m = 0 the calculation reduces to:
>
> ((MSEC_PER_SEC / HZ) - 1) / (MSEC_PER_SEC / HZ)
>
> i.e.
>
> (x - 1) / x where x = [1, 2, 5, 10]
>
> which is guaranteed to be 0 for integer math. If not, you have a compiler
> problem.
>
> Thanks,
>
> tglx
Thanks for your reply. Actually, I have made a low-level mistake.
I am really sorry for that.
Thanks again.
^ permalink raw reply
* Re: [PATCH 3/7] kbuild: do not create wrappers for header-test-y
From: Masahiro Yamada @ 2019-07-09 1:10 UTC (permalink / raw)
To: Linux Kbuild mailing list
Cc: Sam Ravnborg, Joel Fernandes, open list:DOCUMENTATION,
Jonathan Corbet, Linux Kernel Mailing List, Michal Marek
In-Reply-To: <20190701005845.12475-4-yamada.masahiro@socionext.com>
On Mon, Jul 1, 2019 at 10:00 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> header-test-y does not work with headers in sub-directories.
>
> For example, you may want to write a Makefile, like this:
>
> include/linux/Kbuild:
>
> header-test-y += mtd/nand.h
>
> This entry will create a wrapper include/linux/mtd/nand.hdrtest.c
> with the following content:
>
> #include "mtd/nand.h"
>
> To make this work, we need to add $(srctree)/include/linux to the
> header search path. It would be tedious to add ccflags-y.
>
> Instead, we could change the *.hdrtest.c rule to wrap:
>
> #include "nand.h"
>
> This works for in-tree build since #include "..." searches in the
> relative path from the header with this directive. For O=... build,
> we need to add $(srctree)/include/linux/mtd to the header search path,
> which will be even more tedious.
>
> After all, I thought it would be handier to compile headers directly
> without creating wrappers.
>
> I added a new build rule to compile %.h into %.h.s
>
> The target is %.h.s instead of %.h.o because it is slightly faster.
> Also, as for GCC, an empty assembly is smaller than an empty object.
>
> I wrote the build rule:
>
> $(CC) $(c_flags) -S -o $@ -x c /dev/null -include $<
>
> instead of:
>
> $(CC) $(c_flags) -S -o $@ -x c $<
>
> Both work fine with GCC, but the latter is bad for Clang.
>
> This comes down to the difference in the -Wunused-function policy.
> GCC does not warn about unused 'static inline' functions at all.
> Clang does not warn about the ones in included headers, but does
> about the ones in the source. So, we should handle headers as
> headers, not as source files.
>
> In fact, this has been hidden since commit abb2ea7dfd82 ("compiler,
> clang: suppress warning for unused static inline functions"), but we
> should not rely on that.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Tested-by: Jani Nikula <jani.nikula@intel.com>
> ---
To exclude *.h.s from kernel-devel rpm,
I will squash the following change:
diff --git a/scripts/package/mkspec b/scripts/package/mkspec
index 009147d4718e..2d29df4a0a53 100755
--- a/scripts/package/mkspec
+++ b/scripts/package/mkspec
@@ -31,7 +31,7 @@ PROVIDES="$PROVIDES kernel-$KERNELRELEASE"
__KERNELRELEASE=$(echo $KERNELRELEASE | sed -e "s/-/_/g")
EXCLUDES="$RCS_TAR_IGNORE --exclude=.tmp_versions --exclude=*vmlinux* \
--exclude=*.o --exclude=*.ko --exclude=*.cmd --exclude=Documentation \
---exclude=.config.old --exclude=.missing-syscalls.d"
+--exclude=.config.old --exclude=.missing-syscalls.d --exclude=*.s"
# We can label the here-doc lines for conditional output to the spec file
#
--
Best Regards
Masahiro Yamada
^ permalink raw reply related
* Re: [PATCH v2] kbuild: get rid of misleading $(AS) from documents
From: Masahiro Yamada @ 2019-07-09 1:06 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Linux Kbuild mailing list, Sam Ravnborg, open list:DOCUMENTATION,
Linux Kernel Mailing List, Michal Marek
In-Reply-To: <20190708140223.39d15d56@lwn.net>
On Tue, Jul 9, 2019 at 5:02 AM Jonathan Corbet <corbet@lwn.net> wrote:
>
> On Sun, 7 Jul 2019 01:25:08 +0900
> Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
> > The assembler files in the kernel are *.S instead of *.s, so they must
> > be preprocessed. Since 'as' of GNU binutils is not able to preprocess,
> > we always use $(CC) as an assembler driver.
> >
> > $(AS) is almost unused in Kbuild. As of writing, there is just one place
> > that directly invokes $(AS).
> >
> > $ git grep -e '$(AS)' -e '${AS}' -e '$AS' -e '$(AS:' -e '${AS:' -- :^Documentation
> > drivers/net/wan/Makefile: AS68K = $(AS)
> >
> > The documentation about *_AFLAGS* sounds like the flags were passed
> > to $(AS). This is somewhat misleading.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
>
> Would you like me to send this up through the docs tree?
No, I will apply it to linux-kbuild shortly.
Thanks.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 0/2] arm64: Introduce boot parameter to disable TLB flush instruction within the same inner shareable domain
From: Jon Masters @ 2019-07-09 0:25 UTC (permalink / raw)
To: qi.fuli@fujitsu.com, Will Deacon
Cc: Will Deacon, indou.takao@fujitsu.com, linux-doc@vger.kernel.org,
peterz@infradead.org, Catalin Marinas, Jonathan Corbet,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <5999ed84-72d0-9d42-bf7d-b8d56eaa4d4a@jp.fujitsu.com>
On 7/2/19 10:45 PM, qi.fuli@fujitsu.com wrote:
> However, we found that with the increase of that the TLB flash was called,
> the noise was also increasing. Here we understood that the cause of this
> issue is the implementation of Linux's TLB flush for arm64, especially use of
> TLBI-is instruction which is a broadcast to all processor core on the system.
Are you saying that for a microbenchmark in which very large numbers of
threads are created and destroyed rapidly there are a large number of
associated tlb range flushes which always use broadcast TLBIs?
If that's the case, and the hardware doesn't do any ASID filtering and
each TLBI results in a DVM to every PE, would it make sense to look at
whether there are ways to improve batching/switch to an IPI approach
rather than relying on broadcasts, as a more generic solution?
Jon.
^ permalink raw reply
* Re: [PATCH 0/2] arm64: Introduce boot parameter to disable TLB flush instruction within the same inner shareable domain
From: Jon Masters @ 2019-07-09 0:29 UTC (permalink / raw)
To: qi.fuli@fujitsu.com, Will Deacon
Cc: Will Deacon, indou.takao@fujitsu.com, linux-doc@vger.kernel.org,
peterz@infradead.org, Catalin Marinas, Jonathan Corbet,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <675313fe-007b-c850-d730-a629b82ccfc8@jonmasters.org>
On 7/8/19 8:25 PM, Jon Masters wrote:
> On 7/2/19 10:45 PM, qi.fuli@fujitsu.com wrote:
>
>> However, we found that with the increase of that the TLB flash was called,
>> the noise was also increasing. Here we understood that the cause of this
>> issue is the implementation of Linux's TLB flush for arm64, especially use of
>> TLBI-is instruction which is a broadcast to all processor core on the system.
>
> Are you saying that for a microbenchmark in which very large numbers of
> threads are created and destroyed rapidly there are a large number of
> associated tlb range flushes which always use broadcast TLBIs?
>
> If that's the case, and the hardware doesn't do any ASID filtering and
> each TLBI results in a DVM to every PE, would it make sense to look at
> whether there are ways to improve batching/switch to an IPI approach
> rather than relying on broadcasts, as a more generic solution?
What I meant was a heuristic to do this automatically, rather than via a
command line.
Jon.
^ permalink raw reply
* Re: [v1 0/5] allow to reserve memory for normal kexec kernel
From: Pavel Tatashin @ 2019-07-09 0:09 UTC (permalink / raw)
To: Eric W. Biederman
Cc: James Morris, Sasha Levin, kexec, LKML, corbet, Catalin Marinas,
will, linux-doc, Linux ARM
In-Reply-To: <87sgrgjd6i.fsf@xmission.com>
> Something is very very wrong there.
>
> Last I measured memory bandwidth seriously I could touch a Gigabyte per
> second easily, and that was nearly 20 years ago. Did you manage to
> disable caching or have some particularly slow code that does the
> reolocations.
>
> There is a serious cost to reserving memory in that it is simply not
> available at other times. For kexec on panic there is no other reliable
> way to get memory that won't be DMA'd to.
Hi Eric,
Thank you for your comments.
Indeed, but sometimes fast reboot is more important than the cost of
reserving 32M-64M of memory.
>
> We have options in this case and I would strongly encourage you to track
> down why that copy in relocation is so very slow. I suspect a 4KiB page
> size is large enough that it can swamp pointer following costs.
>
> My back of the napkin math says even 20 years ago your copying costs
> should be only 0.037s. The only machine I have ever tested on where
> the copy costs were noticable was my old 386.
>
> Maybe I am out to lunch here but a claim that your memory only runs
> at 100MiB/s (the speed of my spinning rust hard drive) is rather
> incredible.
I agree, my measurement on this machine was 2,857MB/s. Perhaps when
MMU is disabled ARM64 also has caching disabled? The function that
loops through array of pages and relocates them to final destination
is this:
https://soleen.com/source/xref/linux/arch/arm64/kernel/relocate_kernel.S?r=d2912cb1#29
A comment before calling it:
205 /*
206 * cpu_soft_restart will shutdown the MMU, disable data caches, then
207 * transfer control to the reboot_code_buffer which contains a copy of
208 * the arm64_relocate_new_kernel routine. arm64_relocate_new_kernel
209 * uses physical addressing to relocate the new image to its final
210 * position and transfers control to the image entry point when the
211 * relocation is complete.
212 * In kexec case, kimage->start points to purgatory assuming that
213 * kernel entry and dtb address are embedded in purgatory by
214 * userspace (kexec-tools).
215 * In kexec_file case, the kernel starts directly without purgatory.
216 */
https://soleen.com/source/xref/linux/arch/arm64/kernel/machine_kexec.c?r=d2912cb1#206
So, as I understand at least data caches are disabled, and MMU is
disabled, perhaps this is why this function is so incredibly slow?
Perhaps, there is a better way to fix this problem by keeping caches
enabled while still relocating? Any suggestions from Aarch64
developers?
Pasha
^ permalink raw reply
* Re: [v1 0/5] allow to reserve memory for normal kexec kernel
From: Eric W. Biederman @ 2019-07-08 23:53 UTC (permalink / raw)
To: Pavel Tatashin
Cc: jmorris, sashal, kexec, linux-kernel, corbet, catalin.marinas,
will, linux-doc, linux-arm-kernel
In-Reply-To: <20190708211528.12392-1-pasha.tatashin@soleen.com>
Pavel Tatashin <pasha.tatashin@soleen.com> writes:
> Currently, it is only allowed to reserve memory for crash kernel, because
> it is a requirement in order to be able to boot into crash kernel without
> touching memory of crashed kernel is to have memory reserved.
>
> The second benefit for having memory reserved for kexec kernel is
> that it does not require a relocation after segments are loaded into
> memory.
>
> If kexec functionality is used for a fast system update, with a minimal
> downtime, the relocation of kernel + initramfs might take a significant
> portion of reboot.
>
> In fact, on the machine that we are using, that has ARM64 processor
> it takes 0.35s to relocate during kexec, thus taking 52% of kernel reboot
> time:
>
> kernel shutdown 0.03s
> relocation 0.35s
> kernel startup 0.29s
>
> Image: 13M and initramfs is 24M. If initramfs increases, the relocation
> time increases proportionally.
Something is very very wrong there.
Last I measured memory bandwidth seriously I could touch a Gigabyte per
second easily, and that was nearly 20 years ago. Did you manage to
disable caching or have some particularly slow code that does the
reolocations.
There is a serious cost to reserving memory in that it is simply not
available at other times. For kexec on panic there is no other reliable
way to get memory that won't be DMA'd to.
We have options in this case and I would strongly encourage you to track
down why that copy in relocation is so very slow. I suspect a 4KiB page
size is large enough that it can swamp pointer following costs.
My back of the napkin math says even 20 years ago your copying costs
should be only 0.037s. The only machine I have ever tested on where
the copy costs were noticable was my old 386.
Maybe I am out to lunch here but a claim that your memory only runs
at 100MiB/s (the speed of my spinning rust hard drive) is rather
incredible.
Eric
^ permalink raw reply
* Re: [PATCH v6 18/18] MAINTAINERS: add proc sysctl KUnit test to PROC SYSCTL section
From: Iurii Zaikin @ 2019-07-08 23:34 UTC (permalink / raw)
To: Brendan Higgins
Cc: Luis Chamberlain, Frank Rowand, Greg KH, Josh Poimboeuf,
Kees Cook, Kieran Bingham, Peter Zijlstra, Rob Herring,
Stephen Boyd, shuah, Theodore Ts'o, Masahiro Yamada,
devicetree, dri-devel, kunit-dev, open list:DOCUMENTATION,
linux-fsdevel, linux-kbuild, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, linux-nvdimm, linux-um,
Sasha Levin, Bird, Timothy, Amir Goldstein, Dan Carpenter,
Daniel Vetter, Jeff Dike, Joel Stanley, Julia Lawall,
Kevin Hilman, Knut Omang, Logan Gunthorpe, Michael Ellerman,
Petr Mladek, Randy Dunlap, Richard Weinberger, David Rientjes,
Steven Rostedt, wfg
In-Reply-To: <CAAXuY3q1==RvAiw+gw5kfFJmjdR9JEUnnxou4Sv0POd88aD41w@mail.gmail.com>
> On Mon, Jul 8, 2019 at 4:16 PM Brendan Higgins <brendanhiggins@google.com> wrote:
>>
>> CC'ing Iurii Zaikin
>>
>> On Fri, Jul 5, 2019 at 1:48 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>> >
>> > On Wed, Jul 03, 2019 at 05:36:15PM -0700, Brendan Higgins wrote:
>> > > Add entry for the new proc sysctl KUnit test to the PROC SYSCTL section.
>> > >
>> > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>> > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> > > Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
>> > > Acked-by: Luis Chamberlain <mcgrof@kernel.org>
>> >
>> > Come to think of it, I'd welcome Iurii to be added as a maintainer,
>> > with the hope Iurii would be up to review only the kunit changes. Of
>> > course if Iurii would be up to also help review future proc changes,
>> > even better. 3 pair of eyeballs is better than 2 pairs.
>>
>> What do you think, Iurii?
I'm in.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox