All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api
@ 2024-02-20  7:42 Yang Xu via ltp
  2024-02-20  7:42 ` [LTP] [PATCH v4 2/7] libltpswap: alter tst_count_swaps api Yang Xu via ltp
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Yang Xu via ltp @ 2024-02-20  7:42 UTC (permalink / raw)
  To: ltp

Current, the kernel constant MAX_SWAPFILES value is calculated as below
------------------------------------
//#ifdef CONFIG_DEVICE_PRIVATE
//#define SWP_DEVICE_NUM 4
//#else
//#define SWP_DEVICE_NUM 0
//#endif

//#ifdef CONFIG_MIGRATION
//#define SWP_MIGRATION_NUM 3
//#else
//#define SWP_MIGRATION_NUM 0

//#ifdef CONFIG_MEMORY_FAILURE
//#define SWP_HWPOISON_NUM 1
//#else
//#define SWP_HWPOISON_NUM 0
//#endif

//#define SWP_PTE_MARKER_NUM 1
//#define MAX_SWAPFILES_SHIFT   5

//#define MAX_SWAPFILES \
//      ((1 << MAX_SWAPFILES_SHIFT) - SWP_DEVICE_NUM - \
//      SWP_MIGRATION_NUM - SWP_HWPOISON_NUM - \
//      SWP_PTE_MARKER_NUM)
------------------------------------
Also, man-pages missed something after 5.14 kernel. I have sent two patches[1][2] to
add it. The kernel patches modify this kernel constant in[3][4][5][6]. Also, after kernel 6.2.0
[7],kernel always compile in pte markers.

[1]https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=26f3ec74e
[2]https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=6bf3937fc
[3]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/swap.h?id=5042db43cc
[4]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/swap.h?id=b756a3b5e
[5]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/swap.h?id=679d10331
[6]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/swap.h?id=6c287605f
[7]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/swap.h?id=ca92ea3dc5

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/libswap.h         |  6 +++++
 libs/libltpswap/libswap.c | 48 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/include/libswap.h b/include/libswap.h
index bdc5aacc6..361d73175 100644
--- a/include/libswap.h
+++ b/include/libswap.h
@@ -21,4 +21,10 @@ int make_swapfile(const char *swapfile, int blocks, int safe);
  * we are testing on.
  */
 bool is_swap_supported(const char *filename);
+
+/*
+ * Get kernel constant MAX_SWAPFILES value
+ */
+int tst_max_swapfiles(void);
+
 #endif /* __LIBSWAP_H__ */
diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index c79de19d7..a404a4ada 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -16,6 +16,8 @@
 #include "tst_test.h"
 #include "libswap.h"
 #include "lapi/syscalls.h"
+#include "tst_kconfig.h"
+#include "tst_safe_stdio.h"
 
 static const char *const swap_supported_fs[] = {
 	"btrfs",
@@ -217,3 +219,49 @@ bool is_swap_supported(const char *filename)
 
 	return true;
 }
+
+/*
+ * Get kernel constant MAX_SWAPFILES value
+ */
+int tst_max_swapfiles(void)
+{
+	unsigned int max_swapfile = 32;
+	unsigned int swp_migration_num = 0, swp_hwpoison_num = 0, swp_device_num = 0, swp_pte_marker_num = 0;
+	struct tst_kconfig_var migration_kconfig = TST_KCONFIG_INIT("CONFIG_MIGRATION");
+	struct tst_kconfig_var memory_kconfig = TST_KCONFIG_INIT("CONFIG_MEMORY_FAILURE");
+	struct tst_kconfig_var device_kconfig = TST_KCONFIG_INIT("CONFIG_DEVICE_PRIVATE");
+	struct tst_kconfig_var marker_kconfig = TST_KCONFIG_INIT("CONFIG_PTE_MARKER");
+
+	tst_kconfig_read(&migration_kconfig, 1);
+	tst_kconfig_read(&memory_kconfig, 1);
+	tst_kconfig_read(&device_kconfig, 1);
+	tst_kconfig_read(&marker_kconfig, 1);
+
+	if (migration_kconfig.choice == 'y') {
+		if (tst_kvercmp(5, 19, 0) < 0)
+			swp_migration_num = 2;
+		else
+			swp_migration_num = 3;
+	}
+
+	if (memory_kconfig.choice == 'y')
+		swp_hwpoison_num = 1;
+
+	if (device_kconfig.choice == 'y') {
+		if (tst_kvercmp(4, 14, 0) >= 0)
+			swp_device_num = 2;
+		if (tst_kvercmp(5, 14, 0) >= 0)
+			swp_device_num = 4;
+	}
+
+	if (tst_kvercmp(6, 2, 0) >= 0) {
+		swp_pte_marker_num = 1;
+	} else {
+		if (marker_kconfig.choice == 'y') {
+			if (tst_kvercmp(5, 19, 0) >= 0)
+				swp_pte_marker_num = 1;
+		}
+	}
+
+	return max_swapfile - swp_migration_num - swp_hwpoison_num - swp_device_num - swp_pte_marker_num;
+}
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH v4 2/7] libltpswap: alter tst_count_swaps api
  2024-02-20  7:42 [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Yang Xu via ltp
@ 2024-02-20  7:42 ` Yang Xu via ltp
  2024-02-23  9:37   ` Petr Vorel
  2024-02-20  7:42 ` [LTP] [PATCH v4 3/7] syscalls/swapon03: use tst_max_swapfiles() and GET_USED_SWAPFILES() api Yang Xu via ltp
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Yang Xu via ltp @ 2024-02-20  7:42 UTC (permalink / raw)
  To: ltp

Like we count the ipc resource total, we can also add a
similar api for swapfiles, so we can use it for swapon03 case.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/libswap.h         |  5 +++++
 libs/libltpswap/libswap.c | 27 +++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/include/libswap.h b/include/libswap.h
index 361d73175..76a3ed0c0 100644
--- a/include/libswap.h
+++ b/include/libswap.h
@@ -27,4 +27,9 @@ bool is_swap_supported(const char *filename);
  */
 int tst_max_swapfiles(void);
 
+/*
+ * Get the used swapfiles number
+ */
+int tst_count_swaps(void);
+
 #endif /* __LIBSWAP_H__ */
diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index a404a4ada..1f9235f17 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -265,3 +265,30 @@ int tst_max_swapfiles(void)
 
 	return max_swapfile - swp_migration_num - swp_hwpoison_num - swp_device_num - swp_pte_marker_num;
 }
