* [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.