Skip to contentSkip to conversation

syscalls/move_pages: move_pages() can return positive value

PATCH New Jan Stancek 2 participants 4 messages Aug 27, 2018 – Aug 28, 2018

lore
Share & tools
lore

Jan Stancek

author
#0 lore raw

PATCH 1/1 syscalls/move_pages: move_pages() can return positive value

Since the rework done in a49bd4d71637 ("mm, numa: rework do_pages_move"), move_pages() can return also positive value.

This is similar to how migrate_pages() behaves. Current man page does not reflect commit above, man-page thread can be followed here:
https://marc.info/?t=153511369900003&r=1&w=2

This patch only changes immediate test condition after syscall, so they don't treat positive value as FAIL. Test can still fail on subsequent checks.

Fixes #388
Signed-off-by Jan Stancek <[email protected]>
---
 testcases/kernel/syscalls/move_pages/move_pages01.c | 4 +++-
 testcases/kernel/syscalls/move_pages/move_pages02.c | 4 +++-
 testcases/kernel/syscalls/move_pages/move_pages03.c | 4 +++-
 testcases/kernel/syscalls/move_pages/move_pages04.c | 2 ++
 testcases/kernel/syscalls/move_pages/move_pages05.c | 2 ++
 testcases/kernel/syscalls/move_pages/move_pages09.c | 2 +-
 testcases/kernel/syscalls/move_pages/move_pages12.c | 2 +-
 7 files changed, 15 insertions(+), 5 deletions(-)
testcases/kernel/syscalls/move_pages/move_pages01.c +3 -1
index f541e9f227c1..0e83106454fc 100644
@@ -89,10 +89,12 @@ int main(int argc, char **argv)
 			continue;
 
 		ret = numa_move_pages(0, TEST_PAGES, pages, NULL, status, 0);