+
+/*
+ * Get the used swapfiles number
+ */
+int tst_count_swaps(void)
+{
+	FILE *fp;
+	int used = -1;
+	char c;
+
+	fp = fopen("/proc/swaps", "r");
+	if (fp == NULL) {
+		return -1;
+	}
+
+	while ((c = fgetc(fp)) != EOF) {
+		if (c == '\n')
+			used++;
+	}
+
+	fclose(fp);
+	if (used < 0) {
+		tst_brk(TBROK, "can't read /proc/swaps to get used swapfiles resource total");
+	}
+
+	return used;
+}
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH v4 3/7] syscalls/swapon03: use tst_max_swapfiles() and GET_USED_SWAPFILES() api
  2024-02-20  7:42 [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Yang Xu via ltp
  2024-02-20  7:42 ` [LTP] [PATCH v4 2/7] libltpswap: alter tst_count_swaps api Yang Xu via ltp
@ 2024-02-20  7:42 ` Yang Xu via ltp
  2024-02-21  9:37   ` Petr Vorel
  2024-02-23  9:48   ` Petr Vorel
  2024-02-20  7:42 ` [LTP] [PATCH v4 4/7] swaponoff.h: Remove useless header Yang Xu via ltp
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 17+ messages in thread
From: Yang Xu via ltp @ 2024-02-20  7:42 UTC (permalink / raw)
  To: ltp

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/syscalls/swapon/swapon03.c | 40 ++++-----------------
 1 file changed, 6 insertions(+), 34 deletions(-)

diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
index 2a0fc99e6..b0d70b4ad 100644
--- a/testcases/kernel/syscalls/swapon/swapon03.c
+++ b/testcases/kernel/syscalls/swapon/swapon03.c
@@ -19,7 +19,6 @@
 
 #include "tst_test.h"
 #include "lapi/syscalls.h"
-#include "swaponoff.h"
 #include "libswap.h"
 
 #define MNTPOINT	"mntpoint"
@@ -104,47 +103,20 @@ static void verify_swapon(void)
 static int setup_swap(void)
 {
 	pid_t pid;
-	int j, fd;
 	int status;
+	int j, max_swapfiles, used_swapfiles;
 	int res = 0;
 	char filename[FILENAME_MAX];
-	char buf[BUFSIZ + 1];
-
-	/* Find out how many swapfiles (1 line per entry) already exist */
-	swapfiles = 0;
 
 	if (seteuid(0) < 0)
 		tst_brk(TFAIL | TERRNO, "Failed to call seteuid");
 
-	/* This includes the first (header) line */
-	if ((fd = open("/proc/swaps", O_RDONLY)) == -1) {
-		tst_brk(TFAIL | TERRNO,
-			"Failed to find out existing number of swap files");
-	}
-	do {
-		char *p = buf;
-		res = read(fd, buf, BUFSIZ);
-		if (res < 0) {
-			tst_brk(TFAIL | TERRNO,
-				 "Failed to find out existing number of swap files");
-		}
-		buf[res] = '\0';
-		while ((p = strchr(p, '\n'))) {
-			p++;
-			swapfiles++;
-		}
-	} while (BUFSIZ <= res);
-	close(fd);
-	if (swapfiles)
-		swapfiles--;	/* don't count the /proc/swaps header */
-
-	if (swapfiles < 0)
-		tst_brk(TFAIL, "Failed to find existing number of swapfiles");
-
 	/* Determine how many more files are to be created */
-	swapfiles = MAX_SWAPFILES - swapfiles;
-	if (swapfiles > MAX_SWAPFILES)
-		swapfiles = MAX_SWAPFILES;
+	max_swapfiles = tst_max_swapfiles();
+	used_swapfiles = tst_count_swaps();
+	swapfiles = max_swapfiles - used_swapfiles;
+	if (swapfiles > max_swapfiles)
+		swapfiles = max_swapfiles;
 	pid = SAFE_FORK();
 	if (pid == 0) {
 		/*create and turn on remaining swapfiles */
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH v4 4/7] swaponoff.h: Remove useless header
  2024-02-20  7:42 [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Yang Xu via ltp
  2024-02-20  7:42 ` [LTP] [PATCH v4 2/7] libltpswap: alter tst_count_swaps api Yang Xu via ltp
  2024-02-20  7:42 ` [LTP] [PATCH v4 3/7] syscalls/swapon03: use tst_max_swapfiles() and GET_USED_SWAPFILES() api Yang Xu via ltp
@ 2024-02-20  7:42 ` Yang Xu via ltp
  2024-02-23 10:15   ` Petr Vorel
  2024-02-20  7:42 ` [LTP] [PATCH v4 5/7] swapon/Makefile: Remove useless section for MAX_SWAPFILES Yang Xu via ltp
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Yang Xu via ltp @ 2024-02-20  7:42 UTC (permalink / raw)
  To: ltp

Since we have use tst_max_swapfiles() api in swapon03.c, so this header
is useless and can be removed safely.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/syscalls/swapon/swaponoff.h | 8 --------
 1 file changed, 8 deletions(-)
 delete mode 100644 testcases/kernel/syscalls/swapon/swaponoff.h

diff --git a/testcases/kernel/syscalls/swapon/swaponoff.h b/testcases/kernel/syscalls/swapon/swaponoff.h
deleted file mode 100644
index 900761bda..000000000
--- a/testcases/kernel/syscalls/swapon/swaponoff.h
+++ /dev/null
@@ -1,8 +0,0 @@
-
-#ifndef __SWAP_ON_OFF_H_
-#define __SWAP_ON_OFF_H_
-
-#include <linux/version.h>
-#define MAX_SWAPFILES 30
-
-#endif
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH v4 5/7] swapon/Makefile: Remove useless section for MAX_SWAPFILES
  2024-02-20  7:42 [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Yang Xu via ltp
                   ` (2 preceding siblings ...)
  2024-02-20  7:42 ` [LTP] [PATCH v4 4/7] swaponoff.h: Remove useless header Yang Xu via ltp
@ 2024-02-20  7:42 ` Yang Xu via ltp
  2024-02-23 10:48   ` Petr Vorel
  2024-02-20  7:42 ` [LTP] [PATCH v4 6/7] syscalls/swapon03: Simply this case Yang Xu via ltp
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Yang Xu via ltp @ 2024-02-20  7:42 UTC (permalink / raw)
  To: ltp

It seems this section doesn't affect anything, btw it is useless,
so remove it.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/syscalls/swapon/Makefile | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/testcases/kernel/syscalls/swapon/Makefile b/testcases/kernel/syscalls/swapon/Makefile
index 53c795090..6954112a8 100644
--- a/testcases/kernel/syscalls/swapon/Makefile
+++ b/testcases/kernel/syscalls/swapon/Makefile
@@ -1,11 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-or-later
 # Copyright (c) International Business Machines Corp., 2001
 
-NEEDSPECIAL	:= $(shell echo MAX_SWAPFILES | $(CC) -E -xc -include linux/swap.h 2>/dev/null - | tail -n 1 | grep 32; echo $?)
-ifneq ($(strip $(NEEDSPECIAL)),)
-export CFLAGS	+= -DOLDER_DISTRO_RELEASE
-endif
-
 top_srcdir		?= ../../../..
 
 LTPLIBS = ltpswap
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH v4 6/7] syscalls/swapon03: Simply this case
  2024-02-20  7:42 [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Yang Xu via ltp
                   ` (3 preceding siblings ...)
  2024-02-20  7:42 ` [LTP] [PATCH v4 5/7] swapon/Makefile: Remove useless section for MAX_SWAPFILES Yang Xu via ltp
@ 2024-02-20  7:42 ` Yang Xu via ltp
  2024-02-23 11:48   ` Petr Vorel
  2024-02-20  7:42 ` [LTP] [PATCH v4 7/7] Add fallback for RHEL9 Yang Xu via ltp
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Yang Xu via ltp @ 2024-02-20  7:42 UTC (permalink / raw)
  To: ltp

By moving swapfile create stage from verify_swaopon and
test EPERM error more accurate. Also use glibc wrapper by
using swapon/swapoff instead of call syscall number directly
because glibc/musl/binoic also support them since long time ago.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/syscalls/swapon/swapon03.c | 168 +++++---------------
 1 file changed, 42 insertions(+), 126 deletions(-)

diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
index b0d70b4ad..339b11c83 100644
--- a/testcases/kernel/syscalls/swapon/swapon03.c
+++ b/testcases/kernel/syscalls/swapon/swapon03.c
@@ -16,7 +16,7 @@
 #include <errno.h>
 #include <stdlib.h>
 #include <sys/wait.h>
-
+#include <sys/swap.h>
 #include "tst_test.h"
 #include "lapi/syscalls.h"
 #include "libswap.h"
@@ -24,88 +24,13 @@
 #define MNTPOINT	"mntpoint"
 #define TEST_FILE	MNTPOINT"/testswap"
 
-static int setup_swap(void);
-static int clean_swap(void);
-static int check_and_swapoff(const char *filename);
-
 static int swapfiles;
 
-int testfiles = 3;
-static struct swap_testfile_t {
-	char *filename;
-} swap_testfiles[] = {
-	{"firstswapfile"},
-	{"secondswapfile"},
-	{"thirdswapfile"}
-};
-
-int expected_errno = EPERM;
-
-static void verify_swapon(void)
-{
-	if (setup_swap() < 0) {
-		clean_swap();
-		tst_brk(TBROK, "Setup failed, quitting the test");
-	}
-
-	TEST(tst_syscall(__NR_swapon, swap_testfiles[0].filename, 0));
-
-	if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
-		tst_res(TPASS | TTERRNO, "swapon(2) got expected failure");
-	} else if (TST_RET < 0) {
-		tst_res(TFAIL | TTERRNO,
-			"swapon(2) failed to produce expected error "
-			"(%d). System reboot recommended.",
-			expected_errno);
-	} else {
-		/*
-		 * Probably the system supports MAX_SWAPFILES > 30, let's try with
-		 * MAX_SWAPFILES == 32. Call swapon sys call once again for 32
-		 * now we can't receive an error.
-		 */
-		TEST(tst_syscall(__NR_swapon, swap_testfiles[1].filename, 0));
-
-		/* Check return code (now we're expecting success) */
-		if (TST_RET < 0) {
-			tst_res(TFAIL | TTERRNO,
-				"swapon(2) got an unexpected failure");
-		} else {
-			/*
-			 * Call swapon sys call once again for 33 now we have to receive an error.
-			 */
-			TEST(tst_syscall(__NR_swapon, swap_testfiles[2].filename, 0));
-
-			/* Check return code (should be an error) */
-			if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
-				tst_res(TPASS,
-					"swapon(2) got expected failure;"
-					" Got errno = %d, probably your"
-					" MAX_SWAPFILES is 32",
-					expected_errno);
-			} else {
-				tst_res(TFAIL,
-					"swapon(2) failed to produce"
-					" expected error: %d, got %s."
-					" System reboot after execution of LTP"
-					" test suite is recommended.",
-					expected_errno, strerror(TST_ERR));
-			}
-		}
-	}
-
-	if (clean_swap() < 0)
-		tst_brk(TBROK, "Cleanup failed, quitting the test");
-}
-
-/*
- * Create 33 and activate 30 swapfiles.
- */
 static int setup_swap(void)
 {
 	pid_t pid;
 	int status;
 	int j, max_swapfiles, used_swapfiles;
-	int res = 0;
 	char filename[FILENAME_MAX];
 
 	if (seteuid(0) < 0)
@@ -127,16 +52,7 @@ static int setup_swap(void)
 			make_swapfile(filename, 10, 0);
 
 			/* turn on the swap file */
-			res = tst_syscall(__NR_swapon, filename, 0);
-			if (res != 0) {
-				if (errno == EPERM) {
-					printf("Successfully created %d swapfiles\n", j);
-					break;
-				} else {
-					printf("Failed to create swapfile: %s\n", filename);
-					exit(1);
-				}
-			}
+			TST_EXP_PASS_SILENT(swapon(filename, 0));
 		}
 		exit(0);
 	} else
