[LTP] [PATCH v2 1/2] numa_helper: Fix compiler warnings

[LTP] [PATCH v2 1/2] numa_helper: Fix compiler warnings

[Petr Vorel]

about comparison between signed and unsigned integer

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/lib/numa_helper.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/lib/numa_helper.c b/testcases/kernel/lib/numa_helper.c
index a852f152f..afc09b8db 100644
--- a/testcases/kernel/lib/numa_helper.c
+++ b/testcases/kernel/lib/numa_helper.c
@@ -60,7 +60,7 @@ unsigned long get_max_node(void)
 static void get_nodemask_allnodes(nodemask_t * nodemask, unsigned long max_node)
 {
 	unsigned long nodemask_size = max_node / 8;
-	int i;
+	unsigned int i;
 	char fn[64];
 	struct stat st;
 
@@ -95,7 +95,7 @@ static int filter_nodemask_mem(nodemask_t * nodemask, unsigned long max_node)
 		return -2;
 	}
 #else
-	int i;
+	unsigned int i;
 	/*
 	 * old libnuma/kernel don't have MPOL_F_MEMS_ALLOWED, so let's assume
 	 * that we can use any node with memory > 0
@@ -112,7 +112,7 @@ static int filter_nodemask_mem(nodemask_t * nodemask, unsigned long max_node)
 
 static int cpumask_has_cpus(char *cpumask, size_t len)
 {
-	int j;
+	unsigned int j;
 	for (j = 0; j < len; j++)
 		if (cpumask[j] == '\0')
 			return 0;
@@ -129,7 +129,8 @@ static void filter_nodemask_cpu(nodemask_t * nodemask, unsigned long max_node)
 	char fn[64];
 	FILE *f;
 	size_t len;
-	int i, ret;
+	unsigned int i;
+	int ret;
 
 	for (i = 0; i < max_node; i++) {
 		if (!nodemask_isset(nodemask, i))
@@ -164,7 +165,7 @@ int get_allowed_nodes_arr(int flag, int *num_nodes, int **nodes)
 {
 	int ret = 0;
 #if HAVE_NUMA_H
-	int i;
+	unsigned int i;
 	nodemask_t *nodemask = NULL;
 #endif
 	*num_nodes = 0;
-- 
2.14.0

[LTP] [PATCH v2 2/2] syscalls/mbind: cleanup and rewrite mbind01 into new API

[Petr Vorel]

+ removed support for NUMA API < 2 (test is compilable, but TCONF
on older API)

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/mbind/mbind01.c | 450 ++++++++++++------------------
 1 file changed, 186 insertions(+), 264 deletions(-)

diff --git a/testcases/kernel/syscalls/mbind/mbind01.c b/testcases/kernel/syscalls/mbind/mbind01.c
index d5c93f203..8db1deeb5 100644
--- a/testcases/kernel/syscalls/mbind/mbind01.c
+++ b/testcases/kernel/syscalls/mbind/mbind01.c
@@ -1,317 +1,239 @@
-/******************************************************************************/
-/* Copyright (c) Crackerjack Project., 2007-2008			      */
-/* Author(s):	Takahiro Yasui <takahiro.yasui.mp@hitachi.com>,		      */
-/*		Yumiko Sugita <yumiko.sugita.yf@hitachi.com>,		      */
-/*		Satoshi Fujiwara <sa-fuji@sdl.hitachi.co.jp>		      */
-/*									      */
-/* This program is free software;  you can redistribute it and/or modify      */
-/* it under the terms of the GNU General Public License as published by	      */
-/* the Free Software Foundation; either version 2 of the License, or	      */
-/* (at your option) any later version.					      */
-/*									      */
-/* This program is distributed in the hope that it will be useful,	      */
-/* but WITHOUT ANY WARRANTY;  without even the implied warranty of	      */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See		      */
-/* the GNU General Public License for more details.			      */
-/*									      */
-/* You should have received a copy of the GNU General Public License	      */
-/* along with this program;  if not, write to the Free Software		      */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    */
-/*									      */
-/******************************************************************************/
-/******************************************************************************/
-/*									      */
-/* File:	mbind01.c						      */
-/*									      */
-/* Description: This tests the mbind() syscall				      */
-/*									      */
-/* Usage:	<for command-line>					      */
-/* mbind01 [-c n] [-e][-i n] [-I x] [-p x] [-t]				      */
-/*	where,	-c n : Run n copies concurrently.			      */
-/*		-e   : Turn on errno logging.				      */
-/*		-i n : Execute test n times.				      */
-/*		-I x : Execute test for x seconds.			      */
-/*		-P x : Pause for x seconds between iterations.		      */
-/*		-t   : Turn on syscall timing.				      */
-/*									      */
-/* Total Tests: 1							      */
-/*									      */
-/* Test Name:	mbind01							      */
-/* History:	Porting from Crackerjack to LTP is done by		      */
-/*		Manas Kumar Nayak maknayak@in.ibm.com>			      */
-/******************************************************************************/
+/*
+ * Copyright (c) Crackerjack Project., 2007-2008, Hitachi, Ltd
+ * Copyright (c) 2017 Petr Vorel <pvorel@suse.cz>
+ *
+ * Authors:
+ * Takahiro Yasui <takahiro.yasui.mp@hitachi.com>,
+ * Yumiko Sugita <yumiko.sugita.yf@hitachi.com>,
+ * Satoshi Fujiwara <sa-fuji@sdl.hitachi.co.jp>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
 
-#include "config.h"
-#include <sys/types.h>
-#include <sys/mman.h>
-#include <sys/syscall.h>
 #include <errno.h>
-#include <getopt.h>
-#include <libgen.h>
-#if HAVE_NUMA_H
-#include <numa.h>
-#endif
-#if HAVE_NUMAIF_H
-#include <numaif.h>
-#endif
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
 
-#include "test.h"
-#include "lapi/syscalls.h"
-#include "include_j_h.h"
 #include "numa_helper.h"
 
-char *TCID = "mbind01";
-int TST_TOTAL = 2;
-
 #if HAVE_NUMA_H && HAVE_LINUX_MEMPOLICY_H && HAVE_NUMAIF_H && \
 	HAVE_MPOL_CONSTANTS
+#if defined(LIBNUMA_API_VERSION) && LIBNUMA_API_VERSION == 2
+#include "tst_test.h"
 
-#define MEM_LENGTH	      (4 * 1024 * 1024)
+#define MEM_LENGTH (4 * 1024 * 1024)
 
-static int testno;
+#define UNKNOWN_POLICY -1
 
-enum test_type {
-	NORMAL,
-	INVALID_POINTER,
-};
+#define POLICY_DESC(x) .policy = x, .desc = #x
+#define POLICY_DESC_TEXT(x, y) .policy = x, .desc = #x" ("y")"
 
-enum from_node {
-	NONE,
-	SELF,
-};
+struct bitmask *nodemask, *getnodemask;
+
+void test_default(unsigned int i, char *p);
+void test_none(unsigned int i, char *p);
+void test_invalid_nodemask(unsigned int i, char *p);
 
 struct test_case {
-	int ttype;
 	int policy;
-	int from_node;
+	const char *desc;
 	unsigned flags;
+	unsigned no_check_nodemask;
 	int ret;
 	int err;
+	void (*test)(unsigned int, char *);
 };
 
-/* Test cases
- *
- *   test status of errors on man page
- *
- *   EFAULT	v (detect unmapped hole or invalid pointer)
- *   EINVAL	v (invalid arguments)
- *   ENOMEM	can't check because it's difficult to create no-memory
- *   EIO	can't check because we don't have N-node NUMA system
- *		(only we can do is simulate 1-node NUMA)
- */
 static struct test_case tcase[] = {
-	{			/* case00 */
-	 .policy = MPOL_DEFAULT,
-	 .from_node = NONE,
-	 .ret = 0,
-	 .err = 0,
-	 },
-	{			/* case01 */
-	 .policy = MPOL_DEFAULT,
-	 .from_node = SELF,	/* target exists */
-	 .ret = -1,
-	 .err = EINVAL,
-	 },
-	{			/* case02 */
-	 .policy = MPOL_BIND,
-	 .from_node = NONE,	/* no target */
-	 .ret = -1,
-	 .err = EINVAL,
-	 },
-	{			/* case03 */
-	 .policy = MPOL_BIND,
-	 .from_node = SELF,
-	 .ret = 0,
-	 .err = 0,
-	 },
-	{			/* case04 */
-	 .policy = MPOL_INTERLEAVE,
-	 .from_node = NONE,	/* no target */
-	 .ret = -1,
-	 .err = EINVAL,
-	 },
-	{			/* case05 */
-	 .policy = MPOL_INTERLEAVE,
-	 .from_node = SELF,
-	 .ret = 0,
-	 .err = 0,
-	 },
-	{			/* case06 */
-	 .policy = MPOL_PREFERRED,
-	 .from_node = NONE,
-	 .ret = 0,
-	 .err = 0,
-	 },
-	{			/* case07 */
-	 .policy = MPOL_PREFERRED,
-	 .from_node = SELF,
-	 .ret = 0,
-	 .err = 0,
-	 },
-	{			/* case08 */
-	 .policy = -1,		/* unknown policy */
-	 .from_node = NONE,
-	 .ret = -1,
-	 .err = EINVAL,
-	 },
-	{			/* case09 */
-	 .policy = MPOL_DEFAULT,
-	 .from_node = NONE,
-	 .flags = -1,		/* invalid flags */
-	 .ret = -1,
-	 .err = EINVAL,
-	 },
-	{			/* case10 */
-	 .ttype = INVALID_POINTER,
-	 .policy = MPOL_PREFERRED,
-	 .from_node = SELF,
-	 .ret = -1,
-	 .err = EFAULT,
-	 },
+	{
+		POLICY_DESC(MPOL_DEFAULT),
+		.ret = 0,
+		.err = 0,
+		.test = test_none,
+	},
+	{
+		POLICY_DESC_TEXT(MPOL_DEFAULT, "target exists"),
+		.ret = -1,
+		.err = EINVAL,
+		.test = test_default,
+	},
+	{
+		POLICY_DESC_TEXT(MPOL_BIND, "no target"),
+		.ret = -1,
+		.err = EINVAL,
+		.test = test_none,
+	},
+	{
+		POLICY_DESC(MPOL_BIND),
+		.ret = 0,
+		.err = 0,
+		.test = test_default,
+	},
+	{
+		POLICY_DESC_TEXT(MPOL_INTERLEAVE, "no target"),
+		.ret = -1,
+		.err = EINVAL,
+		.test = test_none,
+	},
+	{
+		POLICY_DESC(MPOL_INTERLEAVE),
+		.ret = 0,
+		.err = 0,
+		.test = test_default,
+	},
+	{
+		POLICY_DESC(MPOL_PREFERRED),
+		.ret = 0,
+		.no_check_nodemask = 1,
+		.err = 0,
+		.test = test_none,
+	},
+	{
+		POLICY_DESC(MPOL_PREFERRED),
+		.ret = 0,
+		.err = 0,
+		.test = test_default,
+	},
+	{
+		POLICY_DESC(UNKNOWN_POLICY),
+		.ret = -1,
+		.err = EINVAL,
+		.test = test_none,
+	},
+	{
+		POLICY_DESC_TEXT(MPOL_DEFAULT, "invalid flags"),
+		.flags = -1,
+		.ret = -1,
+		.err = EINVAL,
+		.test = test_none,
+	},
+	{
+		POLICY_DESC_TEXT(MPOL_PREFERRED, "invalid nodemask"),
+		.ret = -1,
+		.err = EFAULT,
+		.test = test_invalid_nodemask,
+	},
 };
 
