[LTP] [PATCH] mem/min_free_kbytes: Fix incorrect pass/fail accounting

[LTP] [PATCH] mem/min_free_kbytes: Fix incorrect pass/fail accounting

[Sachin Sant]

When check_monitor() detected a violation (MemFree < min_free_kbytes) it
called tst_res(TFAIL, ...) from the child process, which atomically
incremented the shared results->failed counter, and then the child exited
with status 0. Back in the parent, min_free_kbytes_test() checked the
child exit status, found it was 0, and fell through to the unconditional
tst_res(TPASS, ...) at the end of the function.

This produced a misleading summary of 'passed 1 / failed 1' on violation:
the TFAIL from the monitor child was correctly counted, but the
unconditional TPASS that followed also added to the pass count regardless
of the violation.

Fix this by:
- Removing the tst_res(TFAIL, ...) from check_monitor() and replacing it
  with a local 'violated' flag; the TINFO diagnostic is kept so the exact
  MemFree and tune values are still logged.
- Returning 'violated' from check_monitor() and passing it directly to
  exit(), so the child exits non-zero when a violation was detected.
- Restructuring the parent's wait-result check so that TPASS is only
  emitted when the monitor child exited normally with status 0; any other
  termination (signal death, unexpected non-zero exit) maps to TFAIL.
  A violation exit (status == 1) emits a semantically accurate message;
  all other abnormal exits fall back to tst_strstatus() for diagnostics.

After the fix the summary correctly reflects the outcome:
  - No violation:  passed 1 / failed 0
  - Violation:     passed 0 / failed 1

Signed-off-by: Sachin Sant <sachinp@linux.ibm.com>
---
 .../kernel/mem/tunable/min_free_kbytes.c      | 28 +++++++++++--------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/testcases/kernel/mem/tunable/min_free_kbytes.c b/testcases/kernel/mem/tunable/min_free_kbytes.c
index bdc9126c2..5bed61db5 100644
--- a/testcases/kernel/mem/tunable/min_free_kbytes.c
+++ b/testcases/kernel/mem/tunable/min_free_kbytes.c
@@ -41,7 +41,7 @@ static unsigned long total_mem;
 
 static void test_tune(unsigned long overcommit_policy);
 static int eatup_mem(unsigned long overcommit_policy);
-static void check_monitor(void);
+static int check_monitor(void);
 static void sighandler(int signo LTP_ATTRIBUTE_UNUSED);
 
 static void min_free_kbytes_test(void)
@@ -57,8 +57,7 @@ static void min_free_kbytes_test(void)
 	pid = SAFE_FORK();
 	if (pid == 0) {
 		/* startup the check monitor */
-		check_monitor();
-		exit(0);
+		exit(check_monitor());
 	}
 
 	test_tune(2);
@@ -68,11 +67,15 @@ static void min_free_kbytes_test(void)
 	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");
+	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+		if (WIFEXITED(status) && WEXITSTATUS(status) == 1)
+			tst_res(TFAIL, "MemFree dropped below min_free_kbytes");
+		else
+			tst_res(TFAIL, "check_monitor child failed: %s",
+				tst_strstatus(status));
+	} else {
+		tst_res(TPASS, "min_free_kbytes test pass");
+	}
 }
 
 static void test_tune(unsigned long overcommit_policy)
@@ -171,8 +174,9 @@ static int eatup_mem(unsigned long overcommit_policy)
 	return ret;
 }
 
-static void check_monitor(void)
+static int check_monitor(void)
 {
+	int violated = 0;
 	unsigned long tune;
 	unsigned long memfree;
 
@@ -182,12 +186,14 @@ static void check_monitor(void)
 
 		if (memfree < tune) {
 			tst_res(TINFO, "MemFree is %lu kB, "
-				 "min_free_kbytes is %lu kB", memfree, tune);
-			tst_res(TFAIL, "MemFree < min_free_kbytes");
+				"min_free_kbytes is %lu kB", memfree, tune);
+			violated = 1;
 		}
 
 		sleep(2);
 	}
+
+	return violated;
 }
 
 static void sighandler(int signo LTP_ATTRIBUTE_UNUSED)
-- 
2.39.1


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

Re: [LTP] mem/min_free_kbytes: Fix incorrect pass/fail accounting

[linuxtestproject.agent]

Hi Sachin,

On Wed, 08 Jul 2026 18:12:05 +0530, Sachin Sant wrote:
> mem/min_free_kbytes: Fix incorrect pass/fail accounting

> @@ -57,8 +57,7 @@ static void min_free_kbytes_test(void)
>  	pid = SAFE_FORK();
>  	if (pid == 0) {
>  		/* startup the check monitor */
> -		check_monitor();
> -		exit(0);
> +		exit(check_monitor());
>  	}

> +	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
> +		if (WIFEXITED(status) && WEXITSTATUS(status) == 1)
> +			tst_res(TFAIL, "MemFree dropped below min_free_kbytes");
> +		else
> +			tst_res(TFAIL, "check_monitor child failed: %s",
> +				tst_strstatus(status));
> +	} else {
> +		tst_res(TPASS, "min_free_kbytes test pass");
> +	}