@@ -145,13 +61,40 @@ static int setup_swap(void)
 	if (WEXITSTATUS(status))
 		tst_brk(TFAIL, "Failed to setup swaps");
 
-	/* Create all needed extra swapfiles for testing */
-	for (j = 0; j < testfiles; j++)
-		make_swapfile(swap_testfiles[j].filename, 10, 0);
+	tst_res(TINFO, "Successfully created %d swapfiles", swapfiles);
+	make_swapfile(TEST_FILE, 10, 0);
 
 	return 0;
 }
 
+/*
+ * Check if the file is at /proc/swaps and remove it giving swapoff
+ */
+static int check_and_swapoff(const char *filename)
+{
+	char cmd_buffer[256];
+	int rc = -1;
+
+	if (snprintf(cmd_buffer, sizeof(cmd_buffer),
+		"grep -q '%s.*file' /proc/swaps", filename) < 0) {
+		tst_res(TWARN, "sprintf() failed to create the command string");
+	} else {
+		rc = 0;
+		if (system(cmd_buffer) == 0) {
+			/* now we need to swapoff the file */
+			if (swapoff(filename) != 0) {
+				tst_res(TWARN, "Failed to turn off swap "
+					"file. system reboot after "
+					"execution of LTP test suite "
+					"is recommended");
+				rc = -1;
+			}
+		}
+	}
+
+	return rc;
+}
+
 /*
  * Turn off all swapfiles previously turned on
  */