-static int do_test(struct test_case *tc);
-static void setup(void);
-static void cleanup(void);
+void test_default(unsigned int i, char *p) {
+	struct test_case *tc = &tcase[i];
+	TEST(mbind(p, MEM_LENGTH, tc->policy, nodemask->maskp,
+		   nodemask->size, tc->flags));
+}
+
+void test_none(unsigned int i, char *p) {
+	struct test_case *tc = &tcase[i];
+	TEST(mbind(p, MEM_LENGTH, tc->policy, NULL, 0, tc->flags));
+}
+
+void test_invalid_nodemask(unsigned int i, char *p) {
+	struct test_case *tc = &tcase[i];
+	/* use invalid nodemask (64 MiB after heap) */
+	TEST(mbind(p, MEM_LENGTH, tc->policy, sbrk(0) + 64*1024*1024,
+		   NUMA_NUM_NODES, tc->flags));
+}
 
-int main(int argc, char **argv)
+static void setup(void)
 {
-	int lc, i, ret;
+	if (!is_numa(NULL, NH_MEMS, 1))
+		tst_brk(TCONF, "requires NUMA with at least 1 node");
+}
 
-	tst_parse_opts(argc, argv, NULL, NULL);
+static void setup_node(void)
+{
+	int test_node = -1;
 
-	setup();
-	testno = (int)(sizeof(tcase) / sizeof(tcase[0]));
+	if (get_allowed_nodes(NH_MEMS, 1, &test_node) < 0)
+		tst_brk(TBROK | TERRNO, "get_allowed_nodes failed");
 
-	for (lc = 0; TEST_LOOPING(lc); ++lc) {
-		tst_count = 0;
-		for (i = 0; i < testno; i++) {
-			tst_resm(TINFO, "(case%02d) START", i);
-			ret = do_test(&tcase[i]);
-			tst_resm((ret == 0 ? TPASS : TFAIL | TERRNO),
-				 "(case%02d) END", i);
-		}
-	}
-	cleanup();
-	tst_exit();
+	nodemask = numa_allocate_nodemask();
+	getnodemask = numa_allocate_nodemask();
+	numa_bitmask_setbit(nodemask, test_node);
 }
 
