[LTP] [PATCH v2 0/5] swapon03: Try to swapon() as many files until it fails

[LTP] [PATCH v2 0/5] swapon03: Try to swapon() as many files until it fails

[Petr Vorel]

Hi,

Changes v1->v2:
* Test first swapon() call (Avinesh). For simplicity and -i working
  properly is swapon() tested twice.
* Use correct variable in TCONF message (Avinesh)
* Improve doc (Li)

3 new cleanup commits:
  swapon03: Don't create swap file twice
  swapon03: Cleanup
  swapon03: Remove grep dependency

Posting a patchset diff below (might be a bit more readable than my
explanation).

Link to v1:
https://lore.kernel.org/ltp/20251105154716.995786-1-pvorel@suse.cz/T/#t
https://patchwork.ozlabs.org/project/ltp/list/?series=481055&state=*

Kind regards,
Petr

Petr Vorel (5):
  swapon03: Don't create swap file twice
  swapon03: Cleanup
  swapon03: Try to swapon() as many files until it fails
  libswap: Remove now unused tst_max_swapfiles()
  swapon03: Remove grep dependency

 include/libswap.h                           |   7 --
 libs/swap/libswap.c                         |  64 ------------
 testcases/kernel/syscalls/swapon/swapon03.c | 105 ++++++++++++--------
 3 files changed, 62 insertions(+), 114 deletions(-)

diff --git testcases/kernel/syscalls/swapon/swapon03.c testcases/kernel/syscalls/swapon/swapon03.c
index 0027f874be..91d797871f 100644
--- testcases/kernel/syscalls/swapon/swapon03.c
+++ testcases/kernel/syscalls/swapon/swapon03.c
@@ -10,9 +10,8 @@
  * number of swap files are already in use.
  *
  * NOTE: test does not try to calculate MAX_SWAPFILES from the internal
- * kernel implementation (which is currently <23, 29> depending on kernel
- * configuration). Instead test exptect that at least 15 swap files minus
- * currently used swap can be created.
+ * kernel implementation, instead make sure few swaps were created before
+ * maximum was reached.
  */
 
 #include <stdio.h>
@@ -24,11 +23,18 @@
 #include "lapi/syscalls.h"
 #include "libswap.h"
 
+/*
+ * MAX_SWAPFILES from the internal kernel implementation is currently <23, 29>,
+ * depending on kernel configuration (see man swapon(2). Chose small enough
+ * value for future changes.
+ */
 #define NUM_SWAP_FILES 15
+
 #define MNTPOINT	"mntpoint"
-#define TEST_FILE	MNTPOINT"/testswap"
+#define TEST_FILE	MNTPOINT "/LTP_" __FILE__ "_testswap"
 
 static int *swapfiles;