@@ -169,49 +112,17 @@ static int clean_swap(void)
 		}
 	}
 
-	for (j = 0; j < testfiles; j++) {
-		if (check_and_swapoff(swap_testfiles[j].filename) != 0) {
-			tst_res(TWARN, "Failed to turn off swap file %s.",
-				 swap_testfiles[j].filename);
-			return -1;
-		}
+	if (check_and_swapoff("testfile") != 0) {
+		tst_res(TWARN, "Failed to turn off swap file testfile");
+		return -1;
 	}
 
 	return 0;
 }
 
-/*
- * Check if the file is at /proc/swaps and remove it giving swapoff
- */
-static int check_and_swapoff(const char *filename)
+static void verify_swapon(void)
 {
-	char cmd_buffer[256];
-	int rc = -1;
-
-	if (snprintf(cmd_buffer, sizeof(cmd_buffer),
-		     "grep -q '%s.*file' /proc/swaps", filename) < 0) {
-		tst_res(TWARN, "sprintf() failed to create the command string");
-	} else {
-
-		rc = 0;
-
-		if (system(cmd_buffer) == 0) {
-
-			/* now we need to swapoff the file */
-			if (tst_syscall(__NR_swapoff, filename) != 0) {
-
-				tst_res(TWARN, "Failed to turn off swap "
-					 "file. system reboot after "
-					 "execution of LTP test suite "
-					 "is recommended");
-				rc = -1;
-
-			}
-
-		}
-	}
-
-	return rc;
+	TST_EXP_FAIL(swapon(TEST_FILE, 0), EPERM, "swapon(%s, 0)", TEST_FILE);
 }
 
 static void setup(void)
@@ -220,6 +131,11 @@ static void setup(void)
 		tst_brk(TCONF, "swap not supported by kernel");
 
 	is_swap_supported(TEST_FILE);
+
+	if (setup_swap() < 0) {
+		clean_swap();
+		tst_brk(TBROK, "Setup failed, quitting the test");
+	}
 }
 
 static void cleanup(void)
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [LTP] [PATCH v4 7/7] Add fallback for RHEL9
  2024-02-20  7:42 [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Yang Xu via ltp
                   ` (4 preceding siblings ...)
  2024-02-20  7:42 ` [LTP] [PATCH v4 6/7] syscalls/swapon03: Simply this case Yang Xu via ltp
@ 2024-02-20  7:42 ` Yang Xu via ltp
  2024-02-23 11:54   ` Petr Vorel
  2024-02-23  9:04 ` [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Petr Vorel
  2024-02-23 11:56 ` Petr Vorel
  7 siblings, 1 reply; 17+ messages in thread
From: Yang Xu via ltp @ 2024-02-20  7:42 UTC (permalink / raw)
  To: ltp

Since device number patch and pte number patch have been backported into
RHEL9,  we should add fallback for this distro.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 libs/libltpswap/libswap.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index 1f9235f17..94de4b4e2 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -17,6 +17,7 @@
 #include "libswap.h"
 #include "lapi/syscalls.h"
 #include "tst_kconfig.h"
+#include "tst_kvercmp.h"
 #include "tst_safe_stdio.h"
 
 static const char *const swap_supported_fs[] = {
@@ -231,6 +232,11 @@ int tst_max_swapfiles(void)
 	struct tst_kconfig_var memory_kconfig = TST_KCONFIG_INIT("CONFIG_MEMORY_FAILURE");
 	struct tst_kconfig_var device_kconfig = TST_KCONFIG_INIT("CONFIG_DEVICE_PRIVATE");
 	struct tst_kconfig_var marker_kconfig = TST_KCONFIG_INIT("CONFIG_PTE_MARKER");
+	struct tst_kern_exv kvers[] = {
+		/* RHEL9 kernel has patch 6c287605f and 679d10331 since 5.14.0-179 */
+		{ "RHEL9", "5.14.0-179" },
+		{ NULL, NULL},
+	};
 
 	tst_kconfig_read(&migration_kconfig, 1);
 	tst_kconfig_read(&memory_kconfig, 1);
@@ -238,7 +244,7 @@ int tst_max_swapfiles(void)
 	tst_kconfig_read(&marker_kconfig, 1);
 
 	if (migration_kconfig.choice == 'y') {
-		if (tst_kvercmp(5, 19, 0) < 0)
+		if (tst_kvercmp2(5, 19, 0, kvers) < 0)
 			swp_migration_num = 2;
 		else
 			swp_migration_num = 3;
@@ -258,7 +264,7 @@ int tst_max_swapfiles(void)
 		swp_pte_marker_num = 1;
 	} else {
 		if (marker_kconfig.choice == 'y') {
-			if (tst_kvercmp(5, 19, 0) >= 0)
+			if (tst_kvercmp2(5, 19, 0, kvers) >= 0)
 				swp_pte_marker_num = 1;
 		}
 	}
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v4 3/7] syscalls/swapon03: use tst_max_swapfiles() and GET_USED_SWAPFILES() api
  2024-02-20  7:42 ` [LTP] [PATCH v4 3/7] syscalls/swapon03: use tst_max_swapfiles() and GET_USED_SWAPFILES() api Yang Xu via ltp
@ 2024-02-21  9:37   ` Petr Vorel
  2024-02-22  1:27     ` Yang Xu (Fujitsu) via ltp
  2024-02-23  9:48   ` Petr Vorel
  1 sibling, 1 reply; 17+ messages in thread
From: Petr Vorel @ 2024-02-21  9:37 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Xu,

I see swapon03 failures on aarch64 and ppc64le on SLES and openSUSE after this commit.

Here is timeout after 31s:

# ./swapon03
...
tst_test.c:1701: TINFO: === Testing on ext2 ===
tst_test.c:1117: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
mke2fs 1.47.0 (5-Feb-2023)
tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swaVqwa6f/mntpoint fstyp=ext2 flags=0
tst_ioctl.c:26: TINFO: FIBMAP ioctl is supported
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
Test timeouted, sending SIGKILL!
tst_test.c:1679: TINFO: If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1
tst_test.c:1681: TBROK: Test killed! (timeout?)

Summary:
passed   0
failed   0
broken   1
skipped  0
warnings 0
### TEST swapon03 COMPLETE >>> 2.

I tried to run with .max_runtime = 60, but even then it fails after 1m 30s:
...
tst_supported_fs_types.c:97: TINFO: Kernel supports tmpfs
tst_supported_fs_types.c:49: TINFO: mkfs is not needed for tmpfs
tst_test.c:1701: TINFO: === Testing on ext2 ===
tst_test.c:1117: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
mke2fs 1.47.0 (5-Feb-2023)
tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swaQsjhAp/mntpoint fstyp=ext2 flags=0
tst_ioctl.c:26: TINFO: FIBMAP ioctl is supported
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
Test timeouted, sending SIGKILL!
tst_test.c:1679: TINFO: If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1
tst_test.c:1681: TBROK: Test killed! (timeout?)

Summary:
passed   0
failed   0
broken   1
skipped  0
warnings 0

I'm trying with LTP_TIMEOUT_MUL=3.

BTW there is still broken swapoff01 on master on ppc64le which I reported [1]:
libswap.c:153: TBROK: Failed to create swapfile
(obviously no change in this patchset)

But I'll ping Li separately.

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/20240131190122.GB30901@pevik/

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v4 3/7] syscalls/swapon03: use tst_max_swapfiles() and GET_USED_SWAPFILES() api
  2024-02-21  9:37   ` Petr Vorel
@ 2024-02-22  1:27     ` Yang Xu (Fujitsu) via ltp
  0 siblings, 0 replies; 17+ messages in thread