-static int do_test(struct test_case *tc)
+static void do_test(unsigned int i)
 {
-	int ret, err, result, cmp_ok = 1;
+	struct test_case *tc = &tcase[i];
 	int policy;
 	char *p = NULL;
-#if !defined(LIBNUMA_API_VERSION) || LIBNUMA_API_VERSION < 2
-	nodemask_t *nodemask, *getnodemask;
-#else
-	struct bitmask *nodemask = numa_allocate_nodemask();
-	struct bitmask *getnodemask = numa_allocate_nodemask();
-#endif
-	unsigned long maxnode = NUMA_NUM_NODES;
-	unsigned long len = MEM_LENGTH;
-	unsigned long *invalid_nodemask;
-	int test_node = -1;
 
-	ret = get_allowed_nodes(NH_MEMS, 1, &test_node);
-	if (ret < 0)
-		tst_brkm(TBROK | TERRNO, cleanup, "get_allowed_nodes: %d", ret);
+	tst_res(TINFO, "case %s", tc->desc);
 
-#if !defined(LIBNUMA_API_VERSION) || LIBNUMA_API_VERSION < 2
-	nodemask = malloc(sizeof(nodemask_t));
-	nodemask_zero(nodemask);
-	nodemask_set(nodemask, test_node);
-	getnodemask = malloc(sizeof(nodemask_t));
-	nodemask_zero(getnodemask);
-#else
-	numa_bitmask_setbit(nodemask, test_node);
-#endif
-	p = mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS,
-		 0, 0);
+	setup_node();
+
+	p = mmap(NULL, MEM_LENGTH, PROT_READ | PROT_WRITE, MAP_PRIVATE |
+			 MAP_ANONYMOUS, 0, 0);
 	if (p == MAP_FAILED)
