March 2022 - Linux-kselftest-mirror

[PATCH] kunit: Make kunit_remove_resource() idempotent

by David Gow

The kunit_remove_resource() function is used to unlink a resource from the list of resources in the test, making it no longer show up in kunit_find_resource(). However, this could lead to a race condition if two threads called kunit_remove_resource() on the same resource at the same time: the resource would be removed from the list twice (causing a crash at the second list_del()), and the refcount for the resource would be decremented twice (instead of once, for the reference held by the resource list). Fix both problems, the first by using list_del_init(), and the second by checking if the resource has already been removed using list_empty(), and only decrementing its refcount if it has not. Also add a KUnit test for the kunit_remove_resource() function which tests this behaviour. Reported-by: Daniel Latypov <dlatypov(a)google.com> Signed-off-by: David Gow <davidgow(a)google.com> --- lib/kunit/kunit-test.c | 35 +++++++++++++++++++++++++++++++++++ lib/kunit/test.c | 8 ++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index 555601d17f79..9005034558aa 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -190,6 +190,40 @@ static void kunit_resource_test_destroy_resource(struct kunit *test) KUNIT_EXPECT_TRUE(test, list_empty(&ctx->test.resources)); } +static void kunit_resource_test_remove_resource(struct kunit *test) +{ + struct kunit_test_resource_context *ctx = test->priv; + struct kunit_resource *res = kunit_alloc_and_get_resource( + &ctx->test, + fake_resource_init, + fake_resource_free, + GFP_KERNEL, + ctx); + + /* The resource is in the list */ + KUNIT_EXPECT_FALSE(test, list_empty(&ctx->test.resources)); + + /* Remove the resource. The pointer is still valid, but it can't be + * found. + */ + kunit_remove_resource(test, res); + KUNIT_EXPECT_TRUE(test, list_empty(&ctx->test.resources)); + /* We haven't been freed yet. */ + KUNIT_EXPECT_TRUE(test, ctx->is_resource_initialized); + + /* Removing the resource multiple times is valid. */ + kunit_remove_resource(test, res); + KUNIT_EXPECT_TRUE(test, list_empty(&ctx->test.resources)); + /* Despite having been removed twice (from only one reference), the + * resource still has not been freed. + */ + KUNIT_EXPECT_TRUE(test, ctx->is_resource_initialized); + + /* Free the resource. */ + kunit_put_resource(res); + KUNIT_EXPECT_FALSE(test, ctx->is_resource_initialized); +} + static void kunit_resource_test_cleanup_resources(struct kunit *test) { int i; @@ -387,6 +421,7 @@ static struct kunit_case kunit_resource_test_cases[] = { KUNIT_CASE(kunit_resource_test_init_resources), KUNIT_CASE(kunit_resource_test_alloc_resource), KUNIT_CASE(kunit_resource_test_destroy_resource), + KUNIT_CASE(kunit_resource_test_remove_resource), KUNIT_CASE(kunit_resource_test_cleanup_resources), KUNIT_CASE(kunit_resource_test_proper_free_ordering), KUNIT_CASE(kunit_resource_test_static), diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 3bca3bf5c15b..8411cdfe40a8 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -680,11 +680,15 @@ EXPORT_SYMBOL_GPL(kunit_alloc_and_get_resource); void kunit_remove_resource(struct kunit *test, struct kunit_resource *res) { unsigned long flags; + bool was_linked; spin_lock_irqsave(&test->lock, flags); - list_del(&res->node); + was_linked = !list_empty(&res->node); + list_del_init(&res->node); spin_unlock_irqrestore(&test->lock, flags); - kunit_put_resource(res); + + if (was_linked) + kunit_put_resource(res); } EXPORT_SYMBOL_GPL(kunit_remove_resource); -- 2.35.1.894.gb6a874cedc-goog

2 years, 1 month

2
1
0 0

[PATCH v2] kunit: tool: more descriptive metavars/--help output

by Daniel Latypov

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

2 years, 1 month

2
1
0 0

[PATCH] kunit: tool: Do not colorize output when redirected

by Kees Cook

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

2 years, 1 month

4
4
0 0

[PATCH v2 1/3] kunit: tool: readability tweaks in KernelCI json generation logic

by Daniel Latypov

Use a more idiomatic check that a list is non-empty (`if mylist:`) and simplify the function body by dedenting and using a dict to map between the kunit TestStatus enum => KernelCI json status string. The dict hopefully makes it less likely to have bugs like commit 9a6bb30a8830 ("kunit: tool: fix --json output for skipped tests"). Signed-off-by: Daniel Latypov <dlatypov(a)google.com> Reviewed-by: David Gow <davidgow(a)google.com> --- tools/testing/kunit/kunit_json.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tools/testing/kunit/kunit_json.py b/tools/testing/kunit/kunit_json.py index 24d103049bca..14a480d3308a 100644 --- a/tools/testing/kunit/kunit_json.py +++ b/tools/testing/kunit/kunit_json.py @@ -16,24 +16,24 @@ from typing import Any, Dict JsonObj = Dict[str, Any] +_status_map: Dict[TestStatus, str] = { + TestStatus.SUCCESS: "PASS", + TestStatus.SKIPPED: "SKIP", + TestStatus.TEST_CRASHED: "ERROR", +} + def _get_group_json(test: Test, def_config: str, build_dir: str) -> JsonObj: sub_groups = [] # List[JsonObj] test_cases = [] # List[JsonObj] for subtest in test.subtests: - if len(subtest.subtests): + if subtest.subtests: sub_group = _get_group_json(subtest, def_config, build_dir) sub_groups.append(sub_group) - else: - test_case = {"name": subtest.name, "status": "FAIL"} - if subtest.status == TestStatus.SUCCESS: - test_case["status"] = "PASS" - elif subtest.status == TestStatus.SKIPPED: - test_case["status"] = "SKIP" - elif subtest.status == TestStatus.TEST_CRASHED: - test_case["status"] = "ERROR" - test_cases.append(test_case) + continue + status = _status_map.get(subtest.status, "FAIL") + test_cases.append({"name": subtest.name, "status": status}) test_group = { "name": test.name, -- 2.35.1.574.g5d30c73bfb-goog

2 years, 1 month

2
5
0 0

[PATCH v4 2/2] selftests/x86/amx: Update the ARCH_REQ_XCOMP_PERM test

by Chang S. Bae

Update the arch_prctl test to check the permission bitmap whether the requested feature is added as expected or not. Every non-dynamic feature that is enabled is permitted already for use. TILECFG is not dynamic feature. Ensure the bit is always on from ARCH_GET_XCOMP_PERM. Signed-off-by: Chang S. Bae <chang.seok.bae(a)intel.com> Cc: Dave Hansen <dave.hansen(a)linux.intel.com> Cc: linux-kselftest(a)vger.kernel.org Cc: x86(a)kernel.org Cc: linux-kernel(a)vger.kernel.org --- tools/testing/selftests/x86/amx.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/x86/amx.c b/tools/testing/selftests/x86/amx.c index 3615ef4a48bb..e1e2c8f3356f 100644 --- a/tools/testing/selftests/x86/amx.c +++ b/tools/testing/selftests/x86/amx.c @@ -368,9 +368,16 @@ static void req_xtiledata_perm(void) static void validate_req_xcomp_perm(enum expected_result exp) { - unsigned long bitmask; + unsigned long bitmask, expected_bitmask; long rc; + rc = syscall(SYS_arch_prctl, ARCH_GET_XCOMP_PERM, &bitmask); + if (rc) { + fatal_error("prctl(ARCH_GET_XCOMP_PERM) error: %ld", rc); + } else if (!(bitmask & XFEATURE_MASK_XTILECFG)) { + fatal_error("ARCH_GET_XCOMP_PERM returns XFEATURE_XTILECFG off."); + } + rc = syscall(SYS_arch_prctl, ARCH_REQ_XCOMP_PERM, XFEATURE_XTILEDATA); if (exp == FAIL_EXPECTED) { if (rc) { @@ -383,10 +390,15 @@ static void validate_req_xcomp_perm(enum expected_result exp) fatal_error("ARCH_REQ_XCOMP_PERM saw unexpected failure.\n"); } + expected_bitmask = bitmask | XFEATURE_MASK_XTILEDATA; + rc = syscall(SYS_arch_prctl, ARCH_GET_XCOMP_PERM, &bitmask); if (rc) { fatal_error("prctl(ARCH_GET_XCOMP_PERM) error: %ld", rc); - } else if (bitmask & XFEATURE_MASK_XTILE) { + } else if (bitmask != expected_bitmask) { + fatal_error("ARCH_REQ_XCOMP_PERM saw a wrong bitmask: %lx, expected: %lx.\n", + bitmask, expected_bitmask); + } else { printf("\tARCH_REQ_XCOMP_PERM is successful.\n"); } } -- 2.17.1

2 years, 1 month

2
2
0 0

[PATCH v6 1/6] kunit: Introduce _NULL and _NOT_NULL macros

by Ricardo Ribalda

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

2 years, 1 month

2
9
0 0

[PATCH 1/5] kunit: tool: drop mostly unused KunitResult.result field

by Daniel Latypov

This field is only used to pass along the parsed Test object from parse_tests(). Everywhere else the `result` field is ignored. Instead make parse_tests() explicitly return a KunitResult and Test so we can retire the `result` field. Signed-off-by: Daniel Latypov <dlatypov(a)google.com> --- tools/testing/kunit/kunit.py | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 7a706f96f68d..9274c6355809 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -17,7 +17,7 @@ assert sys.version_info >= (3, 7), "Python version is too old" from dataclasses import dataclass from enum import Enum, auto -from typing import Any, Iterable, Sequence, List, Optional +from typing import Iterable, List, Optional, Sequence, Tuple import kunit_json import kunit_kernel @@ -32,7 +32,6 @@ class KunitStatus(Enum): @dataclass class KunitResult: status: KunitStatus - result: Any elapsed_time: float @dataclass @@ -82,10 +81,8 @@ def config_tests(linux: kunit_kernel.LinuxSourceTree, config_end = time.time() if not success: return KunitResult(KunitStatus.CONFIG_FAILURE, - 'could not configure kernel', config_end - config_start) return KunitResult(KunitStatus.SUCCESS, - 'configured kernel successfully', config_end - config_start) def build_tests(linux: kunit_kernel.LinuxSourceTree, @@ -100,14 +97,11 @@ def build_tests(linux: kunit_kernel.LinuxSourceTree, build_end = time.time() if not success: return KunitResult(KunitStatus.BUILD_FAILURE, - 'could not build kernel', build_end - build_start) if not success: return KunitResult(KunitStatus.BUILD_FAILURE, - 'could not build kernel', build_end - build_start) return KunitResult(KunitStatus.SUCCESS, - 'built kernel successfully', build_end - build_start) def config_and_build_tests(linux: kunit_kernel.LinuxSourceTree, @@ -173,14 +167,14 @@ def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) - filter_glob=filter_glob, build_dir=request.build_dir) - result = parse_tests(request, run_result) + _, test_result = parse_tests(request, run_result) # run_kernel() doesn't block on the kernel exiting. # That only happens after we get the last line of output from `run_result`. # So exec_time here actually contains parsing + execution time, which is fine. test_end = time.time() exec_time += test_end - test_start - test_counts.add_subtest_counts(result.result.counts) + test_counts.add_subtest_counts(test_result.counts) if len(filter_globs) == 1 and test_counts.crashed > 0: bd = request.build_dir @@ -189,7 +183,7 @@ def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) - bd, bd, kunit_kernel.get_outfile_path(bd), bd, sys.argv[0])) kunit_status = _map_to_overall_status(test_counts.get_status()) - return KunitResult(status=kunit_status, result=result, elapsed_time=exec_time) + return KunitResult(status=kunit_status, elapsed_time=exec_time) def _map_to_overall_status(test_status: kunit_parser.TestStatus) -> KunitStatus: if test_status in (kunit_parser.TestStatus.SUCCESS, kunit_parser.TestStatus.SKIPPED): @@ -197,7 +191,7 @@ def _map_to_overall_status(test_status: kunit_parser.TestStatus) -> KunitStatus: else: return KunitStatus.TEST_FAILURE -def parse_tests(request: KunitParseRequest, input_data: Iterable[str]) -> KunitResult: +def parse_tests(request: KunitParseRequest, input_data: Iterable[str]) -> Tuple[KunitResult, kunit_parser.Test]: parse_start = time.time() test_result = kunit_parser.Test() @@ -231,11 +225,9 @@ def parse_tests(request: KunitParseRequest, input_data: Iterable[str]) -> KunitR print(json_obj) if test_result.status != kunit_parser.TestStatus.SUCCESS: - return KunitResult(KunitStatus.TEST_FAILURE, test_result, - parse_end - parse_start) + return KunitResult(KunitStatus.TEST_FAILURE, parse_end - parse_start), test_result - return KunitResult(KunitStatus.SUCCESS, test_result, - parse_end - parse_start) + return KunitResult(KunitStatus.SUCCESS, parse_end - parse_start), test_result def run_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitRequest) -> KunitResult: @@ -513,7 +505,7 @@ def main(argv, linux=None): request = KunitParseRequest(raw_output=cli_args.raw_output, build_dir='', json=cli_args.json) - result = parse_tests(request, kunit_output) + result, _ = parse_tests(request, kunit_output) if result.status != KunitStatus.SUCCESS: sys.exit(1) else: base-commit: f079ab01b5609fb0c9acc52c88168bf1eed82373 -- 2.34.1.703.g22d0c6ccf7-goog

2 years, 1 month

3
19
0 0

[GIT PULL] KUnit update for Linux 5.18-rc1

by Shuah Khan

Hi Linus, Please pull the following KUnit update for Linux 5.18-rc1. This KUnit update for Linux 5.18-rc1 consists of: - changes to decrease macro layering string, integer, EQ/NE asserts - remove unused macros - several cleanups and fixes - new list tests for list_del_init_careful(), list_is_head() and list_entry_is_head() diff is attached. thanks, -- Shuah ---------------------------------------------------------------- The following changes since commit e783362eb54cd99b2cac8b3a9aeac942e6f6ac07: Linux 5.17-rc1 (2022-01-23 10:12:53 +0200) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest tags/linux-kselftest-kunit-5.18-rc1 for you to fetch changes up to 5debe5bfa02c4c8922bd2d0f82c9c3a70bec8944: list: test: Add a test for list_entry_is_head() (2022-02-25 08:39:01 -0700) ---------------------------------------------------------------- linux-kselftest-kunit-5.18-rc1 This KUnit update for Linux 5.18-rc1 consists of: - changes to decrease macro layering string, integer, EQ/NE asserts - remove unused macros - several cleanups and fixes - new list tests for list_del_init_careful(), list_is_head() and list_entry_is_head() ---------------------------------------------------------------- Daniel Latypov (16): kunit: add example test case showing off all the expect macros kunit: move check if assertion passed into the macros kunit: drop unused kunit* field in kunit_assert kunit: factor out kunit_base_assert_format() call into kunit_fail() kunit: split out part of kunit_assert into a static const kunit: drop unused assert_type from kunit_assert and clean up macros kunit: make KUNIT_EXPECT_EQ() use KUNIT_EXPECT_EQ_MSG(), etc. kunit: drop unused intermediate macros for ptr inequality checks kunit: reduce layering in string assertion macros kunit: decrease macro layering for integer asserts kunit: decrease macro layering for EQ/NE asserts kunit: tool: drop mostly unused KunitResult.result field kunit: remove va_format from kunit_assert kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros kunit: factor out str constants from binary assertion structs kunit: cleanup assertion macro internal variables David Gow (3): list: test: Add test for list_del_init_careful() list: test: Add a test for list_is_head() list: test: Add a test for list_entry_is_head() include/kunit/assert.h | 220 ++++-------- include/kunit/test.h | 745 +++++++++++------------------------------ lib/kunit/assert.c | 80 ++--- lib/kunit/kunit-example-test.c | 42 +++ lib/kunit/test.c | 35 +- lib/list-test.c | 61 ++++ tools/testing/kunit/kunit.py | 24 +- 7 files changed, 420 insertions(+), 787 deletions(-) ----------------------------------------------------------------

2 years, 1 month

2
1
0 0

[GIT PULL] Kselftest update for Linux 5.18-rc1

by Shuah Khan

Hi Linus, Please pull the following Kselftest update for Linux 5.18-rc1 This Kselftest update for Linux 5.18-rc1 consists of several build and cleanup fixes. - removing obsolete config options - removing dependency on internal kernel macros - adding config options - several build fixes related to headers and install paths diff is attached. thanks, -- Shuah ---------------------------------------------------------------- The following changes since commit cfb92440ee71adcc2105b0890bb01ac3cddb8507: Linux 5.17-rc5 (2022-02-20 13:07:20 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest tags/linux-kselftest-next-5.18-rc1 for you to fetch changes up to f6d344cd5fa6a15e1ec2da350470b35a3f55f74c: selftests: Fix build when $(O) points to a relative path (2022-03-03 15:18:13 -0700) ---------------------------------------------------------------- linux-kselftest-next-5.18-rc1 This Kselftest update for Linux 5.18-rc1 consists of several build and cleanup fixes. - removing obsolete config options - removing dependency on internal kernel macros - adding config options - several build fixes related to headers and install paths ---------------------------------------------------------------- Cristian Marussi (1): selftests/kselftest/runner.sh: Pass optional command parameters in environment Geliang Tang (1): selftests: netfilter: fix a build error on openSUSE Mateusz Jończyk (1): selftests/rtc: continuously read RTC in a loop for 30s Muhammad Usama Anjum (19): selftests: futex: set DEFAULT_INSTALL_HDR_PATH selftests: set the BUILD variable to absolute path selftests: Add and export a kernel uapi headers path selftests: Correct the headers install path selftests: futex: Add the uapi headers include variable selftests: kvm: Add the uapi headers include variable selftests: landlock: Add the uapi headers include variable selftests: net: Add the uapi headers include variable selftests: mptcp: Add the uapi headers include variable selftests: vm: Add the uapi headers include variable selftests: vm: remove dependecy from internal kernel macros selftests: Use -isystem instead of -I to include headers selftests/exec: Rename file binfmt_script to binfmt_script.py selftests/lkdtm: Remove dead config option selftests/lkdtm: Add UBSAN config selftests: add kselftest_install to .gitignore selftests/exec: add generated files to .gitignore selftests: kvm: add generated file to the .gitignore selftests: Fix build when $(O) points to a relative path tools/testing/selftests/.gitignore | 1 + tools/testing/selftests/Makefile | 37 ++++++++---- tools/testing/selftests/exec/.gitignore | 2 + .../exec/{binfmt_script => binfmt_script.py} | 0 tools/testing/selftests/futex/functional/Makefile | 6 +- tools/testing/selftests/kselftest/runner.sh | 30 +++++++++- tools/testing/selftests/kvm/.gitignore | 1 + tools/testing/selftests/kvm/Makefile | 2 +- tools/testing/selftests/landlock/Makefile | 2 +- tools/testing/selftests/lkdtm/config | 2 +- tools/testing/selftests/net/Makefile | 2 +- tools/testing/selftests/net/mptcp/Makefile | 2 +- tools/testing/selftests/netfilter/Makefile | 1 + tools/testing/selftests/rtc/rtctest.c | 66 ++++++++++++++++++++++ tools/testing/selftests/rtc/settings | 2 +- tools/testing/selftests/vm/Makefile | 2 +- tools/testing/selftests/vm/userfaultfd.c | 3 + 18 files changed, 140 insertions(+), 23 deletions(-) rename tools/testing/selftests/exec/{binfmt_script => binfmt_script.py} ----------------------------------------------------------------

2 years, 1 month

2
1
0 0

[PATCH] kunit: Rework kunit_resource allocation policy

by David Gow

KUnit's test-managed resources can be created in two ways: - Using the kunit_add_resource() family of functions, which accept a struct kunit_resource pointer, typically allocated statically or on the stack during the test. - Using the kunit_alloc_resource() family of functions, which allocate a struct kunit_resource using kzalloc() behind the scenes. Both of these families of functions accept a 'free' function to be called when the resource is finally disposed of. At present, KUnit will kfree() the resource if this 'free' function is specified, and will not if it is NULL. However, this can lead kunit_alloc_resource() to leak memory (if no 'free' function is passed in), or kunit_add_resource() to incorrectly kfree() memory which was allocated by some other means (on the stack, as part of a larger allocation, etc), if a 'free' function is provided. Instead, always kfree() if the resource was allocated with kunit_alloc_resource(), and never kfree() if it was passed into kunit_add_resource() by the user. (If the user of kunit_add_resource() wishes the resource be kfree()ed, they can call kfree() on the resource from within the 'free' function. This is implemented by adding a 'should_free' member to struct kunit_resource and setting it appropriately. To facilitate this, the various resource add/alloc functions have been refactored somewhat, making them all call a __kunit_add_resource() helper after setting the 'should_free' member appropriately. In the process, all other functions have been made static inline functions. Signed-off-by: David Gow <davidgow(a)google.com> --- include/kunit/test.h | 135 +++++++++++++++++++++++++++++++++++-------- lib/kunit/test.c | 65 +++------------------ 2 files changed, 120 insertions(+), 80 deletions(-) diff --git a/include/kunit/test.h b/include/kunit/test.h index 00b9ff7783ab..5a3aacbadda2 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -36,11 +36,14 @@ typedef void (*kunit_resource_free_t)(struct kunit_resource *); * struct kunit_resource - represents a *test managed resource* * @data: for the user to store arbitrary data. * @name: optional name - * @free: a user supplied function to free the resource. Populated by - * kunit_resource_alloc(). + * @free: a user supplied function to free the resource. * * Represents a *test managed resource*, a resource which will automatically be - * cleaned up at the end of a test case. + * cleaned up at the end of a test case. This cleanup is performed by the 'free' + * function. The resource itself is allocated with kmalloc() and freed with + * kfree() if created with kunit_alloc_{,and_get_}resource(), otherwise it must + * be freed by the user, typically with the 'free' function, or automatically if + * it's allocated on the stack. * * Resources are reference counted so if a resource is retrieved via * kunit_alloc_and_get_resource() or kunit_find_resource(), we need @@ -97,6 +100,7 @@ struct kunit_resource { /* private: internal use only. */ struct kref refcount; struct list_head node; + bool should_free; }; struct kunit; @@ -385,16 +389,6 @@ static inline int kunit_run_all_tests(void) enum kunit_status kunit_suite_has_succeeded(struct kunit_suite *suite); -/* - * Like kunit_alloc_resource() below, but returns the struct kunit_resource - * object that contains the allocation. This is mostly for testing purposes. - */ -struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, - kunit_resource_init_t init, - kunit_resource_free_t free, - gfp_t internal_gfp, - void *context); - /** * kunit_get_resource() - Hold resource for use. Should not need to be used * by most users as we automatically get resources @@ -416,10 +410,14 @@ static inline void kunit_release_resource(struct kref *kref) refcount); /* If free function is defined, resource was dynamically allocated. */ - if (res->free) { + if (res->free) res->free(res); + + /* 'res' is valid here, as if should_free is set, res->free may not free + * 'res' itself, just res->data + */ + if (res->should_free) kfree(res); - } } /** @@ -440,7 +438,9 @@ static inline void kunit_put_resource(struct kunit_resource *res) } /** - * kunit_add_resource() - Add a *test managed resource*. + * __kunit_add_resource() - Internal helper to add a resource. + * + * res->should_free is not initialised. * @test: The test context object. * @init: a user-supplied function to initialize the result (if needed). If * none is supplied, the resource data value is simply set to @data. @@ -449,12 +449,34 @@ static inline void kunit_put_resource(struct kunit_resource *res) * @res: The resource. * @data: value to pass to init function or set in resource data field. */ -int kunit_add_resource(struct kunit *test, +int __kunit_add_resource(struct kunit *test, kunit_resource_init_t init, kunit_resource_free_t free, struct kunit_resource *res, void *data); +/** + * kunit_add_resource() - Add a *test managed resource*. + * @test: The test context object. + * @init: a user-supplied function to initialize the result (if needed). If + * none is supplied, the resource data value is simply set to @data. + * If an init function is supplied, @data is passed to it instead. + * @free: a user-supplied function to free the resource (if needed). + * @res: The resource. + * @data: value to pass to init function or set in resource data field. + */ +static inline int kunit_add_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + struct kunit_resource *res, + void *data) +{ + res->should_free = false; + return __kunit_add_resource(test, init, free, res, data); +} + +static inline struct kunit_resource * +kunit_find_named_resource(struct kunit *test, const char *name); /** * kunit_add_named_resource() - Add a named *test managed resource*. * @test: The test context object. @@ -464,18 +486,84 @@ int kunit_add_resource(struct kunit *test, * @name: name to be set for resource. * @data: value to pass to init function or set in resource data field. */ -int kunit_add_named_resource(struct kunit *test, +static inline int kunit_add_named_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + struct kunit_resource *res, + const char *name, + void *data) +{ + struct kunit_resource *existing; + + if (!name) + return -EINVAL; + + existing = kunit_find_named_resource(test, name); + if (existing) { + kunit_put_resource(existing); + return -EEXIST; + } + + res->name = name; + res->should_free = false; + + return __kunit_add_resource(test, init, free, res, data); +} + +/** + * kunit_alloc_and_get_resource() - Allocates and returns 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 (if needed). + * @internal_gfp: gfp to use for internal allocations, if unsure, use GFP_KERNEL + * @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. + * + * This is effectively identical to kunit_alloc_resource, but returns the + * struct kunit_resource pointer, not just the 'data' pointer. It therefore + * also increments the resource's refcount, so kunit_put_resource() should be + * called when you've finished with it. + * + * Note: KUnit needs to allocate memory for a kunit_resource object. You must + * specify an @internal_gfp that is compatible with the use context of your + * resource. + */ +static inline struct kunit_resource * +kunit_alloc_and_get_resource(struct kunit *test, kunit_resource_init_t init, kunit_resource_free_t free, - struct kunit_resource *res, - const char *name, - void *data); + gfp_t internal_gfp, + void *context) +{ + struct kunit_resource *res; + int ret; + + res = kzalloc(sizeof(*res), internal_gfp); + if (!res) + return NULL; + + res->should_free = true; + + ret = __kunit_add_resource(test, init, free, res, context); + if (!ret) { + /* + * bump refcount for get; kunit_resource_put() should be called + * when done. + */ + kunit_get_resource(res); + return res; + } + return NULL; +} /** * 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. + * @free: a user supplied function to free the resource (if needed). * @internal_gfp: gfp to use for internal allocations, if unsure, use GFP_KERNEL * @context: for the user to pass in arbitrary data to the init function. * @@ -499,7 +587,8 @@ static inline void *kunit_alloc_resource(struct kunit *test, if (!res) return NULL; - if (!kunit_add_resource(test, init, free, res, context)) + res->should_free = true; + if (!__kunit_add_resource(test, init, free, res, context)) return res->data; return NULL; diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 3bca3bf5c15b..b4f10329abb8 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -596,18 +596,19 @@ EXPORT_SYMBOL_GPL(__kunit_test_suites_exit); * Used for static resources and when a kunit_resource * has been created by * kunit_alloc_resource(). When an init function is supplied, @data is passed * into the init function; otherwise, we simply set the resource data field to - * the data value passed in. + * the data value passed in. Doesn't initialize res->should_free. */ -int kunit_add_resource(struct kunit *test, - kunit_resource_init_t init, - kunit_resource_free_t free, - struct kunit_resource *res, - void *data) +int __kunit_add_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + struct kunit_resource *res, + void *data) { int ret = 0; unsigned long flags; res->free = free; + res->should_free = false; kref_init(&res->refcount); if (init) { @@ -625,57 +626,7 @@ int kunit_add_resource(struct kunit *test, return ret; } -EXPORT_SYMBOL_GPL(kunit_add_resource); - -int kunit_add_named_resource(struct kunit *test, - kunit_resource_init_t init, - kunit_resource_free_t free, - struct kunit_resource *res, - const char *name, - void *data) -{ - struct kunit_resource *existing; - - if (!name) - return -EINVAL; - - existing = kunit_find_named_resource(test, name); - if (existing) { - kunit_put_resource(existing); - return -EEXIST; - } - - res->name = name; - - return kunit_add_resource(test, init, free, res, data); -} -EXPORT_SYMBOL_GPL(kunit_add_named_resource); - -struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, - kunit_resource_init_t init, - kunit_resource_free_t free, - gfp_t internal_gfp, - void *data) -{ - struct kunit_resource *res; - int ret; - - res = kzalloc(sizeof(*res), internal_gfp); - if (!res) - return NULL; - - ret = kunit_add_resource(test, init, free, res, data); - if (!ret) { - /* - * bump refcount for get; kunit_resource_put() should be called - * when done. - */ - kunit_get_resource(res); - return res; - } - return NULL; -} -EXPORT_SYMBOL_GPL(kunit_alloc_and_get_resource); +EXPORT_SYMBOL_GPL(__kunit_add_resource); void kunit_remove_resource(struct kunit *test, struct kunit_resource *res) { -- 2.35.1.894.gb6a874cedc-goog

2 years, 1 month

3
5
0 0

2024

2023

2022

2021

2020

2019

2018

2017

Linux-kselftest-mirror March 2022