From: Yang Xu (Fujitsu) via ltp @ 2024-02-22  1:27 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp


> Hi Xu,
> 
> I see swapon03 failures on aarch64 and ppc64le on SLES and openSUSE after this commit.
> 
> Here is timeout after 31s:
> 
> # ./swapon03
> ...
> tst_test.c:1701: TINFO: === Testing on ext2 ===
> tst_test.c:1117: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
> mke2fs 1.47.0 (5-Feb-2023)
> tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swaVqwa6f/mntpoint fstyp=ext2 flags=0
> tst_ioctl.c:26: TINFO: FIBMAP ioctl is supported
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> Test timeouted, sending SIGKILL!
> tst_test.c:1679: TINFO: If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1
> tst_test.c:1681: TBROK: Test killed! (timeout?)
> 
> Summary:
> passed   0
> failed   0
> broken   1
> skipped  0
> warnings 0
> ### TEST swapon03 COMPLETE >>> 2.
> 
> I tried to run with .max_runtime = 60, but even then it fails after 1m 30s:
> ...
> tst_supported_fs_types.c:97: TINFO: Kernel supports tmpfs
> tst_supported_fs_types.c:49: TINFO: mkfs is not needed for tmpfs
> tst_test.c:1701: TINFO: === Testing on ext2 ===
> tst_test.c:1117: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
> mke2fs 1.47.0 (5-Feb-2023)
> tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swaQsjhAp/mntpoint fstyp=ext2 flags=0
> tst_ioctl.c:26: TINFO: FIBMAP ioctl is supported
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> Test timeouted, sending SIGKILL!
> tst_test.c:1679: TINFO: If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1
> tst_test.c:1681: TBROK: Test killed! (timeout?)
> 
> Summary:
> passed   0
> failed   0
> broken   1
> skipped  0
> warnings 0
> 
> I'm trying with LTP_TIMEOUT_MUL=3.
> 
> BTW there is still broken swapoff01 on master on ppc64le which I reported [1]:
> libswap.c:153: TBROK: Failed to create swapfile
> (obviously no change in this patchset)
> 
> But I'll ping Li separately.
> 
> Kind regards,
> Petr
> 
> [1] https://lore.kernel.org/ltp/20240131190122.GB30901@pevik/

To be honest, I don't know why this commit leads case hang.
So can you add some debug info in swapon03 between tst_max_swapfiles and 
tst_count_swaps
api(I can't reproduce it becase I don't have situation)
ps: I guess the following way maybe lead hang in tst_count_swaps
"
+	while ((c = fgetc(fp)) != EOF) {
+		if (c == '\n')
+			used++;
+	}
"

Maybe I should use the old way[1]

[1]https://patchwork.ozlabs.org/project/ltp/patch/20231222050006.148845-2-xuyang2018.jy@fujitsu.com/

Best Regards
Yang Xu

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api
  2024-02-20  7:42 [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Yang Xu via ltp
                   ` (5 preceding siblings ...)
  2024-02-20  7:42 ` [LTP] [PATCH v4 7/7] Add fallback for RHEL9 Yang Xu via ltp
@ 2024-02-23  9:04 ` Petr Vorel
  2024-02-23 11:56 ` Petr Vorel
  7 siblings, 0 replies; 17+ messages in thread
From: Petr Vorel @ 2024-02-23  9:04 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Yang Xu,

Reviewed-by: Petr Vorel <pvorel@suse.cz>
generally LGTM, just minor things below.

nit in subject: s/api/API/

> Current, the kernel constant MAX_SWAPFILES value is calculated as below
nit: I'm not a native speaker, but:
s/Current/Currently/

> ------------------------------------
> //#ifdef CONFIG_DEVICE_PRIVATE
> //#define SWP_DEVICE_NUM 4
> //#else
> //#define SWP_DEVICE_NUM 0
> //#endif

> //#ifdef CONFIG_MIGRATION
> //#define SWP_MIGRATION_NUM 3
> //#else
> //#define SWP_MIGRATION_NUM 0

> //#ifdef CONFIG_MEMORY_FAILURE
> //#define SWP_HWPOISON_NUM 1
> //#else
> //#define SWP_HWPOISON_NUM 0
> //#endif

> //#define SWP_PTE_MARKER_NUM 1
> //#define MAX_SWAPFILES_SHIFT   5

> //#define MAX_SWAPFILES \
> //      ((1 << MAX_SWAPFILES_SHIFT) - SWP_DEVICE_NUM - \
> //      SWP_MIGRATION_NUM - SWP_HWPOISON_NUM - \
> //      SWP_PTE_MARKER_NUM)

nit: if you indent this code with space or tab it will be visible without //.
Using // makes text harder to read.

> ------------------------------------
> Also, man-pages missed something after 5.14 kernel. I have sent two patches[1][2] to
> add it. The kernel patches modify this kernel constant in[3][4][5][6]. Also, after kernel 6.2.0
> [7],kernel always compile in pte markers.

> [1]https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=26f3ec74e
> [2]https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=6bf3937fc
> [3]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/swap.h?id=5042db43cc
> [4]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/swap.h?id=b756a3b5e
> [5]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/swap.h?id=679d10331
> [6]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/swap.h?id=6c287605f
> [7]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/swap.h?id=ca92ea3dc5

> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  include/libswap.h         |  6 +++++
>  libs/libltpswap/libswap.c | 48 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+)