-		tst_brkm(TBROK | TERRNO, cleanup, "mmap");
+		tst_brk(TBROK | TERRNO, "mmap");
 
-	if (tc->ttype == INVALID_POINTER)
-		invalid_nodemask = (unsigned long *)0xc0000000;
+	tc->test(i, p);
 
-	errno = 0;
-	if (tc->from_node == NONE)
-		TEST(ret = ltp_syscall(__NR_mbind, p, len, tc->policy,
-				   NULL, 0, tc->flags));
-	else if (tc->ttype == INVALID_POINTER)
-		TEST(ret = ltp_syscall(__NR_mbind, p, len, tc->policy,
-				   invalid_nodemask, maxnode, tc->flags));
-	else
-#if !defined(LIBNUMA_API_VERSION) || LIBNUMA_API_VERSION < 2
-		TEST(ret = ltp_syscall(__NR_mbind, p, len, tc->policy,
-				   nodemask, maxnode, tc->flags));
-#else
-		TEST(ret = ltp_syscall(__NR_mbind, p, len, tc->policy,
-				   nodemask->maskp, nodemask->size, tc->flags));
-#endif
+	if (TEST_RETURN >= 0) {
+		/* Check policy of the allocated memory */
+		TEST(get_mempolicy(&policy, getnodemask->maskp,
+				   getnodemask->size, p, MPOL_F_ADDR));
+		if (TEST_RETURN < 0) {
+			tst_res(TFAIL | TERRNO, "get_mempolicy failed");
+			return;
+		}
 
-	err = TEST_ERRNO;
-	if (ret < 0)
-		goto TEST_END;
+		/* get_mempolicy doesn't return nodemask for policy MPOL_DEFAULT */
+		if (tc->policy == MPOL_DEFAULT)
+			numa_bitmask_clearall(nodemask);
 
-	/* Check policy of the allocated memory */
-#if !defined(LIBNUMA_API_VERSION) || LIBNUMA_API_VERSION < 2
-	TEST(ltp_syscall(__NR_get_mempolicy, &policy, getnodemask,
-		     maxnode, p, MPOL_F_ADDR));
-#else
-	TEST(ltp_syscall(__NR_get_mempolicy, &policy, getnodemask->maskp,
-		     getnodemask->size, p, MPOL_F_ADDR));
-#endif
-	if (TEST_RETURN < 0) {
-		tst_resm(TFAIL | TERRNO, "get_mempolicy failed");
-		return -1;
-	}
+		if (tc->policy != policy) {
+			tst_res(TFAIL, "Wrong policy: %d, expected: %d",
+				tc->policy, policy);
+			return;
+		}
 
-	/* If policy == MPOL_DEFAULT, get_mempolicy doesn't return nodemask */
-	if (tc->policy == MPOL_DEFAULT)
-#if !defined(LIBNUMA_API_VERSION) || LIBNUMA_API_VERSION < 2
-		nodemask_zero(nodemask);
-#else
-		numa_bitmask_clearall(nodemask);
-#endif
+		if (!tc->no_check_nodemask && !numa_bitmask_equal(nodemask,
+								  getnodemask)) {
+			tst_res(TFAIL, "masks are not equal");
+			return;
+		}
+	}
 