The fix works in practice, but the design trades one problem for another:
encoding pass/fail as a child exit code and interpreting it in the parent
is explicitly the pattern LTP asks to avoid.

The test already sets .forks_child = 1, so the framework's shared-memory
result propagation is active.  tst_res() calls made inside check_monitor()
are automatically visible to the parent's result accounting -- no exit-code
relay needed.

> -static void check_monitor(void)
> +static int check_monitor(void)
>  {
> +	int violated = 0;

The 'violated' flag is fine for deciding when to emit the TINFO line, but
the TPASS/TFAIL themselves should be emitted by the child rather than
signalled through exit():

  if (violated)
      tst_res(TFAIL, "MemFree dropped below min_free_kbytes");
  else
      tst_res(TPASS, "min_free_kbytes test pass");

With that, check_monitor() returns void and the child exits 0 always.
The parent's wait block only needs to guard against truly unexpected
termination (signal, unknown non-zero exit), and must not re-emit a
TPASS/TFAIL for the violation outcome:

  SAFE_KILL(pid, SIGUSR1);
  SAFE_WAITPID(pid, &status, WUNTRACED | WCONTINUED);

  if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
      tst_res(TFAIL, "check_monitor child failed: %s",
          tst_strstatus(status));
  /* pass/fail already reported by the child */

This also solves the original double-accounting bug because the parent
no longer emits any result for the monitor outcome at all.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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

[LTP] [PATCH v2] mem/min_free_kbytes: Fix incorrect pass/fail accounting

[Sachin Sant]

When check_monitor() detected a violation (MemFree < min_free_kbytes) it
called tst_res(TFAIL, ...) from the child process, which atomically
incremented the shared results->failed counter, and then the child exited
with status 0. Back in the parent, min_free_kbytes_test() checked the
child exit status, found it was 0, and fell through to the unconditional
tst_res(TPASS, ...) at the end of the function.

This produced a misleading summary of 'passed 1 / failed 1' on violation:
the TFAIL from the monitor child was correctly counted, but the
unconditional TPASS that followed also added to the pass count regardless
of the violation.

Fix this by moving TPASS/TFAIL reporting into check_monitor() in the child
process. Emit tst_res(TFAIL, ...) immediately on each individual violation
so that the TINFO diagnostic and its corresponding TFAIL are always paired;
the 'violated' flag is kept to suppress the final TPASS when any breach was
seen. Emit tst_res(TPASS, ...) only when no violation was observed across
the entire monitoring run.
The parent's wait-result block is changed to guard against unexpected
termination (signal death or non-zero exit) only; it emits no result for
the monitor outcome since that is already reported by the child. The
original condition WIFEXITED && WEXITSTATUS != 0 missed signal death;
the corrected condition is !WIFEXITED || WEXITSTATUS != 0.

After the fix the summary correctly reflects the outcome:
- No violation:  passed 1 / failed 0
- Violation:     passed 0 / failed N  (one TFAIL per breached sample)

Signed-off-by: Sachin Sant <sachinp@linux.ibm.com>
---
v1 -> v2:
- Addressed review comments by moving TPASS/TFAIL reporting
  into check_monitor() in the child process.

---
 testcases/kernel/mem/tunable/min_free_kbytes.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/mem/tunable/min_free_kbytes.c b/testcases/kernel/mem/tunable/min_free_kbytes.c
index bdc9126c2..132c8fe70 100644
--- a/testcases/kernel/mem/tunable/min_free_kbytes.c
+++ b/testcases/kernel/mem/tunable/min_free_kbytes.c
@@ -68,11 +68,9 @@ static void min_free_kbytes_test(void)
 	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",
+	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
+		tst_res(TFAIL, "check_monitor child failed: %s",
 			tst_strstatus(status));
-
-	tst_res(TPASS, "min_free_kbytes test pass");
 }
 
 static void test_tune(unsigned long overcommit_policy)
@@ -173,6 +171,7 @@ static int eatup_mem(unsigned long overcommit_policy)
 
 static void check_monitor(void)
 {
+	int violated = 0;
 	unsigned long tune;
 	unsigned long memfree;
 
@@ -182,12 +181,16 @@ static void check_monitor(void)
 
 		if (memfree < tune) {
 			tst_res(TINFO, "MemFree is %lu kB, "
-				 "min_free_kbytes is %lu kB", memfree, tune);
+				"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

Re: [LTP] mem/min_free_kbytes: Fix incorrect pass/fail accounting

[linuxtestproject.agent]

Hi Sachin,

On Thu Jul 9 09:43:23 2026 +0530, Sachin Sant wrote:
> mem/min_free_kbytes: Fix incorrect pass/fail accounting

Verdict - Reviewed

---
Pre-existing issues:

In check_monitor(), the flag variable used for signal-handler
communication is declared as:

  volatile int end;

Strict C (C99 and later) requires volatile sig_atomic_t for variables
read/written by a signal handler.  This is pre-existing and harmless on
common architectures, but worth fixing in a follow-up.

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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