> diff --git a/include/libswap.h b/include/libswap.h
> index bdc5aacc6..361d73175 100644
> --- a/include/libswap.h
> +++ b/include/libswap.h
> @@ -21,4 +21,10 @@ int make_swapfile(const char *swapfile, int blocks, int safe);
>   * we are testing on.
>   */
>  bool is_swap_supported(const char *filename);
> +
> +/*
> + * Get kernel constant MAX_SWAPFILES value
nit: . at the end.

> + */
> +int tst_max_swapfiles(void);
> +
>  #endif /* __LIBSWAP_H__ */
> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index c79de19d7..a404a4ada 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -16,6 +16,8 @@
>  #include "tst_test.h"
>  #include "libswap.h"
>  #include "lapi/syscalls.h"
> +#include "tst_kconfig.h"
> +#include "tst_safe_stdio.h"

>  static const char *const swap_supported_fs[] = {
>  	"btrfs",
> @@ -217,3 +219,49 @@ bool is_swap_supported(const char *filename)

>  	return true;
>  }
> +
> +/*
> + * Get kernel constant MAX_SWAPFILES value
nit: . at the end.

> + */
> +int tst_max_swapfiles(void)
> +{
> +	unsigned int max_swapfile = 32;
nit never change, maybe
#define DEFAULT_MAX_SWAPFILE 32

> +	unsigned int swp_migration_num = 0, swp_hwpoison_num = 0, swp_device_num = 0, swp_pte_marker_num = 0;
nit: very long line maybe split (readability):
	unsigned int swp_migration_num = 0, swp_hwpoison_num = 0,
		     swp_device_num = 0, swp_pte_marker_num = 0;

But file is full of ugly lines, feel free to ignore.

> +	struct tst_kconfig_var migration_kconfig = TST_KCONFIG_INIT("CONFIG_MIGRATION");
> +	struct tst_kconfig_var memory_kconfig = TST_KCONFIG_INIT("CONFIG_MEMORY_FAILURE");
> +	struct tst_kconfig_var device_kconfig = TST_KCONFIG_INIT("CONFIG_DEVICE_PRIVATE");
> +	struct tst_kconfig_var marker_kconfig = TST_KCONFIG_INIT("CONFIG_PTE_MARKER");
Maybe shorten variable - remove "_kconfig" (obvious from use by .choice member).

> +
> +	tst_kconfig_read(&migration_kconfig, 1);
> +	tst_kconfig_read(&memory_kconfig, 1);
> +	tst_kconfig_read(&device_kconfig, 1);
> +	tst_kconfig_read(&marker_kconfig, 1);
> +
> +	if (migration_kconfig.choice == 'y') {
> +		if (tst_kvercmp(5, 19, 0) < 0)
> +			swp_migration_num = 2;
> +		else
> +			swp_migration_num = 3;
> +	}
> +
> +	if (memory_kconfig.choice == 'y')
> +		swp_hwpoison_num = 1;
> +
> +	if (device_kconfig.choice == 'y') {
> +		if (tst_kvercmp(4, 14, 0) >= 0)
> +			swp_device_num = 2;
> +		if (tst_kvercmp(5, 14, 0) >= 0)
> +			swp_device_num = 4;
> +	}
> +
> +	if (tst_kvercmp(6, 2, 0) >= 0) {
> +		swp_pte_marker_num = 1;
> +	} else {
> +		if (marker_kconfig.choice == 'y') {
> +			if (tst_kvercmp(5, 19, 0) >= 0)
> +				swp_pte_marker_num = 1;
> +		}

Maybe just:

	if ((marker_kconfig.choice == 'y' && tst_kvercmp(5, 19, 0) >= 0) ||
		tst_kvercmp(6, 2, 0) >= 0) {
		swp_pte_marker_num = 1;
	}

> +	}
> +
> +	return max_swapfile - swp_migration_num - swp_hwpoison_num - swp_device_num - swp_pte_marker_num;
> +}

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v4 2/7] libltpswap: alter tst_count_swaps api
  2024-02-20  7:42 ` [LTP] [PATCH v4 2/7] libltpswap: alter tst_count_swaps api Yang Xu via ltp
@ 2024-02-23  9:37   ` Petr Vorel
  0 siblings, 0 replies; 17+ messages in thread
From: Petr Vorel @ 2024-02-23  9:37 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Yang Xu,

> Like we count the ipc resource total, we can also add a
nit: s/ipc/IPC/

> similar api for swapfiles, so we can use it for swapon03 case.
nit: s/api/API/

Reviewed-by: Petr Vorel <pvorel@suse.cz>