-	if ((tc->policy == MPOL_PREFERRED) && (tc->from_node == NONE))
-		cmp_ok = (tc->policy == policy);
+	if (TEST_ERRNO != tc->err)
+		tst_res(TFAIL | TERRNO, "test mbind() failed: %d, expected: %d",
+			TEST_ERRNO, tc->err);
+	else if (TEST_RETURN != tc->ret)
+		tst_res(TFAIL | TERRNO,
+			"test mbind() wrong return code: %ld, expected: %d",
+			TEST_RETURN, tc->ret);
 	else
-		cmp_ok = ((tc->policy == policy) &&
-#if !defined(LIBNUMA_API_VERSION) || LIBNUMA_API_VERSION < 2
-			  nodemask_equal(nodemask, getnodemask));
-#else
-			  numa_bitmask_equal(nodemask, getnodemask));
-#endif
-TEST_END:
-	result = ((err != tc->err) || (!cmp_ok));
-	PRINT_RESULT_CMP(0, tc->ret, tc->err, ret, err, cmp_ok);
-	return result;
+		tst_res(TPASS, "Test passed");
 }
 
-static void setup(void)
-{
-	/* check syscall availability */
-	ltp_syscall(__NR_mbind, NULL, 0, 0, NULL, 0, 0);
-
-	if (!is_numa(NULL, NH_MEMS, 1))
-		tst_brkm(TCONF, NULL, "requires NUMA with at least 1 node");
 
-	TEST_PAUSE;
-	tst_tmpdir();
-}
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcase),
+	.test = do_test,
+	.setup = setup,
+};
 
-static void cleanup(void)
+#else /* libnuma v1 */
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+int main(void)
 {
-	tst_rmdir();
+	tst_brk(TCONF, "test is only supported on libnuma v2.");
 }
+#endif
 #else /* no NUMA */
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
 int main(void)
 {
-	tst_brkm(TCONF, NULL, "System doesn't have required numa support");
+	tst_brk(TCONF, "system doesn't have required numa support");
 }
 #endif
-- 
2.14.0

[LTP] [PATCH v2 2/2] syscalls/mbind: cleanup and rewrite mbind01 into new API

[Jan Stancek]

Petr,

I suggest changes below on top of your v2:
- drop HAVE_LINUX_MEMPOLICY_H, as it's not used by test
- replace no_check_nodemask with "expected nodemask",
- drop zero-ing of "nodemask" for MPOL_DEFAULT case, it's not needed anymore
- add "static" to global variables/functions
- print masks if comparison fails (tst_res_hexd)
- collapse no numa and numa == v1 into single main() function