+static char *tmpdir;
 
 static void setup_swap(void)
 {
@@ -41,10 +47,8 @@ static void setup_swap(void)
 	used_swapfiles = tst_count_swaps();
 	expected_swapfiles = NUM_SWAP_FILES - used_swapfiles;
 
-	if (expected_swapfiles < 0) {
-		tst_brk(TCONF, "Warning: too many used swap files (%d)",
-			expected_swapfiles);
-	}
+	if (expected_swapfiles < 0)
+		tst_brk(TCONF, "too many used swap files (%d)", used_swapfiles);
 
 	pid = SAFE_FORK();
 	if (pid == 0) {
@@ -53,9 +57,12 @@ static void setup_swap(void)
 			snprintf(filename, sizeof(filename), "%s%02d", TEST_FILE, *swapfiles);
 			MAKE_SMALL_SWAPFILE(filename);
 
-			/* Quit on a first swap file over max */
-			if (swapon(filename, 0) == -1)
+			/* Quit on a first swap file over max, check for EPERM */
+			if (swapon(filename, 0) == -1) {
+				if (errno != EPERM)
+					tst_res(TFAIL | TERRNO, "swapon(%s, 0)", filename);
 				break;
+			}
 			(*swapfiles)++;
 		}
 		exit(0);
@@ -79,12 +86,14 @@ static void setup_swap(void)
  */
 static void check_and_swapoff(const char *filename)
 {
-	char cmd_buffer[256];
+	char buf[256];
+	int foo;
 
-	snprintf(cmd_buffer, sizeof(cmd_buffer), "grep -q '%s.*file' /proc/swaps", filename);
-
-	if (system(cmd_buffer) == 0 && swapoff(filename) != 0)
-		tst_res(TWARN, "Failed to swapoff %s", filename);
+	snprintf(buf, sizeof(buf), "%s/%s %%*s %%*s %%*s %%s", tmpdir, filename);
+	if (!FILE_LINES_SCANF("/proc/swaps", buf, &foo)) {
+		if (swapoff(filename) != 0)
+			tst_res(TWARN | TERRNO, "swapoff(%s) failed", filename);
+	}
 }
 
 /*
@@ -115,6 +124,8 @@ static void setup(void)
 
 	is_swap_supported(TEST_FILE);
 
+	tmpdir = tst_tmpdir_path();
+
 	swapfiles = SAFE_MMAP(NULL, sizeof(*swapfiles), PROT_READ | PROT_WRITE,
 			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
 	*swapfiles = 0;

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

[LTP] [PATCH v2 1/5] swapon03: Don't create swap file twice

[Petr Vorel]

MAKE_SMALL_SWAPFILE(TEST_FILE) call is not needed, because the file is
created by is_swap_supported(TEST_FILE) call.

Fixes: c240726a62 ("libswap: Use {SAFE_,}MAKE_MINIMAL_SWAPFILE()")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v2.

 testcases/kernel/syscalls/swapon/swapon03.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
index d6445d5fc4..01a3b6d8ac 100644
--- a/testcases/kernel/syscalls/swapon/swapon03.c
+++ b/testcases/kernel/syscalls/swapon/swapon03.c
@@ -61,7 +61,6 @@ static int setup_swap(void)
 		tst_brk(TFAIL, "Failed to setup swap files");
 
 	tst_res(TINFO, "Successfully created %d swap files", swapfiles);
-	MAKE_SMALL_SWAPFILE(TEST_FILE);
 
 	return 0;
 }
-- 
2.51.0


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

[LTP] [PATCH v2 2/5] swapon03: Cleanup

[Petr Vorel]

- Remove unused return code in check_and_swapoff() and setup_swap(). The
  purpose was to run cleanup() in the end of the setup() if creating
  swap fails, but return code is always 0. Also cleanup() should be run
  when test exits with tst_brk() anyway.
- Change return code from TFAIL to TBROK.
- Add missing brackets.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
The same as in v1.
I can split this if you like.

 testcases/kernel/syscalls/swapon/swapon03.c | 23 +++++++--------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
index 01a3b6d8ac..d9822c01ef 100644
--- a/testcases/kernel/syscalls/swapon/swapon03.c
+++ b/testcases/kernel/syscalls/swapon/swapon03.c
@@ -25,7 +25,7 @@
 
 static int swapfiles;
 
-static int setup_swap(void)
+static void setup_swap(void)
 {
 	pid_t pid;
 	int status;
@@ -54,33 +54,27 @@ static int setup_swap(void)
 			TST_EXP_PASS_SILENT(swapon(filename, 0));
 		}
 		exit(0);
-	} else
+	} else {
 		waitpid(pid, &status, 0);
+	}
 
 	if (WEXITSTATUS(status))
-		tst_brk(TFAIL, "Failed to setup swap files");
+		tst_brk(TBROK, "Failed to setup swap files");
 
 	tst_res(TINFO, "Successfully created %d swap files", swapfiles);
-
-	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 check_and_swapoff(const char *filename)
 {
 	char cmd_buffer[256];
-	int rc = -1;
 
 	snprintf(cmd_buffer, sizeof(cmd_buffer), "grep -q '%s.*file' /proc/swaps", filename);
 
-	if (system(cmd_buffer) == 0 && swapoff(filename) != 0) {
+	if (system(cmd_buffer) == 0 && swapoff(filename) != 0)
 		tst_res(TWARN, "Failed to swapoff %s", filename);
-		rc = -1;
-	}
-
-	return rc;
 }
 
 /*
@@ -110,10 +104,7 @@ 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");
-	}
+	setup_swap();
 }
 
 static void cleanup(void)
-- 
2.51.0


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

[LTP] [PATCH v2 3/5] swapon03: Try to swapon() as many files until it fails

[Petr Vorel]

Previously tst_max_swapfiles() had fine tuning for a specific kernel
version which was fragile due various backports in enterprise kernels.

Let's try to create and use as many swap files until swapon() fails.
Then check for expected EPERM.

Also test swapon() 2x, first in the setup() when first failure happen,
then in the test function (easier than propagate errno from fork to the
main function).

Suggested-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v1->v2:
* Test first swapon() call (Avinesh). For simplicity and -i working
  properly is swapon() tested twice.
* Use correct variable in TCONF message (Avinesh)
* Improve doc (Li)

 testcases/kernel/syscalls/swapon/swapon03.c | 68 ++++++++++++++-------
 1 file changed, 46 insertions(+), 22 deletions(-)

diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
index d9822c01ef..a8e0cbcdc6 100644
--- a/testcases/kernel/syscalls/swapon/swapon03.c
+++ b/testcases/kernel/syscalls/swapon/swapon03.c
@@ -1,14 +1,17 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * Copyright (c) Linux Test Project, 2009-2025
  * Copyright (c) International Business Machines Corp., 2007
  * Created by <rsalveti@linux.vnet.ibm.com>
- *
  */
 
 /*\
- * This test case checks whether swapon(2) system call returns:
+ * Test checks whether :man2:`swapon` system call returns EPERM when the maximum
+ * number of swap files are already in use.
  *
- *  - EPERM when there are more than MAX_SWAPFILES already in use.
+ * NOTE: test does not try to calculate MAX_SWAPFILES from the internal
+ * kernel implementation, instead make sure few swaps were created before
+ * maximum was reached.
  */
 
 #include <stdio.h>
@@ -20,48 +23,61 @@
 #include "lapi/syscalls.h"
 #include "libswap.h"
 
+/*
+ * MAX_SWAPFILES from the internal kernel implementation is currently <23, 29>,
+ * depending on kernel configuration (see man swapon(2). Chose small enough
+ * value for future changes.
+ */
+#define NUM_SWAP_FILES 15
+
 #define MNTPOINT	"mntpoint"
 #define TEST_FILE	MNTPOINT"/testswap"
 
-static int swapfiles;
+static int *swapfiles;
 
 static void setup_swap(void)
 {
 	pid_t pid;
-	int status;
-	int j, max_swapfiles, used_swapfiles;
+	int status, used_swapfiles, expected_swapfiles;
 	char filename[FILENAME_MAX];
 
 	SAFE_SETEUID(0);
 
-	/* Determine how many more files are to be created */
-	max_swapfiles = tst_max_swapfiles();
 	used_swapfiles = tst_count_swaps();
-	swapfiles = max_swapfiles - used_swapfiles;
-	if (swapfiles > max_swapfiles)
-		swapfiles = max_swapfiles;
+	expected_swapfiles = NUM_SWAP_FILES - used_swapfiles;
+
+	if (expected_swapfiles < 0)
+		tst_brk(TCONF, "too many used swap files (%d)", used_swapfiles);
 
 	pid = SAFE_FORK();
 	if (pid == 0) {
-		/*create and turn on remaining swapfiles */
-		for (j = 0; j < swapfiles; j++) {
-
+		while (true) {
 			/* Create the swapfile */
-			snprintf(filename, sizeof(filename), "%s%02d", TEST_FILE, j + 2);
+			snprintf(filename, sizeof(filename), "%s%02d", TEST_FILE, *swapfiles);
 			MAKE_SMALL_SWAPFILE(filename);
 
-			/* turn on the swap file */
-			TST_EXP_PASS_SILENT(swapon(filename, 0));
+			/* Quit on a first swap file over max, check for EPERM */
+			if (swapon(filename, 0) == -1) {
+				if (errno != EPERM)
+					tst_res(TFAIL | TERRNO, "swapon(%s, 0)", filename);
+				break;
+			}
+			(*swapfiles)++;
 		}
 		exit(0);
 	} else {
 		waitpid(pid, &status, 0);
 	}
 
-	if (WEXITSTATUS(status))
+	if (WEXITSTATUS(status) || *swapfiles == 0)
 		tst_brk(TBROK, "Failed to setup swap files");
 
-	tst_res(TINFO, "Successfully created %d swap files", swapfiles);
+	if (*swapfiles < expected_swapfiles) {
+		tst_res(TWARN, "Successfully created only %d swap files (>= %d expected)",
+			*swapfiles, expected_swapfiles);
+	} else {
+		tst_res(TINFO, "Successfully created %d swap files", *swapfiles);
+	}
 }
 
 /*
@@ -85,8 +101,8 @@ static void clean_swap(void)
 	int j;
 	char filename[FILENAME_MAX];
 
-	for (j = 0; j < swapfiles; j++) {
-		snprintf(filename, sizeof(filename), "%s%02d", TEST_FILE, j + 2);
+	for (j = 0; j < *swapfiles; j++) {
+		snprintf(filename, sizeof(filename), "%s%02d", TEST_FILE, j);
 		check_and_swapoff(filename);
 	}
 
@@ -104,12 +120,20 @@ static void setup(void)
 		tst_brk(TCONF, "swap not supported by kernel");
 
 	is_swap_supported(TEST_FILE);
+
+	swapfiles = SAFE_MMAP(NULL, sizeof(*swapfiles), PROT_READ | PROT_WRITE,
+			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+	*swapfiles = 0;
+
 	setup_swap();
 }
 
 static void cleanup(void)
 {
-	clean_swap();
+	if (swapfiles) {
+		clean_swap();
+		SAFE_MUNMAP(swapfiles, sizeof(*swapfiles));
+	}
 }
 
 static struct tst_test test = {
-- 
2.51.0


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

[LTP] [PATCH v2 4/5] libswap: Remove now unused tst_max_swapfiles()

[Petr Vorel]

The only use in swapon03 was removed in the previous commit.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
The same as in v1.

 include/libswap.h   |  7 -----
 libs/swap/libswap.c | 64 ---------------------------------------------
 2 files changed, 71 deletions(-)

diff --git a/include/libswap.h b/include/libswap.h
index 6904e8f45b..b22b992eeb 100644
--- a/include/libswap.h
+++ b/include/libswap.h
@@ -109,13 +109,6 @@ int make_swapfile(const char *file, const int lineno,
  */
 bool is_swap_supported(const char *filename);
 
-/**
- * tst_max_swapfiles() - Get kernel constant MAX_SWAPFILES value.
- *
- * Return: MAX_SWAPFILES value.
- */
-int tst_max_swapfiles(void);
-
 /**
  * tst_count_swaps() - Get the used swapfiles number.
  *
diff --git a/libs/swap/libswap.c b/libs/swap/libswap.c
index 734cd2612d..3eb589cdd7 100644
--- a/libs/swap/libswap.c
+++ b/libs/swap/libswap.c
@@ -239,70 +239,6 @@ bool is_swap_supported(const char *filename)
 	return true;
 }
 
-int tst_max_swapfiles(void)
-{
-	unsigned int swp_migration_num = 0, swp_hwpoison_num = 0,
-		     swp_device_num = 0, swp_pte_marker_num = 0,
-		     swp_swapin_error_num = 0;
-	struct tst_kconfig_var migration = TST_KCONFIG_INIT("CONFIG_MIGRATION");
-	struct tst_kconfig_var memory = TST_KCONFIG_INIT("CONFIG_MEMORY_FAILURE");
-	struct tst_kconfig_var device = TST_KCONFIG_INIT("CONFIG_DEVICE_PRIVATE");
-	struct tst_kconfig_var marker = TST_KCONFIG_INIT("CONFIG_PTE_MARKER");
-	struct tst_kern_exv kvers_marker_migration[] = {
-		/* RHEL9 kernel has patch 6c287605f and 679d10331 since 5.14.0-179 */
-		{ "RHEL9", "5.14.0-179" },
-		{ NULL, NULL},
-	};
-
-	struct tst_kern_exv kvers_marker_migration2[] = {
-		/* RHEL9 kernel has patch ca92ea3dc5a since 5.14.0-441 */
-		{ "RHEL9", "5.14.0-441" },
-		{ NULL, NULL},
-	};
-
-	struct tst_kern_exv kvers_device[] = {
-		/* SLES12-SP4 has patch 5042db43cc26 since 4.12.14-5.5 */
-		{ "SLES", "4.12.14-5.5" },
-		{ NULL, NULL},
-	};
-
-	tst_kconfig_read(&migration, 1);
-	tst_kconfig_read(&memory, 1);
-	tst_kconfig_read(&device, 1);
-	tst_kconfig_read(&marker, 1);
-
-	if (migration.choice == 'y') {
-		if (tst_kvercmp2(5, 19, 0, kvers_marker_migration) < 0)
-			swp_migration_num = 2;
-		else
-			swp_migration_num = 3;
-	}
-
-	if (memory.choice == 'y')
-		swp_hwpoison_num = 1;
-
-	if (device.choice == 'y') {
-		if (tst_kvercmp2(4, 14, 0, kvers_device) >= 0)
-			swp_device_num = 2;
-		if (tst_kvercmp(5, 14, 0) >= 0)
-			swp_device_num = 4;
-		if (tst_kvercmp(6, 15, 0) >= 0)
-			swp_device_num = 3;
-	}
-
-	if ((marker.choice == 'y' &&
-	     tst_kvercmp2(5, 19, 0, kvers_marker_migration) >= 0)
-	    || tst_kvercmp2(6, 2, 0, kvers_marker_migration2) >= 0) {
-		swp_pte_marker_num = 1;
-	}
-
-	if ((tst_kvercmp(5, 19, 0) >= 0) && (tst_kvercmp(6, 2, 0) < 0))
-		swp_swapin_error_num = 1;
-
-	return DEFAULT_MAX_SWAPFILE - swp_migration_num - swp_hwpoison_num
-		- swp_device_num - swp_pte_marker_num - swp_swapin_error_num;
-}
-
 int tst_count_swaps(void)
 {
 	FILE *fp;
-- 
2.51.0


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

[LTP] [PATCH v2 5/5] swapon03: Remove grep dependency

[Petr Vorel]

Instead of relying on 'grep' run via system() parse output with C
implementation (faster, no dependency).

Also rename swap file to make sure it's more obvious that it was created
by LTP.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v2.

 testcases/kernel/syscalls/swapon/swapon03.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
index a8e0cbcdc6..91d797871f 100644
--- a/testcases/kernel/syscalls/swapon/swapon03.c
+++ b/testcases/kernel/syscalls/swapon/swapon03.c
@@ -31,9 +31,10 @@
 #define NUM_SWAP_FILES 15
 
 #define MNTPOINT	"mntpoint"
-#define TEST_FILE	MNTPOINT"/testswap"
+#define TEST_FILE	MNTPOINT "/LTP_" __FILE__ "_testswap"
 
 static int *swapfiles;
+static char *tmpdir;
 
 static void setup_swap(void)
 {
@@ -85,12 +86,14 @@ static void setup_swap(void)
  */
 static void check_and_swapoff(const char *filename)
 {
-	char cmd_buffer[256];
+	char buf[256];
+	int foo;
 
-	snprintf(cmd_buffer, sizeof(cmd_buffer), "grep -q '%s.*file' /proc/swaps", filename);
-
-	if (system(cmd_buffer) == 0 && swapoff(filename) != 0)
-		tst_res(TWARN, "Failed to swapoff %s", filename);
+	snprintf(buf, sizeof(buf), "%s/%s %%*s %%*s %%*s %%s", tmpdir, filename);
+	if (!FILE_LINES_SCANF("/proc/swaps", buf, &foo)) {
+		if (swapoff(filename) != 0)
+			tst_res(TWARN | TERRNO, "swapoff(%s) failed", filename);
+	}
 }
 
 /*
@@ -121,6 +124,8 @@ static void setup(void)
 
 	is_swap_supported(TEST_FILE);
 
+	tmpdir = tst_tmpdir_path();
+
 	swapfiles = SAFE_MMAP(NULL, sizeof(*swapfiles), PROT_READ | PROT_WRITE,
 			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
 	*swapfiles = 0;
-- 
2.51.0


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

Re: [LTP] [PATCH v2 3/5] swapon03: Try to swapon() as many files until it fails

[Li Wang via ltp]

Hi Petr,

For the patch set:
Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v2 5/5] swapon03: Remove grep dependency

[Avinesh Kumar]

On Thursday, November 6, 2025 5:35:00 PM CET Petr Vorel wrote:
> Instead of relying on 'grep' run via system() parse output with C
> implementation (faster, no dependency).
> 
> Also rename swap file to make sure it's more obvious that it was created
> by LTP.
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>

Hi Petr,

Reviewed-by: Avinesh Kumar <akumar@suse.de>

for all patches in the series.


Thanks,
Avinesh





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

Re: [LTP] [PATCH v2 1/5] swapon03: Don't create swap file twice

[Cyril Hrubis]

Hi!
> MAKE_SMALL_SWAPFILE(TEST_FILE) call is not needed, because the file is
> created by is_swap_supported(TEST_FILE) call.

I find it a bit ugly to depend on a leftover from is_swap_supported() to
create the final swapfile. Maybe it would be cleaner to pass a path to
an already created swapfile to the is_swap_supported() function instead
so that the tests would create the file explicitly.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 2/5] swapon03: Cleanup

[Cyril Hrubis]

Hi!
> - Remove unused return code in check_and_swapoff() and setup_swap(). The
>   purpose was to run cleanup() in the end of the setup() if creating
>   swap fails, but return code is always 0. Also cleanup() should be run
>   when test exits with tst_brk() anyway.
> - Change return code from TFAIL to TBROK.
> - Add missing brackets.
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> The same as in v1.
> I can split this if you like.
> 
>  testcases/kernel/syscalls/swapon/swapon03.c | 23 +++++++--------------
>  1 file changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
> index 01a3b6d8ac..d9822c01ef 100644
> --- a/testcases/kernel/syscalls/swapon/swapon03.c
> +++ b/testcases/kernel/syscalls/swapon/swapon03.c
> @@ -25,7 +25,7 @@
>  
>  static int swapfiles;
>  
> -static int setup_swap(void)
> +static void setup_swap(void)
>  {
>  	pid_t pid;
>  	int status;
> @@ -54,33 +54,27 @@ static int setup_swap(void)
>  			TST_EXP_PASS_SILENT(swapon(filename, 0));
>  		}
>  		exit(0);
> -	} else
> +	} else {
>  		waitpid(pid, &status, 0);
> +	}
>  
>  	if (WEXITSTATUS(status))
> -		tst_brk(TFAIL, "Failed to setup swap files");
> +		tst_brk(TBROK, "Failed to setup swap files");

I wonder why do we run this code in a child process to begin with? This
is probably some leftover from the conversion to the new library. Among
other things this masks proper results propagation from the
MAKE_SMALL_SWAPFILE() because there is at least one tst_brk(TCONF, "")
in there that will be converted to TBROK here.

I guess that we want to remove the fork() here and we want SAFE_SWAPON()
instead of the TST_EXP_PASS_SILENT() so that we do tst_brk() if we fail
to setup the swapfile too. The TST_EXP_PASS_SILENT() does not end the
test on a failure, which is what we want here instead.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 3/5] swapon03: Try to swapon() as many files until it fails

[Cyril Hrubis]

Hi!
>  	is_swap_supported(TEST_FILE);
> +
> +	swapfiles = SAFE_MMAP(NULL, sizeof(*swapfiles), PROT_READ | PROT_WRITE,
> +			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> +	*swapfiles = 0;
> +
>  	setup_swap();
>  }
>  
>  static void cleanup(void)
>  {
> -	clean_swap();
> +	if (swapfiles) {
> +		clean_swap();
> +		SAFE_MUNMAP(swapfiles, sizeof(*swapfiles));
> +	}
>  }

This gets complicated for no good reason since we run the setup code in the
child.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 1/5] swapon03: Don't create swap file twice

[Petr Vorel]

> Hi!
> > MAKE_SMALL_SWAPFILE(TEST_FILE) call is not needed, because the file is
> > created by is_swap_supported(TEST_FILE) call.

> I find it a bit ugly to depend on a leftover from is_swap_supported() to
> create the final swapfile. Maybe it would be cleaner to pass a path to
> an already created swapfile to the is_swap_supported() function instead
> so that the tests would create the file explicitly.

Sounds indeed better. I'll add it to my TODO list. Patchset could be rebased to
not include this for now.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 2/5] swapon03: Cleanup

[Petr Vorel]

> Hi!
> > - Remove unused return code in check_and_swapoff() and setup_swap(). The
> >   purpose was to run cleanup() in the end of the setup() if creating
> >   swap fails, but return code is always 0. Also cleanup() should be run
> >   when test exits with tst_brk() anyway.
> > - Change return code from TFAIL to TBROK.
> > - Add missing brackets.

> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > The same as in v1.
> > I can split this if you like.

> >  testcases/kernel/syscalls/swapon/swapon03.c | 23 +++++++--------------
> >  1 file changed, 7 insertions(+), 16 deletions(-)

> > diff --git a/testcases/kernel/syscalls/swapon/swapon03.c b/testcases/kernel/syscalls/swapon/swapon03.c
> > index 01a3b6d8ac..d9822c01ef 100644
> > --- a/testcases/kernel/syscalls/swapon/swapon03.c
> > +++ b/testcases/kernel/syscalls/swapon/swapon03.c
> > @@ -25,7 +25,7 @@

> >  static int swapfiles;

> > -static int setup_swap(void)
> > +static void setup_swap(void)
> >  {
> >  	pid_t pid;
> >  	int status;
> > @@ -54,33 +54,27 @@ static int setup_swap(void)
> >  			TST_EXP_PASS_SILENT(swapon(filename, 0));
> >  		}
> >  		exit(0);
> > -	} else
> > +	} else {
> >  		waitpid(pid, &status, 0);
> > +	}

> >  	if (WEXITSTATUS(status))
> > -		tst_brk(TFAIL, "Failed to setup swap files");
> > +		tst_brk(TBROK, "Failed to setup swap files");

> I wonder why do we run this code in a child process to begin with? This

Yes, although this is a general recommendation I was thinking about removing it
as well as.  and first intended to remove it. But then I thought it was easier
to handle 'mkswap' binary failure (run in make_swapfile() which is called via
MAKE_SMALL_SWAPFILE()).

Other reason was that I did already quite a lot of cleanup, thus extending it
even more means more iterations. But sure, I can fix it (and postpone fix in the
first commit to later).

> is probably some leftover from the conversion to the new library. Among
> other things this masks proper results propagation from the
> MAKE_SMALL_SWAPFILE() because there is at least one tst_brk(TCONF, "")
> in there that will be converted to TBROK here.

Ah, You mean:
tst_brk_(file, lineno, TCONF, "Insufficient disk space to create swap file");
Indeed, I overlooked that one. It's always useful to look into the tests we
already converted some time ago :).

> I guess that we want to remove the fork() here and we want SAFE_SWAPON()
> instead of the TST_EXP_PASS_SILENT() so that we do tst_brk() if we fail

FYI we don't have SAFE_SWAPON() but this part of is_swap_supported() could be
factored out as SAFE_SWAPON():

	TEST(tst_syscall(__NR_swapon, filename, 0));
	if (TST_RET == -1) {
		if (errno == EPERM) {
			tst_brk(TCONF, "Permission denied for swapon()");
		} else if (errno == EINVAL && fi_contiguous == 0 && sw_support == 0) {
			tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
		} else {
			tst_res(TFAIL | TTERRNO, "swapon() on %s failed", fstype);
			return false;
		}
	}

Also we could use SAFE_MAKE_SMALL_SWAPFILE() instead of MAKE_SMALL_SWAPFILE(),
to catch few errors properly (this is what I did in 3rd commit).

> to setup the swapfile too. The TST_EXP_PASS_SILENT() does not end the
> test on a failure, which is what we want here instead.

Yes, that's the problem now. It could be fixed by calling tst_brk()
instead of TST_EXP_PASS_SILENT() or really create SAFE_SWAPON().
But FYI I'm not sure if there would be use for SAFE_SWAPON(), because I changed
this part in the 3rd commit (this is the main change which expects swapon() to
fail):

-			/* turn on the swap file */
-			TST_EXP_PASS_SILENT(swapon(filename, 0));
+			/* Quit on a first swap file over max, check for EPERM */
+			if (swapon(filename, 0) == -1) {
+				if (errno != EPERM)
+					tst_res(TFAIL | TERRNO, "swapon(%s, 0)", filename);
+				break;
+			}
+			(*swapfiles)++;

and swapon0[12].c swapoff*.c don't need it (they call *MAKE_SWAPFILE*()
functions.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 3/5] swapon03: Try to swapon() as many files until it fails

[Petr Vorel]

Hi Cyril,

> Hi!
> >  	is_swap_supported(TEST_FILE);
> > +
> > +	swapfiles = SAFE_MMAP(NULL, sizeof(*swapfiles), PROT_READ | PROT_WRITE,
> > +			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> > +	*swapfiles = 0;
> > +
> >  	setup_swap();
> >  }

> >  static void cleanup(void)
> >  {
> > -	clean_swap();
> > +	if (swapfiles) {
> > +		clean_swap();
> > +		SAFE_MUNMAP(swapfiles, sizeof(*swapfiles));
> > +	}
> >  }

First, thank you for your review!

> This gets complicated for no good reason since we run the setup code in the
> child.

If swapfiles is not mapped for some reason (can happen or "*swapfiles = 0" is
not run (that should not happen), then using *swapfiles leads to segfault.

*swapfiles is being used not only in SAFE_MUNMAP() here in cleanup(), but
also in clean_swap().

Also, this saves trying to deactivate swap files if they weren't activated.

And, unless "swapon03: Remove grep dependency" (the last commit) is accepted,
there is a corner case I found during debugging:

# ./swapon03 # quit early with ctrl+C => some swap partitions are left not being swapped off
...
Sending SIGKILL to test process...
tst_test.c:1909: TINFO: Killed the leftover descendant processes
tst_test.c:1918: TINFO: If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1
tst_test.c:1920: TBROK: Test killed! (timeout?)

Summary:
passed   0
failed   0
broken   1
skipped  0
warnings 0
tst_device.c:417: TINFO: umount('mntpoint') failed with EBUSY, try  1...
tst_device.c:421: TINFO: Likely gvfsd-trash is probing newly mounted fs, kill it to speed up tests.
tst_device.c:417: TINFO: umount('mntpoint') failed with EBUSY, try  2...
tst_device.c:417: TINFO: umount('mntpoint') failed with EBUSY, try  3...
tst_device.c:417: TINFO: umount('mntpoint') failed with EBUSY, try  4...
tst_device.c:417: TINFO: umount('mntpoint') failed with EBUSY, try  5...
tst_device.c:417: TINFO: umount('mntpoint') failed with EBUSY, try  6...
tst_device.c:417: TINFO: umount('mntpoint') failed with EBUSY, try  7...
tst_device.c:417: TINFO: umount('mntpoint') failed with EBUSY, try  8...
tst_device.c:417: TINFO: umount('mntpoint') failed with EBUSY, try  9...
tst_device.c:417: TINFO: umount('mntpoint') failed with EBUSY, try 10...

Then following run tries to swapoff() partition left from previous run.

# ./swapon03
...
swapon03.c:80: TWARN: Failed to swapoff mntpoint/testswap

But the last commit fixes this problem.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 3/5] swapon03: Try to swapon() as many files until it fails

[Petr Vorel]

> Hi Cyril,

> > Hi!
> > >  	is_swap_supported(TEST_FILE);
> > > +
> > > +	swapfiles = SAFE_MMAP(NULL, sizeof(*swapfiles), PROT_READ | PROT_WRITE,
> > > +			MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> > > +	*swapfiles = 0;
> > > +
> > >  	setup_swap();
> > >  }

> > >  static void cleanup(void)
> > >  {
> > > -	clean_swap();
> > > +	if (swapfiles) {
> > > +		clean_swap();
> > > +		SAFE_MUNMAP(swapfiles, sizeof(*swapfiles));
> > > +	}
> > >  }

> First, thank you for your review!

> > This gets complicated for no good reason since we run the setup code in the
> > child.

> If swapfiles is not mapped for some reason (can happen or "*swapfiles = 0" is
> not run (that should not happen), then using *swapfiles leads to segfault.

... but I'm not happy with the resulting code either. Therefore I'll send
another version where I cleanup unneeded fork() leftover from previous
implementation as you suggested [1]. That will simplify test a bit
(and mmap() will not be needed).

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/aRM0Z-IlZSLdB5ho@yuki.lan/


> *swapfiles is being used not only in SAFE_MUNMAP() here in cleanup(), but
> also in clean_swap().

> Also, this saves trying to deactivate swap files if they weren't activated.

> And, unless "swapon03: Remove grep dependency" (the last commit) is accepted,
> there is a corner case I found during debugging:

...

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 5/5] swapon03: Remove grep dependency

[Petr Vorel]

Hi all,

note for myself.

This change requires to normalize TMPDIR via realpath() otherwise grep will
fail. Either in the test or in the library [1] (I'll submit it separately, no
need to be part of Wei's cleanup).

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/20251114085856.GA43654@pevik/

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

Re: [LTP] [PATCH v2 5/5] swapon03: Remove grep dependency

[Cyril Hrubis]

Hi!
> This change requires to normalize TMPDIR via realpath() otherwise grep will
> fail. Either in the test or in the library [1] (I'll submit it separately, no
> need to be part of Wei's cleanup).

I was wondering if we can avoid matching the full path. Maybe we can
just do strstr() on the line from the /proc/mounts matching the filename
before we attempt to do the scanf().

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 5/5] swapon03: Remove grep dependency

[Petr Vorel]

> Hi!
> > This change requires to normalize TMPDIR via realpath() otherwise grep will
> > fail. Either in the test or in the library [1] (I'll submit it separately, no
> > need to be part of Wei's cleanup).

> I was wondering if we can avoid matching the full path. Maybe we can
> just do strstr() on the line from the /proc/mounts matching the filename
> before we attempt to do the scanf().

Do you mean to use full swap file path from /proc/mounts, i.e.
/tmp/LTP_swaNzeMJr/mntpoint/testswap, /tmp/LTP_swamEVVAc/mntpoint/testswap03,
... (example from run which created /tmp/LTP_swamEVVAc with all swap files,
cleanup interrupted by ctrl+C and following one which created /tmp/LTP_swaNzeMJr
single swap file only):

# cat /proc/swaps
Filename				Type		Size		Used		Priority
/dev/vda3                               partition	2098152		0		-2
/tmp/LTP_swaNzeMJr/mntpoint/testswap    file		1004		0		-25
/tmp/LTP_swamEVVAc/mntpoint/testswap03  file		1000		0		-3
/tmp/LTP_swamEVVAc/mntpoint/testswap04  file		1000		0		-4
/tmp/LTP_swamEVVAc/mntpoint/testswap05  file		1000		0		-5
/tmp/LTP_swamEVVAc/mntpoint/testswap06  file		1000		0		-6
/tmp/LTP_swamEVVAc/mntpoint/testswap07  file		996		0		-7
/tmp/LTP_swamEVVAc/mntpoint/testswap08  file		996		0		-8
/tmp/LTP_swamEVVAc/mntpoint/testswap09  file		996		0		-9
/tmp/LTP_swamEVVAc/mntpoint/testswap10  file		1000		0		-10
/tmp/LTP_swamEVVAc/mntpoint/testswap11  file		1000		0		-11
/tmp/LTP_swamEVVAc/mntpoint/testswap12  file		1000		0		-12
/tmp/LTP_swamEVVAc/mntpoint/testswap13  file		1000		0		-13
/tmp/LTP_swamEVVAc/mntpoint/testswap14  file		1000		0		-14
/tmp/LTP_swamEVVAc/mntpoint/testswap15  file		1000		0		-15
/tmp/LTP_swamEVVAc/mntpoint/testswap16  file		996		0		-16
/tmp/LTP_swamEVVAc/mntpoint/testswap17  file		996		0		-17
/tmp/LTP_swamEVVAc/mntpoint/testswap18  file		996		0		-18
/tmp/LTP_swamEVVAc/mntpoint/testswap19  file		1000		0		-19
/tmp/LTP_swamEVVAc/mntpoint/testswap20  file		1000		0		-20
/tmp/LTP_swamEVVAc/mntpoint/testswap21  file		1000		0		-21
/tmp/LTP_swamEVVAc/mntpoint/testswap22  file		1000		0		-22
/tmp/LTP_swamEVVAc/mntpoint/testswap23  file		1000		0		-23
/tmp/LTP_swamEVVAc/mntpoint/testswap    file		1000		0		-24

This way would swapoff all LTP swaps including possible previous ones.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 5/5] swapon03: Remove grep dependency

[Cyril Hrubis]

Hi!
> Do you mean to use full swap file path from /proc/mounts, i.e.
> /tmp/LTP_swaNzeMJr/mntpoint/testswap, /tmp/LTP_swamEVVAc/mntpoint/testswap03,
> ... (example from run which created /tmp/LTP_swamEVVAc with all swap files,
> cleanup interrupted by ctrl+C and following one which created /tmp/LTP_swaNzeMJr
> single swap file only):

If we make the swapfile names unique enough we can match just that
instead. I think one of the patches did just that by including the test
name in the swap filenames. If we want to be extra sure we can take last
directory component of the test temporary directory as well. What I'm
trying to point out is that the $TMPDIR part of the test temporary
directory is not adding anything unique to the path since that is prefix
that is used by all LTP tests.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 5/5] swapon03: Remove grep dependency

[Petr Vorel]

> Hi!
> > Do you mean to use full swap file path from /proc/mounts, i.e.
> > /tmp/LTP_swaNzeMJr/mntpoint/testswap, /tmp/LTP_swamEVVAc/mntpoint/testswap03,
> > ... (example from run which created /tmp/LTP_swamEVVAc with all swap files,
> > cleanup interrupted by ctrl+C and following one which created /tmp/LTP_swaNzeMJr
> > single swap file only):

> If we make the swapfile names unique enough we can match just that
> instead. I think one of the patches did just that by including the test
> name in the swap filenames. If we want to be extra sure we can take last
> directory component of the test temporary directory as well.

FYI the problem I wanted to address here was during repeatedly running this test
(swapon03.c) during debugging. Therefore $TMPDIR directory component would help
or $PID would help in this case (but sure the test name itself would help in
possible clash of 2 different tests). I'll probably implement it by $TMPDIR
subset unless you see any usefulness of normalizing $TMPDIR.

But I'll postpone it, in v3 I'll just remove fork and "Try to swapon() as many
files until it fails" to get the original problem merged first.

> What I'm
> trying to point out is that the $TMPDIR part of the test temporary
> directory is not adding anything unique to the path since that is prefix
> that is used by all LTP tests.

Understand. FYI I added the full path to the file to avoid the need to create
full path from subset of the path. I'll probably go this way, although I still
think more tests expect normalized path to TMPDIR, but we don't normalize it
nor even check for it (it'd be interesting to see how many tests in
runtest/syscalls will fail with TMPDIR=/run/../tmp or TMPDIR=/tmp///).

Kind regards,
Petr

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