...
> +
> +/*
> + * Get the used swapfiles number
> + */
> +int tst_count_swaps(void)
> +{
> +	FILE *fp;
> +	int used = -1;
> +	char c;
> +
> +	fp = fopen("/proc/swaps", "r");
> +	if (fp == NULL) {
> +		return -1;
Shouldn't we tst_brk(TCONF, "missing /proc/swaps, no swap support?");
This can happen if CONFIG_SWAP is not configured. We have in swapon03.c:

	if (access("/proc/swaps", F_OK))
		tst_brk(TCONF, "swap not supported by kernel");

Once this patchset is solved, I'll add to this code to a function
is_swap_supported in libswap.c, which will be used in all swapo{n,ff}* tests.
(That will be better than require CONFIG_SWAP kconfig.)
Then we can use SAFE_FOPEN() here.

> +	}
> +
> +	while ((c = fgetc(fp)) != EOF) {
> +		if (c == '\n')
> +			used++;
So you read number of lines in /proc/swaps, -1 because file has header.
> +	}
> +
> +	fclose(fp);
Maybe SAFE_FCLOSE() ?

> +	if (used < 0) {
> +		tst_brk(TBROK, "can't read /proc/swaps to get used swapfiles resource total");
> +	}
nit: no need for { }

Kind regards,
Petr

> +
> +	return used;
> +}

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v4 3/7] syscalls/swapon03: use tst_max_swapfiles() and GET_USED_SWAPFILES() api
  2024-02-20  7:42 ` [LTP] [PATCH v4 3/7] syscalls/swapon03: use tst_max_swapfiles() and GET_USED_SWAPFILES() api Yang Xu via ltp
  2024-02-21  9:37   ` Petr Vorel
@ 2024-02-23  9:48   ` Petr Vorel
  1 sibling, 0 replies; 17+ messages in thread
From: Petr Vorel @ 2024-02-23  9:48 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Yang Xu,

LGTM, few comments below.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

> @@ -19,7 +19,6 @@

>  #include "tst_test.h"
>  #include "lapi/syscalls.h"
> -#include "swaponoff.h"
+1

>  #include "libswap.h"

>  #define MNTPOINT	"mntpoint"
> @@ -104,47 +103,20 @@ static void verify_swapon(void)

Here is comment:
/*
 * Create 33 and activate 30 swapfiles.
 */
This would deserve to update, because we use tst_max_swapfiles(), rirght?
Something like: "Create max number of swap files + 1 and activate max number
swap files -2."
>  static int setup_swap(void)
>  {
>  	pid_t pid;
> -	int j, fd;
>  	int status;
> +	int j, max_swapfiles, used_swapfiles;
>  	int res = 0;
>  	char filename[FILENAME_MAX];
> -	char buf[BUFSIZ + 1];
> -
> -	/* Find out how many swapfiles (1 line per entry) already exist */
> -	swapfiles = 0;

>  	if (seteuid(0) < 0)
>  		tst_brk(TFAIL | TERRNO, "Failed to call seteuid");
nit: while at it, could you please use here SAFE_SETEUID(0) ?

> -	/* This includes the first (header) line */
> -	if ((fd = open("/proc/swaps", O_RDONLY)) == -1) {
> -		tst_brk(TFAIL | TERRNO,
> -			"Failed to find out existing number of swap files");
> -	}
> -	do {
> -		char *p = buf;
> -		res = read(fd, buf, BUFSIZ);
> -		if (res < 0) {
> -			tst_brk(TFAIL | TERRNO,
> -				 "Failed to find out existing number of swap files");
> -		}
> -		buf[res] = '\0';
> -		while ((p = strchr(p, '\n'))) {
> -			p++;
> -			swapfiles++;
> -		}
> -	} while (BUFSIZ <= res);
> -	close(fd);
> -	if (swapfiles)
> -		swapfiles--;	/* don't count the /proc/swaps header */
Explicitly mention "don't count the /proc/swaps header" would not hurt in the
previous commit where you use -1.
> -
> -	if (swapfiles < 0)
> -		tst_brk(TFAIL, "Failed to find existing number of swapfiles");
> -
>  	/* Determine how many more files are to be created */
> -	swapfiles = MAX_SWAPFILES - swapfiles;
> -	if (swapfiles > MAX_SWAPFILES)
> -		swapfiles = MAX_SWAPFILES;
> +	max_swapfiles = tst_max_swapfiles();
> +	used_swapfiles = tst_count_swaps();
> +	swapfiles = max_swapfiles - used_swapfiles;
Instead of used_swapfiles directly call tst_count_swaps() can be used.
> +	if (swapfiles > max_swapfiles)
> +		swapfiles = max_swapfiles;

Please add here blank line for readability.
>  	pid = SAFE_FORK();
>  	if (pid == 0) {
>  		/*create and turn on remaining swapfiles */

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v4 4/7] swaponoff.h: Remove useless header
  2024-02-20  7:42 ` [LTP] [PATCH v4 4/7] swaponoff.h: Remove useless header Yang Xu via ltp
@ 2024-02-23 10:15   ` Petr Vorel
  0 siblings, 0 replies; 17+ messages in thread
From: Petr Vorel @ 2024-02-23 10:15 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Yang Xu,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v4 5/7] swapon/Makefile: Remove useless section for MAX_SWAPFILES
  2024-02-20  7:42 ` [LTP] [PATCH v4 5/7] swapon/Makefile: Remove useless section for MAX_SWAPFILES Yang Xu via ltp
@ 2024-02-23 10:48   ` Petr Vorel
  0 siblings, 0 replies; 17+ messages in thread
From: Petr Vorel @ 2024-02-23 10:48 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Yang Xu,

> It seems this section doesn't affect anything, btw it is useless,
> so remove it.

...
> +++ b/testcases/kernel/syscalls/swapon/Makefile
> @@ -1,11 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  # Copyright (c) International Business Machines Corp., 2001

> -NEEDSPECIAL	:= $(shell echo MAX_SWAPFILES | $(CC) -E -xc -include linux/swap.h 2>/dev/null - | tail -n 1 | grep 32; echo $?)
> -ifneq ($(strip $(NEEDSPECIAL)),)
> -export CFLAGS	+= -DOLDER_DISTRO_RELEASE

Yeah, OLDER_DISTRO_RELEASE macro is not used at all.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v4 6/7] syscalls/swapon03: Simply this case
  2024-02-20  7:42 ` [LTP] [PATCH v4 6/7] syscalls/swapon03: Simply this case Yang Xu via ltp