No need to re-post if you are OK with the changes.

Regards,
Jan

diff --git a/testcases/kernel/syscalls/mbind/mbind01.c b/testcases/kernel/syscalls/mbind/mbind01.c
index 8db1deeb579c..648cdab97880 100644
--- a/testcases/kernel/syscalls/mbind/mbind01.c
+++ b/testcases/kernel/syscalls/mbind/mbind01.c
@@ -25,9 +25,8 @@

 #include "numa_helper.h"

-#if HAVE_NUMA_H && HAVE_LINUX_MEMPOLICY_H && HAVE_NUMAIF_H && \
-	HAVE_MPOL_CONSTANTS
-#if defined(LIBNUMA_API_VERSION) && LIBNUMA_API_VERSION == 2
+#if HAVE_NUMA_H && HAVE_NUMAIF_H && HAVE_MPOL_CONSTANTS && \
+	defined(LIBNUMA_API_VERSION) && LIBNUMA_API_VERSION >= 2
 #include "tst_test.h"

 #define MEM_LENGTH (4 * 1024 * 1024)
@@ -37,20 +36,20 @@
 #define POLICY_DESC(x) .policy = x, .desc = #x
 #define POLICY_DESC_TEXT(x, y) .policy = x, .desc = #x" ("y")"

-struct bitmask *nodemask, *getnodemask;
+static struct bitmask *nodemask, *getnodemask, *empty_nodemask;

