On 09/03/2018 01:04 PM, Rafael David Tinoco wrote:
This commit re-organizes membarrier test, solving issues when testing LTS kernels. Now, the code:
- always run the same amount of tests (even on older kernels).
- allows each test to succeed, fail or be skipped independently.
- allows testing features even when explicitly unsupported (force=1).
- able to consider different return codes for diff kernel versions.
- checks false positive/negative by checking ret code and errno.
- can be extended easily: to expand an array with commands.
Link: https://bugs.linaro.org/show_bug.cgi?id=3771 Signed-off-by: Rafael David Tinoco rafael.tinoco@linaro.org
.../selftests/membarrier/membarrier_test.c | 482 +++++++++--------- 1 file changed, 241 insertions(+), 241 deletions(-)
diff --git a/tools/testing/selftests/membarrier/membarrier_test.c b/tools/testing/selftests/membarrier/membarrier_test.c index 6793f8ecc8e7..151bc8a944a3 100644 --- a/tools/testing/selftests/membarrier/membarrier_test.c +++ b/tools/testing/selftests/membarrier/membarrier_test.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #define _GNU_SOURCE #include <linux/membarrier.h> +#include <sys/utsname.h> #include <syscall.h> #include <stdio.h> #include <errno.h> @@ -8,305 +9,304 @@ #include "../kselftest.h" -static int sys_membarrier(int cmd, int flags) +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) +#define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c))
+struct memb_tests {
- char testname[80];
- int command;
- int flags;
- int exp_ret;
- int exp_errno;
- int enabled;
- int force;
- int force_exp_errno;
- int above;
- int bellow;
+};
+struct memb_tests mbt[] = {
- {
.testname = "cmd_fail\0",.command = -1,.exp_ret = -1,.exp_errno = EINVAL,.enabled = 1,},- {
.testname = "cmd_flags_fail\0",.command = MEMBARRIER_CMD_QUERY,.flags = 1,.exp_ret = -1,.exp_errno = EINVAL,.enabled = 1,},- {
.testname = "cmd_global_success\0",.command = MEMBARRIER_CMD_GLOBAL,.flags = 0,.exp_ret = 0,},- /*
* PRIVATE EXPEDITED (forced)*/- {
.testname = "cmd_private_expedited_fail\0",.command = MEMBARRIER_CMD_PRIVATE_EXPEDITED,.flags = 0,.exp_ret = -1,.exp_errno = EPERM,.force = 1,.force_exp_errno = EINVAL,.bellow = KERNEL_VERSION(4, 10, 0),},- {
.testname = "cmd_private_expedited_fail\0",.command = MEMBARRIER_CMD_PRIVATE_EXPEDITED,.flags = 0,.exp_ret = -1,.exp_errno = EPERM,.force = 1,.force_exp_errno = EPERM,.above = KERNEL_VERSION(4, 10, 0),},- {
.testname = "cmd_register_private_expedited_success\0",.command = MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED,.flags = 0,.exp_ret = 0,.force = 1,.force_exp_errno = EINVAL,},- {
.testname = "cmd_private_expedited_success\0",.command = MEMBARRIER_CMD_PRIVATE_EXPEDITED,.flags = 0,.exp_ret = 0,.force = 1,.force_exp_errno = EINVAL,},/** PRIVATE EXPEDITED SYNC CORE*/- {
.testname = "cmd_private_expedited_sync_core_fail\0",.command = MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE,.flags = 0,.exp_ret = -1,.exp_errno = EPERM,},- {
.testname = "cmd_register_private_expedited_sync_core_success\0",.command = MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE,.flags = 0,.exp_ret = 0,},- {
.testname = "cmd_private_expedited_sync_core_success\0",.command = MEMBARRIER_CMD_PRIVATE_EXPEDITED,.flags = 0,.exp_ret = 0,},- /*
* GLOBAL EXPEDITED* global membarrier from a non-registered process is valid*/- {
.testname = "cmd_global_expedited_success\0",.command = MEMBARRIER_CMD_GLOBAL_EXPEDITED,.flags = 0,.exp_ret = 0,},- {
.testname = "cmd_register_global_expedited_success\0",.command = MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED,.flags = 0,.exp_ret = 0,},- {
.testname = "cmd_global_expedited_success\0",.command = MEMBARRIER_CMD_GLOBAL_EXPEDITED,.flags = 0,.exp_ret = 0,},+};
+static void +info_passed_ok(struct memb_tests test) {
- return syscall(__NR_membarrier, cmd, flags);
- ksft_test_result_pass("sys_membarrier(): %s succeeded.\n",
test.testname);}
Why do we need to add new routines for these conditions. Why can't handle these strings in array. For example you can define an array of strings for
passed unexpectedly etc. and the pass the string to appropriate ksft_* interface instead of adding of these routines. Also it is hard to review the code this way.
I do like the direction though. Also please run get_maintainer.pl and cc everybody it suggests.
thanks, -- Shuah
This commit re-organizes membarrier test, solving issues when testing LTS kernels. Now, the code:
- always run the same amount of tests (even on older kernels). - allows each test to succeed, fail or be skipped independently. - allows testing features even when explicitly unsupported (force=1). - checks false positive/negative by checking ret code and errno. - can be extended easily: to expand an array with commands.
Note: like this, the test is pretty close to the LTP membarrier basic tests, and both can be maintained together.
Link: https://bugs.linaro.org/show_bug.cgi?id=3771 Link: http://lists.linux.it/pipermail/ltp/2018-October/009578.html Signed-off-by: Rafael David Tinoco rafael.tinoco@linaro.org --- .../selftests/membarrier/membarrier_test.c | 566 ++++++++++-------- 1 file changed, 314 insertions(+), 252 deletions(-)
diff --git a/tools/testing/selftests/membarrier/membarrier_test.c b/tools/testing/selftests/membarrier/membarrier_test.c index 6793f8ecc8e7..7eb0e2395cbd 100644 --- a/tools/testing/selftests/membarrier/membarrier_test.c +++ b/tools/testing/selftests/membarrier/membarrier_test.c @@ -8,305 +8,367 @@
#include "../kselftest.h"
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) + +struct test_case { + char testname[80]; + int command; /* membarrier cmd */ + int needregister; /* membarrier cmd needs register cmd */ + int flags; /* flags for given membarrier cmd */ + long exp_ret; /* expected return code for given cmd */ + int exp_errno; /* expected errno for given cmd failure */ + int enabled; /* enabled, despite results from CMD_QUERY */ + int always; /* CMD_QUERY should always enable this test */ + int force; /* force if CMD_QUERY reports not enabled */ + int force_exp_errno; /* expected errno after forced cmd */ +}; + +struct test_case tc[] = { + { + /* + * case 00) invalid cmd + * - enabled by default + * - should always fail with EINVAL + */ + .testname = "cmd_fail", + .command = -1, + .exp_ret = -1, + .exp_errno = EINVAL, + .enabled = 1, + }, + { + /* + * case 01) invalid flags + * - enabled by default + * - should always fail with EINVAL + */ + .testname = "cmd_flags_fail", + .command = MEMBARRIER_CMD_QUERY, + .flags = 1, + .exp_ret = -1, + .exp_errno = EINVAL, + .enabled = 1, + }, + { + /* + * case 02) global barrier + * - should ALWAYS be enabled by CMD_QUERY + * - should always succeed + */ + .testname = "cmd_global_success", + .command = MEMBARRIER_CMD_GLOBAL, + .flags = 0, + .exp_ret = 0, + .always = 1, + }, + /* + * commit 22e4ebb975 (v4.14-rc1) added cases 03, 04 and 05 features: + */ + { + /* + * case 03) private expedited barrier with no registrations + * - should fail with errno=EPERM due to no registrations + * - or be skipped if unsupported by running kernel + */ + .testname = "cmd_private_expedited_fail", + .command = MEMBARRIER_CMD_PRIVATE_EXPEDITED, + .flags = 0, + .exp_ret = -1, + .exp_errno = EPERM, + }, + { + /* + * case 04) register private expedited + * - should succeed when supported by running kernel + * - or fail with errno=EINVAL if unsupported and forced + */ + .testname = "cmd_private_expedited_register_success", + .command = MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED, + .flags = 0, + .exp_ret = 0, + .force = 1, + .force_exp_errno = EINVAL, + }, + { + /* + * case 05) private expedited barrier with registration + * - should succeed due to existing registration + * - or fail with errno=EINVAL if unsupported and forced + * - NOTE: commit 70216e18e5 (v4.16-rc1) changed behavior: + * - (a) if unsupported, and forced, < 4.16 , errno is EINVAL + * - (b) if unsupported, and forced, >= 4.16, errno is EPERM + */ + .testname = "cmd_private_expedited_success", + .needregister = MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED, + .command = MEMBARRIER_CMD_PRIVATE_EXPEDITED, + .flags = 0, + .exp_ret = 0, + .force = 1, + .force_exp_errno = EPERM, + }, + /* + * commit 70216e18e5 (v4.16-rc1) added cases 06, 07 and 08 features: + */ + { + /* + * case 06) private expedited sync core barrier with no registrations + * - should fail with errno=EPERM due to no registrations + * - or be skipped if unsupported by running kernel + */ + .testname = "cmd_private_expedited_sync_core_fail", + .command = MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE, + .flags = 0, + .exp_ret = -1, + .exp_errno = EPERM, + }, + { + /* + * case 07) register private expedited sync core + * - should succeed when supported by running kernel + * - or fail with errno=EINVAL if unsupported and forced + */ + .testname = "cmd_private_expedited_sync_core_register_success", + .command = MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE, + .flags = 0, + .exp_ret = 0, + .force = 1, + .force_exp_errno = EINVAL, + }, + { + /* + * case 08) private expedited sync core barrier with registration + * - should succeed due to existing registration + * - or fail with errno=EINVAL if unsupported and forced + */ + .testname = "cmd_private_expedited_sync_core_success", + .needregister = MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE, + .command = MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE, + .flags = 0, + .exp_ret = 0, + .force = 1, + .force_exp_errno = EINVAL, + }, + /* + * commit c5f58bd58f4 (v4.16-rc1) added cases 09, 10 and 11 features: + */ + { + /* + * case 09) global expedited barrier with no registrations + * - should never fail due to no registrations + * - or be skipped if unsupported by running kernel + */ + .testname = "cmd_global_expedited_success", + .command = MEMBARRIER_CMD_GLOBAL_EXPEDITED, + .flags = 0, + .exp_ret = 0, + }, + { + /* + * case 10) register global expedited + * - should succeed when supported by running kernel + * - or fail with errno=EINVAL if unsupported and forced + */ + .testname = "cmd_global_expedited_register_success", + .command = MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED, + .flags = 0, + .exp_ret = 0, + .force = 1, + .force_exp_errno = EINVAL, + }, + { + /* + * case 11) global expedited barrier with registration + * - should also succeed with registrations + * - or fail with errno=EINVAL if unsupported and forced + */ + .testname = "cmd_global_expedited_success", + .needregister = MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED, + .command = MEMBARRIER_CMD_GLOBAL_EXPEDITED, + .flags = 0, + .exp_ret = 0, + .force = 1, + .force_exp_errno = EINVAL, + }, +}; + +#define passed_ok(_test) \ + do { \ + ksft_test_result_pass("membarrier(2): %s passed.\n", \ + _test.testname); \ + return; \ + } while (0) + +#define passed_unexpec(_test) \ + do { \ + ksft_exit_fail_msg("membarrier(2): %s passed unexpectedly. " \ + "ret = %ld with errno %d were expected. (force: %d)\n",\ + _test.testname, _test.exp_ret, _test.exp_errno, \ + _test.force); \ + return; \ + } while (0) + +#define failed_ok(_test) \ + do { \ + ksft_test_result_pass("membarrier(2): %s failed as " \ + "expected.\n", _test.testname); \ + return; \ + } while (0) + +#define failed_ok_unsupported(_test) \ + do { \ + ksft_test_result_pass("membarrier(2): %s failed as expected.\n"\ + "(unsupported)", _test.testname); \ + return; \ + } while (0) + +#define failed_not_ok(_test, _gotret, _goterr) \ + do { \ + ksft_exit_fail_msg("membarrier(2): %s failed. " \ + "ret = %ld when expected was %ld. " \ + "errno = %d when expected was %d. (force: %d)\n", \ + _test.testname, _gotret, _test.exp_ret, _goterr, \ + _test.exp_errno, _test.force); \ + return; \ + } while (0) + +#define failed_unexpec(_test, _gotret, _goterr) \ + do { \ + ksft_exit_fail_msg("membarrier(2): %s failed unexpectedly. " \ + "Got ret = %ld with errno %d. (force: %d)\n", \ + _test.testname, _gotret, _goterr, _test.force); \ + return; \ + } while (0) + +#define skipped(_test) \ + do { \ + ksft_test_result_pass("membarrier(2): %s skipped (unsupp)\n", \ + _test.testname); \ + return; \ + } while (0) + +#define skipped_fail(_test) \ + do { \ + ksft_exit_fail_msg("membarrier(2): %s reported as " \ + "unsupported\n", _test.testname); \ + return; \ + } while (0) + static int sys_membarrier(int cmd, int flags) { return syscall(__NR_membarrier, cmd, flags); }
-static int test_membarrier_cmd_fail(void) +static void test_membarrier_setup(void) { - int cmd = -1, flags = 0; - const char *test_name = "sys membarrier invalid command"; + size_t i; + int ret;
- if (sys_membarrier(cmd, flags) != -1) { - ksft_exit_fail_msg( - "%s test: command = %d, flags = %d. Should fail, but passed\n", - test_name, cmd, flags); - } - if (errno != EINVAL) { - ksft_exit_fail_msg( - "%s test: flags = %d. Should return (%d: "%s"), but returned (%d: "%s").\n", - test_name, flags, EINVAL, strerror(EINVAL), - errno, strerror(errno)); + ret = sys_membarrier(MEMBARRIER_CMD_QUERY, 0); + if (ret < 0) { + if (errno == ENOSYS) + ksft_test_result_skip("membarrier(2): not supported\n"); }
- ksft_test_result_pass( - "%s test: command = %d, flags = %d, errno = %d. Failed as expected\n", - test_name, cmd, flags, errno); - return 0; -} - -static int test_membarrier_flags_fail(void) -{ - int cmd = MEMBARRIER_CMD_QUERY, flags = 1; - const char *test_name = "sys membarrier MEMBARRIER_CMD_QUERY invalid flags"; - - if (sys_membarrier(cmd, flags) != -1) { - ksft_exit_fail_msg( - "%s test: flags = %d. Should fail, but passed\n", - test_name, flags); + for (i = 0; i < ARRAY_SIZE(tc); i++) { + if ((tc[i].command > 0) && (ret & tc[i].command)) + tc[i].enabled = 1; } - if (errno != EINVAL) { - ksft_exit_fail_msg( - "%s test: flags = %d. Should return (%d: "%s"), but returned (%d: "%s").\n", - test_name, flags, EINVAL, strerror(EINVAL), - errno, strerror(errno)); - } - - ksft_test_result_pass( - "%s test: flags = %d, errno = %d. Failed as expected\n", - test_name, flags, errno); - return 0; }
-static int test_membarrier_global_success(void) +static void test_membarrier_run(unsigned int i) { - int cmd = MEMBARRIER_CMD_GLOBAL, flags = 0; - const char *test_name = "sys membarrier MEMBARRIER_CMD_GLOBAL"; + int ret, err = 0;
- if (sys_membarrier(cmd, flags) != 0) { - ksft_exit_fail_msg( - "%s test: flags = %d, errno = %d\n", - test_name, flags, errno); - } + /* not enabled and not enforced: test is skipped */
- ksft_test_result_pass( - "%s test: flags = %d\n", test_name, flags); - return 0; -} + if (!tc[i].enabled && !tc[i].force) {
-static int test_membarrier_private_expedited_fail(void) -{ - int cmd = MEMBARRIER_CMD_PRIVATE_EXPEDITED, flags = 0; - const char *test_name = "sys membarrier MEMBARRIER_CMD_PRIVATE_EXPEDITED not registered failure"; + if (tc[i].always == 0) + skipped(tc[i]);
- if (sys_membarrier(cmd, flags) != -1) { - ksft_exit_fail_msg( - "%s test: flags = %d. Should fail, but passed\n", - test_name, flags); - } - if (errno != EPERM) { - ksft_exit_fail_msg( - "%s test: flags = %d. Should return (%d: "%s"), but returned (%d: "%s").\n", - test_name, flags, EPERM, strerror(EPERM), - errno, strerror(errno)); + skipped_fail(tc[i]); }
- ksft_test_result_pass( - "%s test: flags = %d, errno = %d\n", - test_name, flags, errno); - return 0; -} + /* iterations: registration needed for some cases */
-static int test_membarrier_register_private_expedited_success(void) -{ - int cmd = MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED, flags = 0; - const char *test_name = "sys membarrier MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED"; - - if (sys_membarrier(cmd, flags) != 0) { - ksft_exit_fail_msg( - "%s test: flags = %d, errno = %d\n", - test_name, flags, errno); + if (tc[i].needregister && tc[i].enabled) { + ret = sys_membarrier(tc[i].needregister, 0); + if (ret < 0) { + ksft_test_result_fail("membarrier(2): %s could not" + "register\n", tc[i].testname); + } }
- ksft_test_result_pass( - "%s test: flags = %d\n", - test_name, flags); - return 0; -} + ret = sys_membarrier(tc[i].command, tc[i].flags); + err = errno;
-static int test_membarrier_private_expedited_success(void) -{ - int cmd = MEMBARRIER_CMD_PRIVATE_EXPEDITED, flags = 0; - const char *test_name = "sys membarrier MEMBARRIER_CMD_PRIVATE_EXPEDITED"; + /* enabled and not enforced: regular expected results only */
- if (sys_membarrier(cmd, flags) != 0) { - ksft_exit_fail_msg( - "%s test: flags = %d, errno = %d\n", - test_name, flags, errno); - } + if (tc[i].enabled && !tc[i].force) {
- ksft_test_result_pass( - "%s test: flags = %d\n", - test_name, flags); - return 0; -} + if (ret >= 0 && tc[i].exp_ret == ret) + passed_ok(tc[i]);
-static int test_membarrier_private_expedited_sync_core_fail(void) -{ - int cmd = MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE, flags = 0; - const char *test_name = "sys membarrier MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE not registered failure"; - - if (sys_membarrier(cmd, flags) != -1) { - ksft_exit_fail_msg( - "%s test: flags = %d. Should fail, but passed\n", - test_name, flags); - } - if (errno != EPERM) { - ksft_exit_fail_msg( - "%s test: flags = %d. Should return (%d: "%s"), but returned (%d: "%s").\n", - test_name, flags, EPERM, strerror(EPERM), - errno, strerror(errno)); - } - - ksft_test_result_pass( - "%s test: flags = %d, errno = %d\n", - test_name, flags, errno); - return 0; -} - -static int test_membarrier_register_private_expedited_sync_core_success(void) -{ - int cmd = MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE, flags = 0; - const char *test_name = "sys membarrier MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE"; - - if (sys_membarrier(cmd, flags) != 0) { - ksft_exit_fail_msg( - "%s test: flags = %d, errno = %d\n", - test_name, flags, errno); + if (ret < 0) { + if (tc[i].exp_ret == ret) + failed_ok(tc[i]); + else + failed_not_ok(tc[i], ret, err); + } }
- ksft_test_result_pass( - "%s test: flags = %d\n", - test_name, flags); - return 0; -} + /* not enabled and enforced: failure and expected errors */
-static int test_membarrier_private_expedited_sync_core_success(void) -{ - int cmd = MEMBARRIER_CMD_PRIVATE_EXPEDITED, flags = 0; - const char *test_name = "sys membarrier MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE"; + if (!tc[i].enabled && tc[i].force) {
- if (sys_membarrier(cmd, flags) != 0) { - ksft_exit_fail_msg( - "%s test: flags = %d, errno = %d\n", - test_name, flags, errno); - } + if (ret >= 0) + passed_unexpec(tc[i]);
- ksft_test_result_pass( - "%s test: flags = %d\n", - test_name, flags); - return 0; -} - -static int test_membarrier_register_global_expedited_success(void) -{ - int cmd = MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED, flags = 0; - const char *test_name = "sys membarrier MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED"; - - if (sys_membarrier(cmd, flags) != 0) { - ksft_exit_fail_msg( - "%s test: flags = %d, errno = %d\n", - test_name, flags, errno); + if (ret < 0) { + if (tc[i].force_exp_errno == err) + failed_ok_unsupported(tc[i]); + else + failed_unexpec(tc[i], ret, err); + } }
- ksft_test_result_pass( - "%s test: flags = %d\n", - test_name, flags); - return 0; -} + /* enabled and enforced: tricky */
-static int test_membarrier_global_expedited_success(void) -{ - int cmd = MEMBARRIER_CMD_GLOBAL_EXPEDITED, flags = 0; - const char *test_name = "sys membarrier MEMBARRIER_CMD_GLOBAL_EXPEDITED"; + if (tc[i].enabled && tc[i].force) {
- if (sys_membarrier(cmd, flags) != 0) { - ksft_exit_fail_msg( - "%s test: flags = %d, errno = %d\n", - test_name, flags, errno); - } + if (ret >= 0) { + if (tc[i].exp_ret == ret) + passed_ok(tc[i]); + else + passed_unexpec(tc[i]); + }
- ksft_test_result_pass( - "%s test: flags = %d\n", - test_name, flags); - return 0; -} + if (ret < 0) {
-static int test_membarrier(void) -{ - int status; - - status = test_membarrier_cmd_fail(); - if (status) - return status; - status = test_membarrier_flags_fail(); - if (status) - return status; - status = test_membarrier_global_success(); - if (status) - return status; - status = test_membarrier_private_expedited_fail(); - if (status) - return status; - status = test_membarrier_register_private_expedited_success(); - if (status) - return status; - status = test_membarrier_private_expedited_success(); - if (status) - return status; - status = sys_membarrier(MEMBARRIER_CMD_QUERY, 0); - if (status < 0) { - ksft_test_result_fail("sys_membarrier() failed\n"); - return status; - } - if (status & MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE) { - status = test_membarrier_private_expedited_sync_core_fail(); - if (status) - return status; - status = test_membarrier_register_private_expedited_sync_core_success(); - if (status) - return status; - status = test_membarrier_private_expedited_sync_core_success(); - if (status) - return status; - } - /* - * It is valid to send a global membarrier from a non-registered - * process. - */ - status = test_membarrier_global_expedited_success(); - if (status) - return status; - status = test_membarrier_register_global_expedited_success(); - if (status) - return status; - status = test_membarrier_global_expedited_success(); - if (status) - return status; - return 0; -} + if (tc[i].exp_ret == ret) {
-static int test_membarrier_query(void) -{ - int flags = 0, ret; + if (tc[i].exp_errno == err) + failed_ok(tc[i]); + else + failed_unexpec(tc[i], ret, err); + }
- ret = sys_membarrier(MEMBARRIER_CMD_QUERY, flags); - if (ret < 0) { - if (errno == ENOSYS) { - /* - * It is valid to build a kernel with - * CONFIG_MEMBARRIER=n. However, this skips the tests. - */ - ksft_exit_skip( - "sys membarrier (CONFIG_MEMBARRIER) is disabled.\n"); + /* unknown on force failure if enabled and forced */ + failed_unexpec(tc[i], ret, err); } - ksft_exit_fail_msg("sys_membarrier() failed\n"); } - if (!(ret & MEMBARRIER_CMD_GLOBAL)) - ksft_exit_skip( - "sys_membarrier unsupported: CMD_GLOBAL not found.\n"); - - ksft_test_result_pass("sys_membarrier available\n"); - return 0; }
int main(int argc, char **argv) { + size_t i; + ksft_print_header();
- test_membarrier_query(); - test_membarrier(); + test_membarrier_setup(); + + for (i = 0; i < ARRAY_SIZE(tc); i++) + test_membarrier_run(i);
return ksft_exit_pass(); }
Why do we need to add new routines for these conditions. Why can't handle these strings in array. For example you can define an array of strings for passed unexpectedly etc. and the pass the string to appropriate ksft_* interface instead of adding of these routines. Also it is hard to review the code this way.
I was able to fit all the logic in the 80 char limit and, still, give a notion what was being called on each condition (instead of using an array number or equivalent). Considering this is not the core code, and the this has been already accepted and reviewed in LTP project, would you mind accepting it so both can be maintained together ? It is much better than the existing one, anyway...
Note: I have removed the part where we test for older return codes, since kselftest is not focusing in those (but LTP does).
I do like the direction though. Also please run get_maintainer.pl and cc everybody it suggests.
Done in v5.
Thanks a lot.
thanks, -- Shuah
This commit re-organizes membarrier test, solving issues when testing LTS kernels. Now, the code:
- always run the same amount of tests (even on older kernels).
- allows each test to succeed, fail or be skipped independently.
- allows testing features even when explicitly unsupported (force=1).
- checks false positive/negative by checking ret code and errno.
- can be extended easily: to expand an array with commands.
Note: like this, the test is pretty close to the LTP membarrier basic tests, and both can be maintained together.
Link: https://bugs.linaro.org/show_bug.cgi?id=3771 Link: http://lists.linux.it/pipermail/ltp/2018-October/009578.html Signed-off-by: Rafael David Tinoco rafael.tinoco@linaro.org
linux-kselftest-mirror@lists.linaro.org
-
Rafael David Tinoco -
Shuah Khan