@ 2024-02-23 11:48   ` Petr Vorel
  0 siblings, 0 replies; 17+ messages in thread
From: Petr Vorel @ 2024-02-23 11:48 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Yang Xu,

> By moving swapfile create stage from verify_swaopon and
> test EPERM error more accurate. Also use glibc wrapper by

> using swapon/swapoff instead of call syscall number directly
> because glibc/musl/binoic also support them since long time ago.
+1 thanks for checking.

FYI uClibc-ng only support with UCLIBC_LINUX_SPECIFIC config enabled, but it's
by default enabled. And I guess nobody runs uClibc-ng on Linux without this
enabled => really safe to depend on libc wrapper.

...
> +			TST_EXP_PASS_SILENT(swapon(filename, 0));
+1

>  		}
>  		exit(0);
>  	} else
> @@ -145,13 +61,40 @@ static int setup_swap(void)
>  	if (WEXITSTATUS(status))
>  		tst_brk(TFAIL, "Failed to setup swaps");
nit: s/swaps/swap files/

> -	/* Create all needed extra swapfiles for testing */
> -	for (j = 0; j < testfiles; j++)
> -		make_swapfile(swap_testfiles[j].filename, 10, 0);
> +	tst_res(TINFO, "Successfully created %d swapfiles", swapfiles);
nit: s/swapfiles/swap files/

> +	make_swapfile(TEST_FILE, 10, 0);

>  	return 0;
>  }

> +/*
> + * Check if the file is at /proc/swaps and remove it giving swapoff
> + */
> +static int check_and_swapoff(const char *filename)
> +{
> +	char cmd_buffer[256];
> +	int rc = -1;
> +
> +	if (snprintf(cmd_buffer, sizeof(cmd_buffer),
> +		"grep -q '%s.*file' /proc/swaps", filename) < 0) {
> +		tst_res(TWARN, "sprintf() failed to create the command string");
nit: we don't have SAFE_SNPRINTF() and don't even check snprintf() / sprintf()
return value. Shouldn't we add SAFE_SNPRINTF() which TBROK?
This can be handled later, thus I would here either use plain snprintf() or
tst_brk(TBROK).

if you add return -1 here, the following block does not have to be in else
(=> fewer indentation => text can be longer fewer string splits).

> +	} else {
> +		rc = 0;
> +		if (system(cmd_buffer) == 0) {
> +			/* now we need to swapoff the file */
> +			if (swapoff(filename) != 0) {

Why not single if?
		if (system(cmd_buffer) == 0) && swapoff(filename) != 0) {

> +				tst_res(TWARN, "Failed to turn off swap "
> +					"file. system reboot after "
> +					"execution of LTP test suite "
> +					"is recommended");
Then this string would not need to be split several times (bad for searching
with 'git grep'). Maybe shorten just to
"Failed to swapoff %", filename"
=> more important than suggest to reboot (which is obvious) is to point out
problematic swap file, which was kept on.


> +				rc = -1;
> +			}
> +		}
> +	}
> +
> +	return rc;
> +}
> +
>  /*
>   * Turn off all swapfiles previously turned on
>   */
> @@ -169,49 +112,17 @@ static int clean_swap(void)
Return code of clean_swap() is not used. How about to make it void?
>  		}
>  	}

> -	for (j = 0; j < testfiles; j++) {
> -		if (check_and_swapoff(swap_testfiles[j].filename) != 0) {
> -			tst_res(TWARN, "Failed to turn off swap file %s.",
> -				 swap_testfiles[j].filename);
> -			return -1;
> -		}
> +	if (check_and_swapoff("testfile") != 0) {
> +		tst_res(TWARN, "Failed to turn off swap file testfile");
We have the warning in the function, why also here?
> +		return -1;
>  	}

>  	return 0;
>  }

...
> +static void verify_swapon(void)
>  {
> +	TST_EXP_FAIL(swapon(TEST_FILE, 0), EPERM, "swapon(%s, 0)", TEST_FILE);
+1

Kind regards,
Petr
>  }

>  static void setup(void)
> @@ -220,6 +131,11 @@ static void setup(void)
>  		tst_brk(TCONF, "swap not supported by kernel");

>  	is_swap_supported(TEST_FILE);
> +
> +	if (setup_swap() < 0) {
> +		clean_swap();
> +		tst_brk(TBROK, "Setup failed, quitting the test");
> +	}

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v4 7/7] Add fallback for RHEL9
  2024-02-20  7:42 ` [LTP] [PATCH v4 7/7] Add fallback for RHEL9 Yang Xu via ltp
@ 2024-02-23 11:54   ` Petr Vorel
  0 siblings, 0 replies; 17+ messages in thread
From: Petr Vorel @ 2024-02-23 11:54 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Yang Xu,

> Since device number patch and pte number patch have been backported into
> RHEL9,  we should add fallback for this distro.

This is obviously for Li or Jan, but LGTM.
Acked-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api
  2024-02-20  7:42 [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Yang Xu via ltp
                   ` (6 preceding siblings ...)
  2024-02-23  9:04 ` [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Petr Vorel
@ 2024-02-23 11:56 ` Petr Vorel
  7 siblings, 0 replies; 17+ messages in thread
From: Petr Vorel @ 2024-02-23 11:56 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Yang Xu,

you introduced new formatting warnings, could you please fix it?

$ make check-libswap
libswap.c:279: WARNING: braces {} are not necessary for single statement blocks
libswap.c:289: WARNING: braces {} are not necessary for single statement blocks

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2024-02-23 11:56 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-20  7:42 [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Yang Xu via ltp
2024-02-20  7:42 ` [LTP] [PATCH v4 2/7] libltpswap: alter tst_count_swaps api Yang Xu via ltp
2024-02-23  9:37   ` Petr Vorel
2024-02-20  7:42 ` [LTP] [PATCH v4 3/7] syscalls/swapon03: use tst_max_swapfiles() and GET_USED_SWAPFILES() api Yang Xu via ltp
2024-02-21  9:37   ` Petr Vorel
2024-02-22  1:27     ` Yang Xu (Fujitsu) via ltp
2024-02-23  9:48   ` Petr Vorel
2024-02-20  7:42 ` [LTP] [PATCH v4 4/7] swaponoff.h: Remove useless header Yang Xu via ltp
2024-02-23 10:15   ` Petr Vorel
2024-02-20  7:42 ` [LTP] [PATCH v4 5/7] swapon/Makefile: Remove useless section for MAX_SWAPFILES Yang Xu via ltp
2024-02-23 10:48   ` Petr Vorel
2024-02-20  7:42 ` [LTP] [PATCH v4 6/7] syscalls/swapon03: Simply this case Yang Xu via ltp
2024-02-23 11:48   ` Petr Vorel
2024-02-20  7:42 ` [LTP] [PATCH v4 7/7] Add fallback for RHEL9 Yang Xu via ltp
2024-02-23 11:54   ` Petr Vorel
2024-02-23  9:04 ` [LTP] [PATCH v4 1/7] libltpswap: Add tst_max_swapfiles api Petr Vorel
2024-02-23 11:56 ` Petr Vorel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.