-void test_default(unsigned int i, char *p);
-void test_none(unsigned int i, char *p);
-void test_invalid_nodemask(unsigned int i, char *p);
+static void test_default(unsigned int i, char *p);
+static void test_none(unsigned int i, char *p);
+static void test_invalid_nodemask(unsigned int i, char *p);

 struct test_case {
 	int policy;
 	const char *desc;
 	unsigned flags;
-	unsigned no_check_nodemask;
 	int ret;
 	int err;
 	void (*test)(unsigned int, char *);
+	struct bitmask **exp_nodemask;
 };

 static struct test_case tcase[] = {
@@ -59,6 +58,7 @@ static struct test_case tcase[] = {
 		.ret = 0,
 		.err = 0,
 		.test = test_none,
+		.exp_nodemask = &empty_nodemask,
 	},
 	{
 		POLICY_DESC_TEXT(MPOL_DEFAULT, "target exists"),
@@ -77,6 +77,7 @@ static struct test_case tcase[] = {
 		.ret = 0,
 		.err = 0,
 		.test = test_default,
+		.exp_nodemask = &nodemask,
 	},
 	{
 		POLICY_DESC_TEXT(MPOL_INTERLEAVE, "no target"),
@@ -89,11 +90,11 @@ static struct test_case tcase[] = {
 		.ret = 0,
 		.err = 0,
 		.test = test_default,
+		.exp_nodemask = &nodemask,
 	},
 	{
-		POLICY_DESC(MPOL_PREFERRED),
+		POLICY_DESC_TEXT(MPOL_PREFERRED, "no target"),
 		.ret = 0,
-		.no_check_nodemask = 1,
 		.err = 0,
 		.test = test_none,
 	},
@@ -102,6 +103,7 @@ static struct test_case tcase[] = {
 		.ret = 0,
 		.err = 0,
 		.test = test_default,
+		.exp_nodemask = &nodemask,
 	},
 	{
 		POLICY_DESC(UNKNOWN_POLICY),
@@ -124,19 +126,25 @@ static struct test_case tcase[] = {
 	},
 };

-void test_default(unsigned int i, char *p) {
+static void test_default(unsigned int i, char *p)
+{
 	struct test_case *tc = &tcase[i];
+
 	TEST(mbind(p, MEM_LENGTH, tc->policy, nodemask->maskp,
 		   nodemask->size, tc->flags));
 }

-void test_none(unsigned int i, char *p) {
+static void test_none(unsigned int i, char *p)
+{
 	struct test_case *tc = &tcase[i];
+
 	TEST(mbind(p, MEM_LENGTH, tc->policy, NULL, 0, tc->flags));
 }

-void test_invalid_nodemask(unsigned int i, char *p) {
+static void test_invalid_nodemask(unsigned int i, char *p)
+{
 	struct test_case *tc = &tcase[i];
+
 	/* use invalid nodemask (64 MiB after heap) */
 	TEST(mbind(p, MEM_LENGTH, tc->policy, sbrk(0) + 64*1024*1024,
 		   NUMA_NUM_NODES, tc->flags));
@@ -146,6 +154,7 @@ static void setup(void)
 {
 	if (!is_numa(NULL, NH_MEMS, 1))
 		tst_brk(TCONF, "requires NUMA with at least 1 node");
+	empty_nodemask = numa_allocate_nodemask();
 }

 static void setup_node(void)
@@ -163,7 +172,7 @@ static void setup_node(void)
 static void do_test(unsigned int i)
 {
 	struct test_case *tc = &tcase[i];
-	int policy;
+	int policy, fail = 0;
 	char *p = NULL;

 	tst_res(TINFO, "case %s", tc->desc);
@@ -182,58 +191,53 @@ static void do_test(unsigned int i)
 		TEST(get_mempolicy(&policy, getnodemask->maskp,
 				   getnodemask->size, p, MPOL_F_ADDR));
 		if (TEST_RETURN < 0) {
-			tst_res(TFAIL | TERRNO, "get_mempolicy failed");
+			tst_res(TFAIL | TTERRNO, "get_mempolicy failed");
 			return;
 		}
-
-		/* get_mempolicy doesn't return nodemask for policy MPOL_DEFAULT */
-		if (tc->policy == MPOL_DEFAULT)
-			numa_bitmask_clearall(nodemask);
-
 		if (tc->policy != policy) {
 			tst_res(TFAIL, "Wrong policy: %d, expected: %d",
 				tc->policy, policy);
-			return;
+			fail = 1;
 		}
-
-		if (!tc->no_check_nodemask && !numa_bitmask_equal(nodemask,
-								  getnodemask)) {
-			tst_res(TFAIL, "masks are not equal");
-			return;
+		if (tc->exp_nodemask) {
+			struct bitmask *exp_mask = *(tc->exp_nodemask);
+
+			if (!numa_bitmask_equal(exp_mask, getnodemask)) {
+				tst_res(TFAIL, "masks are not equal");
+				tst_res_hexd(TINFO, exp_mask->maskp,
+					exp_mask->size / 8, "exp_mask: ");
+				tst_res_hexd(TINFO, getnodemask->maskp,
+					getnodemask->size / 8, "returned: ");
+				fail = 1;
+			}
 		}
 	}

-	if (TEST_ERRNO != tc->err)
-		tst_res(TFAIL | TERRNO, "test mbind() failed: %d, expected: %d",
-			TEST_ERRNO, tc->err);
-	else if (TEST_RETURN != tc->ret)
-		tst_res(TFAIL | TERRNO,
-			"test mbind() wrong return code: %ld, expected: %d",
+	if (TEST_RETURN != tc->ret) {
+		tst_res(TFAIL, "wrong return code: %ld, expected: %d",
 			TEST_RETURN, tc->ret);
-	else
+		fail = 1;
+	}
+	if (TEST_RETURN == -1 && TEST_ERRNO != tc->err) {
+		tst_res(TFAIL | TTERRNO, "expected errno: %s, got",
+			tst_strerrno(tc->err));
+		fail = 1;
+	}
+	if (!fail)
 		tst_res(TPASS, "Test passed");
 }

-
 static struct tst_test test = {
 	.tcnt = ARRAY_SIZE(tcase),
 	.test = do_test,
 	.setup = setup,
 };

-#else /* libnuma v1 */
-#define TST_NO_DEFAULT_MAIN
-#include "tst_test.h"
-int main(void)
-{
-	tst_brk(TCONF, "test is only supported on libnuma v2.");
-}
-#endif
-#else /* no NUMA */
+#else /* libnuma >= 2 */
 #define TST_NO_DEFAULT_MAIN
 #include "tst_test.h"
 int main(void)
 {
-	tst_brk(TCONF, "system doesn't have required numa support");
+	tst_brk(TCONF, "test requires libnuma >= 2.");
 }
 #endif

[LTP] [PATCH v2 2/2] syscalls/mbind: cleanup and rewrite mbind01 into new API

[Petr Vorel]

Hi Jan,

thanks for your review.

> I suggest changes below on top of your v2:
> - drop HAVE_LINUX_MEMPOLICY_H, as it's not used by test
> - replace no_check_nodemask with "expected nodemask",

> - drop zero-ing of "nodemask" for MPOL_DEFAULT case, it's not needed anymore
I'm ok with all but this one. It's needed for a first case (POLICY_DESC(MPOL_DEFAULT),),
no mater whether it's first or not. So I didn't put expected_nodemask to it in my v3. Or
am I'm missing something.

> - add "static" to global variables/functions
> - print masks if comparison fails (tst_res_hexd)
> - collapse no numa and numa == v1 into single main() function


> No need to re-post if you are OK with the changes.
I understand this as you've already prepared your changes. In that case push it. Otherwise
I've got v3 which I can post.


Kind regards,
Petr

[LTP] [PATCH v2 2/2] syscalls/mbind: cleanup and rewrite mbind01 into new API

[Jan Stancek]

----- Original Message -----
> Hi Jan,
> 
> thanks for your review.
> 
> > I suggest changes below on top of your v2:
> > - drop HAVE_LINUX_MEMPOLICY_H, as it's not used by test
> > - replace no_check_nodemask with "expected nodemask",
> 
> > - drop zero-ing of "nodemask" for MPOL_DEFAULT case, it's not needed
> > anymore
> I'm ok with all but this one. It's needed for a first case
> (POLICY_DESC(MPOL_DEFAULT),),
> no mater whether it's first or not. So I didn't put expected_nodemask to it
> in my v3. Or
> am I'm missing something.

It's not needed, because we compare against "empty_nodemask"
in this case. "nodemask" is not used at all. mbind() doesn't use
any mask (in test_none()) and numa_bitmask_equal() compares
"getnodemask" with "empty_nodemask".

Regards,
Jan

> 
> > - add "static" to global variables/functions
> > - print masks if comparison fails (tst_res_hexd)
> > - collapse no numa and numa == v1 into single main() function
> 
> 
> > No need to re-post if you are OK with the changes.
> I understand this as you've already prepared your changes. In that case push
> it. Otherwise
> I've got v3 which I can post.
> 
> 
> Kind regards,
> Petr
> 

[LTP] [PATCH v2 2/2] syscalls/mbind: cleanup and rewrite mbind01 into new API

[Petr Vorel]

Hi Jan,

> > I'm ok with all but this one. It's needed for a first case
> > (POLICY_DESC(MPOL_DEFAULT),),
> > no mater whether it's first or not. So I didn't put expected_nodemask to it
> > in my v3. Or
> > am I'm missing something.

> It's not needed, because we compare against "empty_nodemask"
> in this case. "nodemask" is not used at all. mbind() doesn't use
> any mask (in test_none()) and numa_bitmask_equal() compares
> "getnodemask" with "empty_nodemask".

Thanks for an explanation. Please push it all.


Kind regards,
Petr

[LTP] [PATCH v2 2/2] syscalls/mbind: cleanup and rewrite mbind01 into new API

[Jan Stancek]

----- Original Message -----
> Hi Jan,
> 
> > > I'm ok with all but this one. It's needed for a first case
> > > (POLICY_DESC(MPOL_DEFAULT),),
> > > no mater whether it's first or not. So I didn't put expected_nodemask to
> > > it
> > > in my v3. Or
> > > am I'm missing something.
> 
> > It's not needed, because we compare against "empty_nodemask"
> > in this case. "nodemask" is not used at all. mbind() doesn't use
> > any mask (in test_none()) and numa_bitmask_equal() compares
> > "getnodemask" with "empty_nodemask".
> 
> Thanks for an explanation. Please push it all.

Pushed.

I used a version that adds int cast for 1/2,
no additional changes in 2/2.

Regards,
Jan

[LTP] [PATCH v2 2/2] syscalls/mbind: cleanup and rewrite mbind01 into new API

[Petr Vorel]

Hi Jan,

> Pushed.

> I used a version that adds int cast for 1/2,
> no additional changes in 2/2.
Thanks for finishing my work. I'll try not to leave silly mistakes next time.

I might add safe macros/functions for some of NUMA functions (is_numa(), numa_available(),
mbind(), get_mempolicy() are candidates).

Kind regards,
Petr