On this v9 I've dropped the generic sysfs deadlock fix given Ming Lei
has provided alternative fixes for the zram driver without incurring
a generic lock *and* we don't yet have full assessment of how wide
spread the deadlock case might be in the kernel. A full assessment
effort is still underway using Coccinelle with iteration support,
however that effort will take a bit more time to complete. We can
re-evaluate the value of a generic fix later after the assessment
is complete.
This series now just adds the test_sysfs selftest and failure injection
support for it on kernfs. The most valuable tests are those which
confirm that once a kernfs active reference is obtained with
kernfs_get_active() the pointers used there are still valid, and so
using sysfs ops *are* safe if we race against module removal. Likewise
it also confirms how module removal will *wait* for these ops to
complete if a kernfs node is already active.
This v9 series also addresses feedback mostly provided by Kees Cook and Greg.
I also made a few changes to the test_sysfs driver to account for changes in
the block layer. I also improved the kernfs failure injection tests with
documentation of how they work and to account for the real expected return
value of a write before the kernfs active reference is obtained. Upstream
commit 8e141f9eb803e ("block: drain file system I/O on del_gendisk") has
revealed that small minor induced delays on del_gendisk() can make a few
writes succeed if the delays used are small. So we clarify the logic of why
writes could either fail or succeed before the kernfs active reference is taken.
These changes also availble on this tree:
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?…
v9:
* rebased onto linux-next tag next-20211029
* add Reviewed-by tags for the SPDX change, and the drivers which
get the tag for it
* drop the generic sysfs deadlock fix for now as the scope of how
wide spread the issue is still needs to be assessed
* drop the zram patches as they are replaced by Ming Lei's fixes
* drop already merged patches
* try_module_get() docs: enhanced using feedback from Kees Cook. I
extended the documention to make it clear that if proper care is not
taken the use of this routine could crash the kernel.
* kernfs: move failure injection knobs under /sys/kernel/debug/fail_kernfs
as suggested by Kees Cook
* kernfs: rename failure injection file to fault_inject.c as suggested
by Kees Cook
* kernfs: split up documentation of failure injection knobs as
suggested by Kees Cook
* kernfs: move the wait into debug call, and use a simple one liner
may_wait() calls to make the changes much less intrusive and more
readable as suggested by Kees Cook
* kernfs: drop __func__ uses as suggested by Kees Cook
* test_sysfs: use sizeof() instead of open coded 16 as suggested by
Kees Cook
* test_sysfs: use sysfs_emit as suggested by Kees Cook
* test_sysfs: drop boiler place license as suggested by Greg KH
* test_sysfs: use depends instead of select as suggested by Kees Cook
* test_sysfs: drop #ifdefery as suggested by Kees Cook
* test_sysfs: clarified that the use of a lock on rmmod which causes
a deadlock is something drivers should avoid, and its why we leave
the test disabled.
* test_sysfs: now that device_add_disk() returns an error, use the
new error return code, otherwise this is going to prevent us from
eventually embracing __must_check() on that call on the block layer.
* test_syfs: testdev_submit_bio() needed to change data types as now
it returns void.
* test_sysfs: enhance kernfs failure injection tests with documenation
and correct the expected return value for writes
Luis Chamberlain (6):
LICENSES: Add the copyleft-next-0.3.1 license
testing: use the copyleft-next-0.3.1 SPDX tag
selftests: add tests_sysfs module
kernfs: add initial failure injection support
test_sysfs: add support to use kernfs failure injection
kernel/module: add documentation for try_module_get()
.../fault-injection/fault-injection.rst | 50 +
LICENSES/dual/copyleft-next-0.3.1 | 237 +++
MAINTAINERS | 9 +-
fs/kernfs/Makefile | 1 +
fs/kernfs/fault_inject.c | 93 ++
fs/kernfs/file.c | 9 +
fs/kernfs/kernfs-internal.h | 70 +
include/linux/kernfs.h | 5 +
include/linux/module.h | 37 +-
lib/Kconfig.debug | 23 +
lib/Makefile | 1 +
lib/test_kmod.c | 12 +-
lib/test_sysctl.c | 12 +-
lib/test_sysfs.c | 913 +++++++++++
tools/testing/selftests/kmod/kmod.sh | 13 +-
tools/testing/selftests/sysctl/sysctl.sh | 12 +-
tools/testing/selftests/sysfs/Makefile | 12 +
tools/testing/selftests/sysfs/config | 5 +
tools/testing/selftests/sysfs/settings | 1 +
tools/testing/selftests/sysfs/sysfs.sh | 1411 +++++++++++++++++
20 files changed, 2878 insertions(+), 48 deletions(-)
create mode 100644 LICENSES/dual/copyleft-next-0.3.1
create mode 100644 fs/kernfs/fault_inject.c
create mode 100644 lib/test_sysfs.c
create mode 100644 tools/testing/selftests/sysfs/Makefile
create mode 100644 tools/testing/selftests/sysfs/config
create mode 100644 tools/testing/selftests/sysfs/settings
create mode 100755 tools/testing/selftests/sysfs/sysfs.sh
--
2.30.2
Build of kselftests fail if kernel's top most Makefile is used for
running or building kselftests with separate output directory. The
absolute path is needed to reference other files during this kind of
build. Set KBUILD_ABS_SRCTREE to use absolute path during the build. It
fixes the following different types of errors:
make kselftest-all O=/linux_mainline/build
Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory
make kselftest-all O=build
Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory
Signed-off-by: Muhammad Usama Anjum <usama.anjum(a)collabora.com>
---
I've tested this patch on top of next-20220217. The latest next-20220222
have missing patches.
---
Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 86f633c2809ea..62b3eb8a102ab 100644
--- a/Makefile
+++ b/Makefile
@@ -1411,10 +1411,10 @@ tools/%: FORCE
PHONY += kselftest
kselftest:
- $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests
+ $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 run_tests
kselftest-%: FORCE
- $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
+ $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 $*
PHONY += kselftest-merge
kselftest-merge:
--
2.30.2
Commit ddbd60c779b4 ("kunit: use --build_dir=.kunit as default") changed
the default --build_dir, which had the side effect of making
`.kunitconfig` move to `.kunit/.kunitconfig`.
However, the first few lines of kunit/start.rst never got updated, oops.
Fix this by telling people to run kunit.py first, which will
automatically generate the .kunit directory and .kunitconfig file, and
then edit the file manually as desired.
Reported-by: Yifan Yuan <alpc_metic(a)live.com>
Signed-off-by: Daniel Latypov <dlatypov(a)google.com>
---
v1 -> v2: rebase onto 5.17 (had the kunit docs rewrite)
---
Documentation/dev-tools/kunit/start.rst | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst
index ad168d16968f..867a4bba6bf6 100644
--- a/Documentation/dev-tools/kunit/start.rst
+++ b/Documentation/dev-tools/kunit/start.rst
@@ -41,13 +41,18 @@ or ``VFAT_FS``. To run ``FAT_KUNIT_TEST``, the ``.kunitconfig`` has:
CONFIG_MSDOS_FS=y
CONFIG_FAT_KUNIT_TEST=y
-1. A good starting point for the ``.kunitconfig``, is the KUnit default
- config. Run the command:
+1. A good starting point for the ``.kunitconfig`` is the KUnit default config.
+ You can generate it by running:
.. code-block:: bash
cd $PATH_TO_LINUX_REPO
- cp tools/testing/kunit/configs/default.config .kunitconfig
+ tools/testing/kunit/kunit.py config
+ cat .kunit/.kunitconfig
+
+.. note ::
+ ``.kunitconfig`` lives in the ``--build_dir`` used by kunit.py, which is
+ ``.kunit`` by default.
.. note ::
You may want to remove CONFIG_KUNIT_ALL_TESTS from the ``.kunitconfig`` as
base-commit: c2741453478badf571ef020d160053e8d5e1ba94
--
2.35.0.rc2.247.g8bbb082509-goog
Before, our help output contained lines like
--kconfig_add KCONFIG_ADD
--qemu_config qemu_config
--jobs jobs
They're not very helpful.
The former kind come from the automatic 'metavar' we get from argparse,
the uppercase version of the flag name.
The latter are where we manually specified metavar as the flag name.
After:
--build_dir DIR
--make_options X=Y
--kunitconfig PATH
--kconfig_add CONFIG_X=Y
--arch ARCH
--cross_compile PREFIX
--qemu_config FILE
--jobs N
--timeout SECONDS
--raw_output [{all,kunit}]
--json [FILE]
This patch tries to make the code more clear by specifying the _type_ of
input we expect, e.g. --build_dir is a DIR, --qemu_config is a FILE.
I also switched it to uppercase since it looked more clearly like
placeholder text that way.
This patch also changes --raw_output to specify `choices` to make it
more clear what the options are, and this way argparse can validate it
for us, as shown by the added test case.
Signed-off-by: Daniel Latypov <dlatypov(a)google.com>
Reviewed-by: David Gow <davidgow(a)google.com>
---
tools/testing/kunit/kunit.py | 26 ++++++++++++--------------
tools/testing/kunit/kunit_tool_test.py | 5 +++++
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 9274c6355809..63d2c4bb9503 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -206,8 +206,6 @@ def parse_tests(request: KunitParseRequest, input_data: Iterable[str]) -> Tuple[
pass
elif request.raw_output == 'kunit':
output = kunit_parser.extract_tap_lines(output)
- else:
- print(f'Unknown --raw_output option "{request.raw_output}"', file=sys.stderr)
for line in output:
print(line.rstrip())
@@ -281,10 +279,10 @@ def add_common_opts(parser) -> None:
parser.add_argument('--build_dir',
help='As in the make command, it specifies the build '
'directory.',
- type=str, default='.kunit', metavar='build_dir')
+ type=str, default='.kunit', metavar='DIR')
parser.add_argument('--make_options',
help='X=Y make option, can be repeated.',
- action='append')
+ action='append', metavar='X=Y')
parser.add_argument('--alltests',
help='Run all KUnit tests through allyesconfig',
action='store_true')
@@ -292,11 +290,11 @@ def add_common_opts(parser) -> None:
help='Path to Kconfig fragment that enables KUnit tests.'
' If given a directory, (e.g. lib/kunit), "/.kunitconfig" '
'will get automatically appended.',
- metavar='kunitconfig')
+ metavar='PATH')
parser.add_argument('--kconfig_add',
help='Additional Kconfig options to append to the '
'.kunitconfig, e.g. CONFIG_KASAN=y. Can be repeated.',
- action='append')
+ action='append', metavar='CONFIG_X=Y')
parser.add_argument('--arch',
help=('Specifies the architecture to run tests under. '
@@ -304,7 +302,7 @@ def add_common_opts(parser) -> None:
'string passed to the ARCH make param, '
'e.g. i386, x86_64, arm, um, etc. Non-UML '
'architectures run on QEMU.'),
- type=str, default='um', metavar='arch')
+ type=str, default='um', metavar='ARCH')
parser.add_argument('--cross_compile',
help=('Sets make\'s CROSS_COMPILE variable; it should '
@@ -316,18 +314,18 @@ def add_common_opts(parser) -> None:
'if you have downloaded the microblaze toolchain '
'from the 0-day website to a directory in your '
'home directory called `toolchains`).'),
- metavar='cross_compile')
+ metavar='PREFIX')
parser.add_argument('--qemu_config',
help=('Takes a path to a path to a file containing '
'a QemuArchParams object.'),
- type=str, metavar='qemu_config')
+ type=str, metavar='FILE')
def add_build_opts(parser) -> None:
parser.add_argument('--jobs',
help='As in the make command, "Specifies the number of '
'jobs (commands) to run simultaneously."',
- type=int, default=get_default_jobs(), metavar='jobs')
+ type=int, default=get_default_jobs(), metavar='N')
def add_exec_opts(parser) -> None:
parser.add_argument('--timeout',
@@ -336,7 +334,7 @@ def add_exec_opts(parser) -> None:
'tests.',
type=int,
default=300,
- metavar='timeout')
+ metavar='SECONDS')
parser.add_argument('filter_glob',
help='Filter which KUnit test suites/tests run at '
'boot-time, e.g. list* or list*.*del_test',
@@ -346,7 +344,7 @@ def add_exec_opts(parser) -> None:
metavar='filter_glob')
parser.add_argument('--kernel_args',
help='Kernel command-line parameters. Maybe be repeated',
- action='append')
+ action='append', metavar='')
parser.add_argument('--run_isolated', help='If set, boot the kernel for each '
'individual suite/test. This is can be useful for debugging '
'a non-hermetic test, one that might pass/fail based on '
@@ -357,13 +355,13 @@ def add_exec_opts(parser) -> None:
def add_parse_opts(parser) -> None:
parser.add_argument('--raw_output', help='If set don\'t format output from kernel. '
'If set to --raw_output=kunit, filters to just KUnit output.',
- type=str, nargs='?', const='all', default=None)
+ type=str, nargs='?', const='all', default=None, choices=['all', 'kunit'])
parser.add_argument('--json',
nargs='?',
help='Stores test results in a JSON, and either '
'prints to stdout or saves to file if a '
'filename is specified',
- type=str, const='stdout', default=None)
+ type=str, const='stdout', default=None, metavar='FILE')
def main(argv, linux=None):
parser = argparse.ArgumentParser(
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 352369dffbd9..eb2011d12c78 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -595,6 +595,11 @@ class KUnitMainTest(unittest.TestCase):
self.assertNotEqual(call, mock.call(StrContains('Testing complete.')))
self.assertNotEqual(call, mock.call(StrContains(' 0 tests run')))
+ def test_run_raw_output_invalid(self):
+ self.linux_source_mock.run_kernel = mock.Mock(return_value=[])
+ with self.assertRaises(SystemExit) as e:
+ kunit.main(['run', '--raw_output=invalid'], self.linux_source_mock)
+
def test_run_raw_output_does_not_take_positional_args(self):
# --raw_output is a string flag, but we don't want it to consume
# any positional arguments, only ones after an '='
base-commit: 5debe5bfa02c4c8922bd2d0f82c9c3a70bec8944
--
2.35.1.574.g5d30c73bfb-goog
Filling log files with color codes makes diffs and other comparisons
difficult. Only emit vt100 codes when the stdout is a TTY.
Cc: Brendan Higgins <brendanhiggins(a)google.com>
Cc: linux-kselftest(a)vger.kernel.org
Cc: kunit-dev(a)googlegroups.com
Signed-off-by: Kees Cook <keescook(a)chromium.org>
---
tools/testing/kunit/kunit_parser.py | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
index 05ff334761dd..807ed2bd6832 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -11,6 +11,7 @@
from __future__ import annotations
import re
+import sys
import datetime
from enum import Enum, auto
@@ -503,14 +504,20 @@ RESET = '\033[0;0m'
def red(text: str) -> str:
"""Returns inputted string with red color code."""
+ if not sys.stdout.isatty():
+ return text
return '\033[1;31m' + text + RESET
def yellow(text: str) -> str:
"""Returns inputted string with yellow color code."""
+ if not sys.stdout.isatty():
+ return text
return '\033[1;33m' + text + RESET
def green(text: str) -> str:
"""Returns inputted string with green color code."""
+ if not sys.stdout.isatty():
+ return text
return '\033[1;32m' + text + RESET
ANSI_LEN = len(red(''))
--
2.30.2
Today, when we want to check if a pointer is NULL and not ERR we have
two options:
KUNIT_EXPECT_TRUE(test, ptr == NULL);
or
KUNIT_EXPECT_PTR_NE(test, ptr, (struct mystruct *)NULL);
Create a new set of macros that take care of NULL checks.
Reviewed-by: Brendan Higgins <brendanhiggins(a)google.com>
Reviewed-by: Daniel Latypov <dlatypov(a)google.com>
Signed-off-by: Ricardo Ribalda <ribalda(a)chromium.org>
---
include/kunit/test.h | 84 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 84 insertions(+)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 00b9ff7783ab..e6c18b609b47 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -1218,6 +1218,48 @@ do { \
fmt, \
##__VA_ARGS__)
+/**
+ * KUNIT_EXPECT_NULL() - Expects that @ptr is null.
+ * @test: The test context object.
+ * @ptr: an arbitrary pointer.
+ *
+ * Sets an expectation that the value that @ptr evaluates to is null. This is
+ * semantically equivalent to KUNIT_EXPECT_PTR_EQ(@test, ptr, NULL).
+ * See KUNIT_EXPECT_TRUE() for more information.
+ */
+#define KUNIT_EXPECT_NULL(test, ptr) \
+ KUNIT_EXPECT_NULL_MSG(test, \
+ ptr, \
+ NULL)
+
+#define KUNIT_EXPECT_NULL_MSG(test, ptr, fmt, ...) \
+ KUNIT_BINARY_PTR_ASSERTION(test, \
+ KUNIT_EXPECTATION, \
+ ptr, ==, NULL, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_NOT_NULL() - Expects that @ptr is not null.
+ * @test: The test context object.
+ * @ptr: an arbitrary pointer.
+ *
+ * Sets an expectation that the value that @ptr evaluates to is not null. This
+ * is semantically equivalent to KUNIT_EXPECT_PTR_NE(@test, ptr, NULL).
+ * See KUNIT_EXPECT_TRUE() for more information.
+ */
+#define KUNIT_EXPECT_NOT_NULL(test, ptr) \
+ KUNIT_EXPECT_NOT_NULL_MSG(test, \
+ ptr, \
+ NULL)
+
+#define KUNIT_EXPECT_NOT_NULL_MSG(test, ptr, fmt, ...) \
+ KUNIT_BINARY_PTR_ASSERTION(test, \
+ KUNIT_EXPECTATION, \
+ ptr, !=, NULL, \
+ fmt, \
+ ##__VA_ARGS__)
+
/**
* KUNIT_EXPECT_NOT_ERR_OR_NULL() - Expects that @ptr is not null and not err.
* @test: The test context object.
@@ -1485,6 +1527,48 @@ do { \
fmt, \
##__VA_ARGS__)
+/**
+ * KUNIT_ASSERT_NULL() - Asserts that pointers @ptr is null.
+ * @test: The test context object.
+ * @ptr: an arbitrary pointer.
+ *
+ * Sets an assertion that the values that @ptr evaluates to is null. This is
+ * the same as KUNIT_EXPECT_NULL(), except it causes an assertion
+ * failure (see KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_NULL(test, ptr) \
+ KUNIT_ASSERT_NULL_MSG(test, \
+ ptr, \
+ NULL)
+
+#define KUNIT_ASSERT_NULL_MSG(test, ptr, fmt, ...) \
+ KUNIT_BINARY_PTR_ASSERTION(test, \
+ KUNIT_ASSERTION, \
+ ptr, ==, NULL, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_ASSERT_NOT_NULL() - Asserts that pointers @ptr is not null.
+ * @test: The test context object.
+ * @ptr: an arbitrary pointer.
+ *
+ * Sets an assertion that the values that @ptr evaluates to is not null. This
+ * is the same as KUNIT_EXPECT_NOT_NULL(), except it causes an assertion
+ * failure (see KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_NOT_NULL(test, ptr) \
+ KUNIT_ASSERT_NOT_NULL_MSG(test, \
+ ptr, \
+ NULL)
+
+#define KUNIT_ASSERT_NOT_NULL_MSG(test, ptr, fmt, ...) \
+ KUNIT_BINARY_PTR_ASSERTION(test, \
+ KUNIT_ASSERTION, \
+ ptr, !=, NULL, \
+ fmt, \
+ ##__VA_ARGS__)
+
/**
* KUNIT_ASSERT_NOT_ERR_OR_NULL() - Assertion that @ptr is not null and not err.
* @test: The test context object.
--
2.35.1.265.g69c8d7142f-goog