[PATCH v3 2/2] mem/min_free_kbytes: refactor to use SAFE_FORK and simplify tune logic

Sachin Sant <[email protected]>
Newsgroups gmane.linux.ltp
Message-ID <[email protected]>
The test mixed old bare-fork patterns with the newer LTP API, making it
hard to follow and easy to miscount pass/fail results.  Extract the tune
selection into set_min_free_kbytes(), replace raw fork() with SAFE_FORK()
throughout, and drop the manual SAFE_WAITPID on the monitor child — the
framework's tst_reap_children() handles it after test_all() returns.

Suggested-by: Andrea Cervesato <[email protected]>
Signed-off-by: Sachin Sant <[email protected]>
---
v2 -> v3:
- Extracted tune selection into a new set_min_free_kbytes() helper,
  replacing the open-coded if/else chain in test_tune().
- Replaced raw fork() with SAFE_FORK() in test_tune(); pid array
  collapsed to a single pid_t since each child is waited inline.
- Dropped SAFE_WAITPID on the monitor child from min_free_kbytes_test();
  tst_reap_children() called by the framework after test_all() returns
  handles it instead.
- Replaced bare MIN() open-code in tune selection with the MIN() macro.
- Dropped leftover TPASS/TFAIL from the parent's wait block;
  result reporting stays solely in check_monitor(). 

v1 -> v2:
- Addressed review comments by moving TPASS/TFAIL reporting
  into check_monitor() in the child process.
---
 .../kernel/mem/tunable/min_free_kbytes.c      | 67 ++++++++++---------
 1 file changed, 34 insertions(+), 33 deletions(-)

diff --git a/testcases/kernel/mem/tunable/min_free_kbytes.c b/testcases/kernel/mem/tunable/min_free_kbytes.c
index b1d00e453..4f991025a 100644
--- a/testcases/kernel/mem/tunable/min_free_kbytes.c
+++ b/testcases/kernel/mem/tunable/min_free_kbytes.c
@@ -44,9 +44,30 @@ static int eatup_mem(unsigned long overcommit_policy);
 static void check_monitor(void);
 static void sighandler(int signo LTP_ATTRIBUTE_UNUSED);
 
+static void set_min_free_kbytes(int i)
+{
+	unsigned long memfree, memtotal, tune;
+
+	switch (i) {
+	case 0:
+		tune = default_tune;
+		break;
+	case 1:
+		tune = 2 * default_tune;
+		break;
+	default:
+		memfree = SAFE_READ_MEMINFO("MemFree:");
+		memtotal = SAFE_READ_MEMINFO("MemTotal:");
+		tune = MIN(memfree / 20, memtotal / 50);
+		break;
+	}
+
+	TST_SYS_CONF_LONG_SET(PATH_VM_MIN_FREE_KBYTES, tune, 1);
+}
+
 static void min_free_kbytes_test(void)
 {
-	int pid, status;
+	pid_t pid;
 	struct sigaction sa;
 
 	sa.sa_handler = sighandler;
@@ -56,7 +77,6 @@ static void min_free_kbytes_test(void)
 
 	pid = SAFE_FORK();
 	if (pid == 0) {
-		/* startup the check monitor */
 		check_monitor();
 		exit(0);
 	}
@@ -66,50 +86,26 @@ static void min_free_kbytes_test(void)
 	test_tune(1);
 
 	SAFE_KILL(pid, SIGUSR1);
-	SAFE_WAITPID(pid, &status, WUNTRACED | WCONTINUED);
-
-	if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
-		tst_res(TFAIL, "check_monitor child exit with status: %s",
-			tst_strstatus(status));
-
-	tst_res(TPASS, "min_free_kbytes test pass");
 }
 
 static void test_tune(unsigned long overcommit_policy)
 {
 	int status;
-	int pid[3];
-	int ret, i;
-	unsigned long tune, memfree, memtotal;
+	pid_t pid;
+	int i;
 
 	TST_SYS_CONF_LONG_SET(PATH_VM_OVERCOMMIT_MEMORY, overcommit_policy, 1);
 
 	for (i = 0; i < 3; i++) {
-		if (i == 0)
-			TST_SYS_CONF_LONG_SET(PATH_VM_MIN_FREE_KBYTES, default_tune, 1);
-		else if (i == 1) {
-			TST_SYS_CONF_LONG_SET(PATH_VM_MIN_FREE_KBYTES, 2 * default_tune, 1);
-		} else {
-			memfree = SAFE_READ_MEMINFO("MemFree:");
-			memtotal = SAFE_READ_MEMINFO("MemTotal:");
-			tune = memfree / 20;
-			if (tune > (memtotal / 50))
-				tune = memtotal / 50;
-
-			TST_SYS_CONF_LONG_SET(PATH_VM_MIN_FREE_KBYTES, tune, 1);
-		}
+		set_min_free_kbytes(i);
 
 		fflush(stdout);
-		switch (pid[i] = fork()) {
-		case -1:
-			tst_brk(TBROK | TERRNO, "fork");
-			break;
-		case 0:
-			ret = eatup_mem(overcommit_policy);
-			exit(ret);
+		pid = SAFE_FORK();
+		if (pid == 0) {
+			exit(eatup_mem(overcommit_policy));
 		}
 
-		SAFE_WAITPID(pid[i], &status, WUNTRACED | WCONTINUED);
+		SAFE_WAITPID(pid, &status, WUNTRACED | WCONTINUED);
 
 		if (overcommit_policy == 2) {
 			if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
@@ -172,6 +168,7 @@ static int eatup_mem(unsigned long overcommit_policy)
 
 static void check_monitor(void)
 {
+	int violated = 0;
 	unsigned long tune;
 	unsigned long memfree;
 
@@ -183,10 +180,14 @@ static void check_monitor(void)
 			tst_res(TINFO, "MemFree is %lu kB, "
 				"min_free_kbytes is %lu kB", memfree, tune);
 			tst_res(TFAIL, "MemFree < min_free_kbytes");
+			violated = 1;
 		}
 
 		sleep(2);
 	}
+
+	if (!violated)
+		tst_res(TPASS, "min_free_kbytes test pass");
 }
 
 static void sighandler(int signo LTP_ATTRIBUTE_UNUSED)
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.