-		if (ret != 0) {
+		if (ret == -1) {
 			tst_resm(TFAIL|TERRNO, "move_pages failed");
 			free_pages(pages, TEST_PAGES);
 			continue;
+		} else if (ret > 0) {
+			tst_resm(TINFO, "move_pages() returned %d\n", ret);
 		}
 
 		verify_pages_linear(pages, status, TEST_PAGES);
testcases/kernel/syscalls/move_pages/move_pages02.c +3 -1
index 728f88f5ad2e..827a152cf14c 100644
@@ -103,10 +103,12 @@ int main(int argc, char **argv)
 		ret =
 		    numa_move_pages(0, TEST_PAGES, pages, nodes, status,
 				    MPOL_MF_MOVE);
-		if (ret != 0) {
+		if (ret == -1) {
 			tst_resm(TFAIL|TERRNO, "move_pages failed");
 			free_pages(pages, TEST_PAGES);
 			continue;
+		} else if (ret > 0) {
+			tst_resm(TINFO, "move_pages() returned %d\n", ret);
 		}
 
 		for (i = 0; i < TEST_PAGES; i++)
testcases/kernel/syscalls/move_pages/move_pages03.c +3 -1
index 88a94d92dd9e..c65597c1d36d 100644
@@ -164,9 +164,11 @@ int main(int argc, char **argv)
 
 		ret = numa_move_pages(0, TEST_PAGES, pages, nodes,
 				      status, MPOL_MF_MOVE_ALL);
-		if (ret != 0) {
+		if (ret == -1) {
 			tst_resm(TFAIL|TERRNO, "move_pages failed");
 			goto err_kill_child;
+		} else if (ret > 0) {
+			tst_resm(TINFO, "move_pages() returned %d\n", ret);
 		}
 
 		verify_pages_on_node(pages, status, TEST_PAGES, to_node);
testcases/kernel/syscalls/move_pages/move_pages04.c +2 -0
index bf24a0895261..df5e334e23e3 100644
@@ -131,6 +131,8 @@ int main(int argc, char **argv)
 			tst_resm(TFAIL | TERRNO,
 				 "move_pages unexpectedly failed");
 			goto err_free_pages;
+		} else if (ret > 0) {
+			tst_resm(TINFO, "move_pages() returned %d\n", ret);
 		}
 
 		if (status[UNTOUCHED_PAGE] == exp_status) {
testcases/kernel/syscalls/move_pages/move_pages05.c +2 -0
index 51ac058e058b..7db3c47a9fb4 100644
@@ -175,6 +175,8 @@ int main(int argc, char **argv)
 			tst_resm(TFAIL | TERRNO,
 				 "move_pages unexpectedly failed");
 			goto err_kill_child;
+		} else if (ret > 0) {
+			tst_resm(TINFO, "move_pages() returned %d\n", ret);
 		}
 
 		if (status[SHARED_PAGE] == -EACCES)
testcases/kernel/syscalls/move_pages/move_pages09.c +1 -1
index 273c750ff8f9..5c2d8d320d12 100644
@@ -109,7 +109,7 @@ int main(int argc, char **argv)
 		 *     mm: stop returning -ENOENT from sys_move_pages() if nothing got migrated
 		 */
 		if ((tst_kvercmp(2, 6, 28)) >= 0) {
-			if (ret == 0)
+			if (ret >= 0)
 				tst_resm(TPASS, "move_pages succeeded");
 			else
 				tst_resm(TFAIL | TERRNO, "move_pages");
testcases/kernel/syscalls/move_pages/move_pages12.c +1 -1
index b1187e307559..c0ff27114110 100644
@@ -87,7 +87,7 @@ static void do_child(void)
 
 		TEST(numa_move_pages(ppid, test_pages,
 			pages, nodes, status, MPOL_MF_MOVE_ALL));
-		if (TST_RET) {
+		if (TST_RET == -1) {
 			tst_res(TFAIL | TTERRNO, "move_pages failed");
 			break;
 		}
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH 1/1 syscalls/move_pages: move_pages() can return positive value

Hi!

testcases/kernel/syscalls/move_pages/move_pages01.c int main(int argc, char **argv) · line 92

Quoted 3 lines
testcases/kernel/syscalls/move_pages/move_pages01.c +1 -1
 		ret = numa_move_pages(0, TEST_PAGES, pages, NULL, status, 0);
-		if (ret != 0) {
+		if (ret == -1) {

Maybe we should go for (ret < 0) but other than that this looks good.

Signature · 2 lines
Cyril Hrubis
[email protected]
author
#2 lore raw

PATCH 1/1 syscalls/move_pages: move_pages() can return positive value

----- Original Message -----

Quoted 6 lines

Hi!

testcases/kernel/syscalls/move_pages/move_pages01.c int main(int argc, char **argv) · line 92

Quoted 3 lines
testcases/kernel/syscalls/move_pages/move_pages01.c +1 -1
 		ret = numa_move_pages(0, TEST_PAGES, pages, NULL, status, 0);
-		if (ret != 0) {
+		if (ret == -1) {

Maybe we should go for (ret < 0) but other than that this looks good.

Agreed, we don't check for other negative values, so we better report those as failures too.

Jan

Quoted 5 lines

--
Cyril Hrubis
[email protected]

author
#3 lore raw

PATCH 1/1 syscalls/move_pages: move_pages() can return positive value

----- Original Message -----

Quoted 11 lines

----- Original Message -----

Quoted 6 lines

Hi!

testcases/kernel/syscalls/move_pages/move_pages01.c int main(int argc, char **argv) · line 92

Quoted 3 lines
testcases/kernel/syscalls/move_pages/move_pages01.c +1 -1
 		ret = numa_move_pages(0, TEST_PAGES, pages, NULL, status, 0);
-		if (ret != 0) {
+		if (ret == -1) {

Maybe we should go for (ret < 0) but other than that this looks good.

Agreed, we don't check for other negative values, so we better report those as failures too.

Pushed with suggested change and your ACK.

Regards,
Jan