Message-Id: <20181109154911.24464-1-rafael.tinoco@linaro.org>
Date:   Fri,  9 Nov 2018 13:49:11 -0200
From:   Rafael David Tinoco <rafael.tinoco@...aro.org>
To:     shuah@...nel.org
Cc:     linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        rafael.tinoco@...aro.org, peterz@...radead.org, tglx@...utronix.de,
        mingo@...nel.org, mathieu.desnoyers@...icios.com
Subject: [PATCH v5] selftests: membarrier: re-organize test

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@...aro.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();
 }
-- 
2.19.1

Powered by blists - more mailing lists