All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries
@ 2026-07-08 10:42 Wake Liu via ltp
  2026-07-08 12:25 ` [LTP] " linuxtestproject.agent
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Wake Liu via ltp @ 2026-07-08 10:42 UTC (permalink / raw)
  To: ltp; +Cc: Wake Liu

### Background
When executing `madvise09`, the test intermittently fails with:

```
madvise09.c:163: TFAIL: MADV_FREE pages were freed immediately
...
madvise09.c:116: TINFO: 0x7476845000 unexpected   (0) at 1 expected 'a'
madvise09.c:232: TFAIL: Found 2 corrupted page(s)
```

---

### Root Cause Analysis
By analyzing the failure logs, we discovered that these failures only occurred when the test entered its retry block ("Both children killed, retrying...").

1. **Why does it retry? (OOM Race Condition)**
   The test relies on inducing memory pressure to trigger the OOM killer on the `memory_pressure_child`. However, because the entire cgroup is constrained by a strict `memory.max` of 8MB (`MEM_LIMIT`), we observed instances where the parent process (`child()`) was also terminated by the OOM killer.
   Specifically, when the memory-pressure child is killed, the parent process wakes up from `SAFE_WAIT(&status)` (which invokes `wait4`). While writing the exit status to the user-space stack pointer `&status`, the parent triggers a swap-in page fault (`do_swap_page` -> `mem_cgroup_swapin_charge_folio`).
   Since the memory-pressure child was just killed and its memory charges have not yet been fully freed, the parent's page fault hits the strict 8MB limit, invoking the OOM killer on the parent process. The test framework catches `WIFSIGNALED(status)` in `run()` and jumps to `retry:`.

2. **Why does the retry fail? (Cgroup Limit Leakage)**
   When retrying, `run()` forks a new `child()`. However, the cgroup `memory.max` (8MB) and `memory.swap.max` (16MB) set in the previous run were never reset.
   As a result, the new child starts executing in a cgroup that is already heavily constrained by the inherited 8MB limit.
   - When the child allocates pages, writes `'a'`s, and calls `madvise(..., MADV_FREE)`, the kernel immediately drops and reclaims the pages to relieve the 8MB memory pressure. This triggers the first failure: `MADV_FREE pages were freed immediately`.
   - Subsequently, when the test writes `'b'` to the first byte of `TOUCHED_PAGE1` (offset 0), it triggers a Copy-on-Write (CoW) on a zero-page (since the page was already reclaimed). The page is refaulted with `'b'` at offset 0, but offsets 1-4095 remain `0` (instead of the original `'a'`).
   - Finally, `check_page_baaa()` reads offset 1, finds `0` instead of `'a'`, and reports false-positive "page corruption" failures.

---

### Fix
This patch resets both `memory.max` and `memory.swap.max` to `"max"` (unconstrained) at the beginning of the `child()` function. This guarantees that each retry run starts in a clean cgroup environment. The strict memory limits will only be applied later in the test flow after `MADV_FREE` has been marked and initial page edits are completed.

Signed-off-by: Wake Liu <wakel@google.com>
---
 testcases/kernel/syscalls/madvise/madvise09.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/testcases/kernel/syscalls/madvise/madvise09.c b/testcases/kernel/syscalls/madvise/madvise09.c
index 87fe096fb..e6fb74fdb 100644
--- a/testcases/kernel/syscalls/madvise/madvise09.c
+++ b/testcases/kernel/syscalls/madvise/madvise09.c
@@ -146,6 +146,16 @@ static void child(void)
 
 	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
 
+	/*
+	 * Reset cgroup memory limits to default ("max") in case this is a retry run.
+	 * Otherwise, the retried child inherits the strict MEM_LIMIT from the previous
+	 * run, causing MADV_FREE pages to be dropped immediately before we touch them.
+	 */
+	if (SAFE_CG_HAS(tst_cg, "memory.max"))
+		SAFE_CG_PRINTF(tst_cg, "memory.max", "max");
+	if (swap_accounting_enabled && SAFE_CG_HAS(tst_cg, "memory.swap.max"))
+		SAFE_CG_PRINTF(tst_cg, "memory.swap.max", "max");
+
 	ptr = SAFE_MMAP(NULL, PAGES * page_size, PROT_READ | PROT_WRITE,
 			MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
-- 
2.55.0.795.g602f6c329a-goog


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

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

* Re: [LTP] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries
  2026-07-08 10:42 [LTP] [PATCH] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries Wake Liu via ltp
@ 2026-07-08 12:25 ` linuxtestproject.agent
  2026-07-09 16:05 ` [LTP] [PATCH] " Andrea Cervesato via ltp
  2026-07-10  2:15 ` [LTP] [PATCH v2] " Wake Liu via ltp
  2 siblings, 0 replies; 16+ messages in thread
From: linuxtestproject.agent @ 2026-07-08 12:25 UTC (permalink / raw)
  To: Wake Liu; +Cc: ltp

Hi Wake,

On Wed, 08 Jul 2026 10:42:15 +0000, Wake Liu wrote:
> ltp: syscalls/madvise09: Reset cgroup memory limits before test retries

The commit message is missing a Signed-off-by: tag.

> +	/*
> +	 * Reset cgroup memory limits to default ("max") in case this is a retry run.
> +	 * Otherwise, the retried child inherits the strict MEM_LIMIT from the previous
> +	 * run, causing MADV_FREE pages to be dropped immediately before we touch them.
> +	 */
> +	if (SAFE_CG_HAS(tst_cg, "memory.max"))
> +		SAFE_CG_PRINTF(tst_cg, "memory.max", "max");

The SAFE_CG_HAS guard here is inconsistent with the unconditional
SAFE_CG_PRINTF call further down in the same function:

    SAFE_CG_PRINTF(tst_cg, "memory.max", "%d", MEM_LIMIT);

Both lines are on the same execution path. If memory.max could be absent,
the existing call would abort via the SAFE_* macro before the test even
reaches the limit-setting step. Since .needs_cgroup_ctrls requires the
memory controller, memory.max will always be present, making the guard
redundant. Dropping it and calling SAFE_CG_PRINTF directly would match the
pattern used in the rest of the function.

> +	if (swap_accounting_enabled && SAFE_CG_HAS(tst_cg, "memory.swap.max"))
> +		SAFE_CG_PRINTF(tst_cg, "memory.swap.max", "max");

The SAFE_CG_HAS check is redundant here. swap_accounting_enabled is set
in setup() only when SAFE_CG_HAS(tst_cg, "memory.swap.max") returns true,
so the condition already implies the file exists. The existing code that
sets SWAP_LIMIT just checks `if (swap_accounting_enabled)` -- the new code
should do the same.

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

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

* Re: [LTP] [PATCH] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries
  2026-07-08 10:42 [LTP] [PATCH] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries Wake Liu via ltp
  2026-07-08 12:25 ` [LTP] " linuxtestproject.agent
@ 2026-07-09 16:05 ` Andrea Cervesato via ltp
  2026-07-10  2:15 ` [LTP] [PATCH v2] " Wake Liu via ltp
  2 siblings, 0 replies; 16+ messages in thread
From: Andrea Cervesato via ltp @ 2026-07-09 16:05 UTC (permalink / raw)
  To: Wake Liu via ltp; +Cc: Wake Liu, ltp

Hi Wake,

patch doesn't compile:
https://github.com/linux-test-project/ltp/actions/runs/28939178833/job/85856906947

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

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

* [LTP] [PATCH v2] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries
  2026-07-08 10:42 [LTP] [PATCH] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries Wake Liu via ltp
  2026-07-08 12:25 ` [LTP] " linuxtestproject.agent
  2026-07-09 16:05 ` [LTP] [PATCH] " Andrea Cervesato via ltp
@ 2026-07-10  2:15 ` Wake Liu via ltp
  2026-07-10  4:56   ` [LTP] " linuxtestproject.agent
                     ` (3 more replies)
  2 siblings, 4 replies; 16+ messages in thread
From: Wake Liu via ltp @ 2026-07-10  2:15 UTC (permalink / raw)
  To: ltp; +Cc: Wake Liu

### Background
When executing `madvise09`, the test intermittently fails with:

```
madvise09.c:163: TFAIL: MADV_FREE pages were freed immediately
...
madvise09.c:116: TINFO: 0x7476845000 unexpected   (0) at 1 expected 'a'
madvise09.c:232: TFAIL: Found 2 corrupted page(s)
```

---

### Root Cause Analysis
By analyzing the failure logs, we discovered that these failures only occurred when the test entered its retry block ("Both children killed, retrying...").

1. **Why does it retry? (OOM Race Condition)**
   The test relies on inducing memory pressure to trigger the OOM killer on the `memory_pressure_child`. However, because the entire cgroup is constrained by a strict `memory.max` of 8MB (`MEM_LIMIT`), we observed instances where the parent process (`child()`) was also terminated by the OOM killer.
   Specifically, when the memory-pressure child is killed, the parent process wakes up from `SAFE_WAIT(&status)` (which invokes `wait4`). While writing the exit status to the user-space stack pointer `&status`, the parent triggers a swap-in page fault (`do_swap_page` -> `mem_cgroup_swapin_charge_folio`).
   Since the memory-pressure child was just killed and its memory charges have not yet been fully freed, the parent's page fault hits the strict 8MB limit, invoking the OOM killer on the parent process. The test framework catches `WIFSIGNALED(status)` in `run()` and jumps to `retry:`.

2. **Why does the retry fail? (Cgroup Limit Leakage)**
   When retrying, `run()` forks a new `child()`. However, the cgroup `memory.max` (8MB) and `memory.swap.max` (16MB) set in the previous run were never reset.
   As a result, the new child starts executing in a cgroup that is already heavily constrained by the inherited 8MB limit.
   - When the child allocates pages, writes `'a'`s, and calls `madvise(..., MADV_FREE)`, the kernel immediately drops and reclaims the pages to relieve the 8MB memory pressure. This triggers the first failure: `MADV_FREE pages were freed immediately`.
   - Subsequently, when the test writes `'b'` to the first byte of `TOUCHED_PAGE1` (offset 0), it triggers a Copy-on-Write (CoW) on a zero-page (since the page was already reclaimed). The page is refaulted with `'b'` at offset 0, but offsets 1-4095 remain `0` (instead of the original `'a'`).
   - Finally, `check_page_baaa()` reads offset 1, finds `0` instead of `'a'`, and reports false-positive "page corruption" failures.

---

### Fix
This patch resets both `memory.max` and `memory.swap.max` to `"max"` (unconstrained) at the beginning of the `child()` function. This guarantees that each retry run starts in a clean cgroup environment. The strict memory limits will only be applied later in the test flow after `MADV_FREE` has been marked and initial page edits are completed.

Signed-off-by: Wake Liu <wakel@google.com>
---
 testcases/kernel/syscalls/madvise/madvise09.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/testcases/kernel/syscalls/madvise/madvise09.c b/testcases/kernel/syscalls/madvise/madvise09.c
index 87fe096fb..9eb6e82ec 100644
--- a/testcases/kernel/syscalls/madvise/madvise09.c
+++ b/testcases/kernel/syscalls/madvise/madvise09.c
@@ -146,6 +146,16 @@ static void child(void)
 
 	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
 
+	/*
+	 * Reset cgroup memory limits to default ("max") in case this is a retry run.
+	 * Otherwise, the retried child inherits the strict MEM_LIMIT from the previous
+	 * run, causing MADV_FREE pages to be dropped immediately before we touch them.
+	 */
+	if (SAFE_CG_HAS(tst_cg, "memory.max"))
+		SAFE_CG_PRINT(tst_cg, "memory.max", "max");
+	if (swap_accounting_enabled && SAFE_CG_HAS(tst_cg, "memory.swap.max"))
+		SAFE_CG_PRINT(tst_cg, "memory.swap.max", "max");
+
 	ptr = SAFE_MMAP(NULL, PAGES * page_size, PROT_READ | PROT_WRITE,
 			MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
-- 
2.55.0.795.g602f6c329a-goog


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

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

* Re: [LTP] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries
  2026-07-10  2:15 ` [LTP] [PATCH v2] " Wake Liu via ltp
@ 2026-07-10  4:56   ` linuxtestproject.agent
  2026-07-10  6:52   ` [LTP] [PATCH v2] " Andrea Cervesato via ltp
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: linuxtestproject.agent @ 2026-07-10  4:56 UTC (permalink / raw)
  To: Wake Liu; +Cc: ltp

Hi Wake,

On Fri, 10 Jul 2026 02:15:02 +0000, Wake Liu wrote:
> ltp: syscalls/madvise09: Reset cgroup memory limits before test retries

> ### Background
> When executing `madvise09`, the test intermittently fails with:
>
> ```
> madvise09.c:163: TFAIL: MADV_FREE pages were freed immediately
> ...
> madvise09.c:116: TINFO: 0x7476845000 unexpected   (0) at 1 expected 'a'
> madvise09.c:232: TFAIL: Found 2 corrupted page(s)
> ```

The Signed-off-by: tag is missing from the commit message. It is
mandatory and must appear at the end of the body.

The commit body shows the failure symptoms but does not explain the
root cause. The code comment explains it well (cgroup memory.max
persists at MEM_LIMIT after a retry, so the new child inherits a
constrained limit before it can even touch the madvised pages). That
root cause belongs in the commit message body, not only in the source
comment.

> +	if (SAFE_CG_HAS(tst_cg, "memory.max"))
> +		SAFE_CG_PRINT(tst_cg, "memory.max", "max");

> 	SAFE_CG_PRINTF(tst_cg, "memory.max", "%d", MEM_LIMIT);

There is an inconsistency here. The reset is guarded by
SAFE_CG_HAS(tst_cg, "memory.max"), but the subsequent write to the
same file (a few lines later) is unconditional. If memory.max can be
absent, the unconditional write would abort. If it is always present
when the memory controller is available (as the unconditional write
assumes), the guard is dead code and misleading.

setup() uses SAFE_CG_HAS only for memory.swap.max, because swap
accounting is optional. memory.max is mandatory with the memory
controller. Dropping the SAFE_CG_HAS guard from the new code would
be consistent with the rest of the file.

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

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

* Re: [LTP] [PATCH v2] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries
  2026-07-10  2:15 ` [LTP] [PATCH v2] " Wake Liu via ltp
  2026-07-10  4:56   ` [LTP] " linuxtestproject.agent
@ 2026-07-10  6:52   ` Andrea Cervesato via ltp
  2026-07-13  1:18     ` Wake Liu via ltp
  2026-07-13  1:25   ` Wake Liu via ltp
  2026-07-13  1:25   ` [LTP] [PATCH v3] madvise09: Reset cgroup limits before retrying test Wake Liu via ltp
  3 siblings, 1 reply; 16+ messages in thread
From: Andrea Cervesato via ltp @ 2026-07-10  6:52 UTC (permalink / raw)
  To: Wake Liu via ltp; +Cc: Wake Liu, ltp

Hi Wake,

the commit message is unwatchable. It's not following any commit standard,
including text length, format and description. Please follow the basics,
otherwise we will reject the patch before reading it:

https://cbea.ms/git-commit/

If you really want to use AI, as it seems, please use the rules in this
repo: https://github.com/linux-test-project/ltp-agent

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

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

* Re: [LTP] [PATCH v2] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries
  2026-07-10  6:52   ` [LTP] [PATCH v2] " Andrea Cervesato via ltp
@ 2026-07-13  1:18     ` Wake Liu via ltp
  0 siblings, 0 replies; 16+ messages in thread
From: Wake Liu via ltp @ 2026-07-13  1:18 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: Wake Liu via ltp

Ack. Apologies for the non-standard commit message. I will correct the
commit message according to the guidelines and send v3.
Thanks for the feedback and the link to the ltp-agent repository, they
are useful.

On Fri, Jul 10, 2026 at 2:52 PM Andrea Cervesato
<andrea.cervesato@suse.com> wrote:
>
> Hi Wake,
>
> the commit message is unwatchable. It's not following any commit standard,
> including text length, format and description. Please follow the basics,
> otherwise we will reject the patch before reading it:
>
> https://cbea.ms/git-commit/
>
> If you really want to use AI, as it seems, please use the rules in this
> repo: https://github.com/linux-test-project/ltp-agent
>
> Regards,
> --
> Andrea Cervesato
> SUSE QE Automation Engineer Linux
> andrea.cervesato@suse.com



-- 
Best Regards,
Wake Liu

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

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

* Re: [LTP] [PATCH v2] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries
  2026-07-10  2:15 ` [LTP] [PATCH v2] " Wake Liu via ltp
  2026-07-10  4:56   ` [LTP] " linuxtestproject.agent
  2026-07-10  6:52   ` [LTP] [PATCH v2] " Andrea Cervesato via ltp
@ 2026-07-13  1:25   ` Wake Liu via ltp
  2026-07-13  1:25   ` [LTP] [PATCH v3] madvise09: Reset cgroup limits before retrying test Wake Liu via ltp
  3 siblings, 0 replies; 16+ messages in thread
From: Wake Liu via ltp @ 2026-07-13  1:25 UTC (permalink / raw)
  To: ltp

Ack. Apologies for the non-standard commit message. I will correct the commit message according to the guidelines and send v3. 
Thanks for the feedback and the link to the ltp-agent repository, they are useful.

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

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

* [LTP] [PATCH v3] madvise09: Reset cgroup limits before retrying test
  2026-07-10  2:15 ` [LTP] [PATCH v2] " Wake Liu via ltp
                     ` (2 preceding siblings ...)
  2026-07-13  1:25   ` Wake Liu via ltp
@ 2026-07-13  1:25   ` Wake Liu via ltp
  2026-07-13  2:44     ` [LTP] " linuxtestproject.agent
  2026-07-13 11:42     ` [LTP] [PATCH v4] " Wake Liu via ltp
  3 siblings, 2 replies; 16+ messages in thread
From: Wake Liu via ltp @ 2026-07-13  1:25 UTC (permalink / raw)
  To: ltp; +Cc: Wake Liu

When madvise09 enters its retry loop, the cgroup limits (memory.max and
memory.swap.max) configured in the previous run remain active. If the
previous run ended due to memory pressure, the inherited limits will
cause the new child to immediately reclaim MADV_FREE pages. This
results in false-positive failures during test verification.

Reset both memory.max and memory.swap.max to "max" at the start of
child() to ensure each retry run begins in an unconstrained cgroup.

Signed-off-by: Wake Liu <wakel@google.com>
---
 testcases/kernel/syscalls/madvise/madvise09.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/testcases/kernel/syscalls/madvise/madvise09.c b/testcases/kernel/syscalls/madvise/madvise09.c
index 87fe096fb..9eb6e82ec 100644
--- a/testcases/kernel/syscalls/madvise/madvise09.c
+++ b/testcases/kernel/syscalls/madvise/madvise09.c
@@ -146,6 +146,16 @@ static void child(void)
 
 	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
 
+	/*
+	 * Reset cgroup memory limits to default ("max") in case this is a retry run.
+	 * Otherwise, the retried child inherits the strict MEM_LIMIT from the previous
+	 * run, causing MADV_FREE pages to be dropped immediately before we touch them.
+	 */
+	if (SAFE_CG_HAS(tst_cg, "memory.max"))
+		SAFE_CG_PRINT(tst_cg, "memory.max", "max");
+	if (swap_accounting_enabled && SAFE_CG_HAS(tst_cg, "memory.swap.max"))
+		SAFE_CG_PRINT(tst_cg, "memory.swap.max", "max");
+
 	ptr = SAFE_MMAP(NULL, PAGES * page_size, PROT_READ | PROT_WRITE,
 			MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
-- 
2.55.0.795.g602f6c329a-goog


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

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

* Re: [LTP] madvise09: Reset cgroup limits before retrying test
  2026-07-13  1:25   ` [LTP] [PATCH v3] madvise09: Reset cgroup limits before retrying test Wake Liu via ltp
@ 2026-07-13  2:44     ` linuxtestproject.agent
  2026-07-13 11:23       ` Andrea Cervesato via ltp
  2026-07-13 11:42     ` [LTP] [PATCH v4] " Wake Liu via ltp
  1 sibling, 1 reply; 16+ messages in thread
From: linuxtestproject.agent @ 2026-07-13  2:44 UTC (permalink / raw)
  To: Wake Liu; +Cc: ltp

Hi Wake,

On Mon Jul 13 01:25:45 2026, Wake Liu wrote:
> madvise09: Reset cgroup limits before retrying test

> +	if (SAFE_CG_HAS(tst_cg, "memory.max"))
> +		SAFE_CG_PRINT(tst_cg, "memory.max", "max");

The test requires `.needs_cgroup_ctrls = { "memory", NULL }`, so
memory.max is guaranteed to exist when the memory controller is
available. The SAFE_CG_HAS guard is unnecessary here and creates an
inconsistency with the unconditional write to the same file later:

    SAFE_CG_PRINTF(tst_cg, "memory.max", "%d", MEM_LIMIT);

Can the guard be dropped to match the style used for the later write?

> +	if (swap_accounting_enabled && SAFE_CG_HAS(tst_cg, "memory.swap.max"))
> +		SAFE_CG_PRINT(tst_cg, "memory.swap.max", "max");

swap_accounting_enabled is already set in setup() based on
SAFE_CG_HAS(tst_cg, "memory.swap.max"), so the second SAFE_CG_HAS
call is redundant. The later write is correctly guarded with just
swap_accounting_enabled:

    if (swap_accounting_enabled) {
        SAFE_CG_PRINTF(tst_cg, "memory.swap.max", "%d", SWAP_LIMIT);

Can this be simplified to match?

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

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

* Re: [LTP] madvise09: Reset cgroup limits before retrying test
  2026-07-13  2:44     ` [LTP] " linuxtestproject.agent
@ 2026-07-13 11:23       ` Andrea Cervesato via ltp
  0 siblings, 0 replies; 16+ messages in thread
From: Andrea Cervesato via ltp @ 2026-07-13 11:23 UTC (permalink / raw)
  To: linuxtestproject.agent; +Cc: Wake Liu, ltp

Hi Wake,

the agent review is correct, please update the patch to v4.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

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

* [LTP] [PATCH v4] madvise09: Reset cgroup limits before retrying test
  2026-07-13  1:25   ` [LTP] [PATCH v3] madvise09: Reset cgroup limits before retrying test Wake Liu via ltp
  2026-07-13  2:44     ` [LTP] " linuxtestproject.agent
@ 2026-07-13 11:42     ` Wake Liu via ltp
  2026-07-13 14:09       ` [LTP] " linuxtestproject.agent
  2026-07-14  7:57       ` [LTP] [PATCH v5] " Wake Liu via ltp
  1 sibling, 2 replies; 16+ messages in thread
From: Wake Liu via ltp @ 2026-07-13 11:42 UTC (permalink / raw)
  To: ltp; +Cc: Wake Liu

When madvise09 enters its retry loop, the cgroup limits (memory.max and
memory.swap.max) configured in the previous run remain active. If the
previous run ended due to memory pressure, the inherited limits will
cause the new child to immediately reclaim MADV_FREE pages. This
results in false-positive failures during test verification.

Reset both memory.max and memory.swap.max to "max" at the start of
child() to ensure each retry run begins in an unconstrained cgroup.

Signed-off-by: Wake Liu <wakel@google.com>
---
v4:
- Remove redundant SAFE_CG_HAS checks for memory.max and memory.swap.max.

v3:
- Fix commit message layout and length to follow guidelines.
- Use SAFE_CG_PRINT instead of SAFE_CG_PRINTF to fix compile errors.

v2:
- Reset memory limits to max in child.

 testcases/kernel/syscalls/madvise/madvise09.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/testcases/kernel/syscalls/madvise/madvise09.c b/testcases/kernel/syscalls/madvise/madvise09.c
index 87fe096fb..821d40545 100644
--- a/testcases/kernel/syscalls/madvise/madvise09.c
+++ b/testcases/kernel/syscalls/madvise/madvise09.c
@@ -146,6 +146,15 @@ static void child(void)
 
 	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
 
+	/*
+	 * Reset cgroup memory limits to default ("max") in case this is a retry run.
+	 * Otherwise, the retried child inherits the strict MEM_LIMIT from the previous
+	 * run, causing MADV_FREE pages to be dropped immediately before we touch them.
+	 */
+	SAFE_CG_PRINT(tst_cg, "memory.max", "max");
+	if (swap_accounting_enabled)
+		SAFE_CG_PRINT(tst_cg, "memory.swap.max", "max");
+
 	ptr = SAFE_MMAP(NULL, PAGES * page_size, PROT_READ | PROT_WRITE,
 			MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
-- 
2.55.0.795.g602f6c329a-goog


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

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

* Re: [LTP] madvise09: Reset cgroup limits before retrying test
  2026-07-13 11:42     ` [LTP] [PATCH v4] " Wake Liu via ltp
@ 2026-07-13 14:09       ` linuxtestproject.agent
  2026-07-14  7:57       ` [LTP] [PATCH v5] " Wake Liu via ltp
  1 sibling, 0 replies; 16+ messages in thread
From: linuxtestproject.agent @ 2026-07-13 14:09 UTC (permalink / raw)
  To: Wake Liu; +Cc: ltp

Hi Wake,

On Mon Jul 13 11:42:20 2026, Wake Liu wrote:
> madvise09: Reset cgroup limits before retrying test

> Reset both memory.max and memory.swap.max to "max" at the start of
> child() to ensure each retry run begins in an unconstrained cgroup.

The description says "reset both" unconditionally, but memory.swap.max
is only reset when swap_accounting_enabled is true:

> +	SAFE_CG_PRINT(tst_cg, "memory.max", "max");
> +	if (swap_accounting_enabled)
> +		SAFE_CG_PRINT(tst_cg, "memory.swap.max", "max");

The code itself is correct -- it mirrors the conditional set pattern
already in child().  The commit message just omits the conditional.
Something like:

  Reset memory.max unconditionally, and memory.swap.max when swap
  accounting is enabled, to "max" at the start of child() ...

would match the code more accurately.

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

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

* [LTP] [PATCH v5] madvise09: Reset cgroup limits before retrying test
  2026-07-13 11:42     ` [LTP] [PATCH v4] " Wake Liu via ltp
  2026-07-13 14:09       ` [LTP] " linuxtestproject.agent
@ 2026-07-14  7:57       ` Wake Liu via ltp
  2026-07-14  8:06         ` [LTP] " linuxtestproject.agent
  2026-07-14  8:09         ` [LTP] [PATCH v5] " Andrea Cervesato via ltp
  1 sibling, 2 replies; 16+ messages in thread
From: Wake Liu via ltp @ 2026-07-14  7:57 UTC (permalink / raw)
  To: ltp; +Cc: Wake Liu

When madvise09 enters its retry loop, the cgroup limits (memory.max and
memory.swap.max) configured in the previous run remain active. If the
previous run ended due to memory pressure, the inherited limits will
cause the new child to immediately reclaim MADV_FREE pages. This
results in false-positive failures during test verification.

Reset memory.max unconditionally, and memory.swap.max when swap
accounting is enabled, to "max" at the start of child() to ensure
each retry run begins in an unconstrained cgroup.

Signed-off-by: Wake Liu <wakel@google.com>
---
v5:
- Update commit message description to clarify that memory.swap.max is only
  reset when swap accounting is enabled, matching the code logic.

v4:
- Remove redundant SAFE_CG_HAS checks for memory.max and memory.swap.max.

v3:
- Fix commit message layout and length to follow guidelines.
- Use SAFE_CG_PRINT instead of SAFE_CG_PRINTF to fix compile errors.

v2:
- Reset memory limits to max in child.

 testcases/kernel/syscalls/madvise/madvise09.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/testcases/kernel/syscalls/madvise/madvise09.c b/testcases/kernel/syscalls/madvise/madvise09.c
index 87fe096fb..821d40545 100644
--- a/testcases/kernel/syscalls/madvise/madvise09.c
+++ b/testcases/kernel/syscalls/madvise/madvise09.c
@@ -146,6 +146,15 @@ static void child(void)
 
 	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
 
+	/*
+	 * Reset cgroup memory limits to default ("max") in case this is a retry run.
+	 * Otherwise, the retried child inherits the strict MEM_LIMIT from the previous
+	 * run, causing MADV_FREE pages to be dropped immediately before we touch them.
+	 */
+	SAFE_CG_PRINT(tst_cg, "memory.max", "max");
+	if (swap_accounting_enabled)
+		SAFE_CG_PRINT(tst_cg, "memory.swap.max", "max");
+
 	ptr = SAFE_MMAP(NULL, PAGES * page_size, PROT_READ | PROT_WRITE,
 			MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
-- 
2.55.0.795.g602f6c329a-goog


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

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

* Re: [LTP] madvise09: Reset cgroup limits before retrying test
  2026-07-14  7:57       ` [LTP] [PATCH v5] " Wake Liu via ltp
@ 2026-07-14  8:06         ` linuxtestproject.agent
  2026-07-14  8:09         ` [LTP] [PATCH v5] " Andrea Cervesato via ltp
  1 sibling, 0 replies; 16+ messages in thread
From: linuxtestproject.agent @ 2026-07-14  8:06 UTC (permalink / raw)
  To: Wake Liu; +Cc: ltp

Hi Wake,

On Tue, 14 Jul 2026, Wake Liu wrote:
> madvise09: Reset cgroup limits before retrying test

Verdict - Reviewed

---
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

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

* Re: [LTP] [PATCH v5] madvise09: Reset cgroup limits before retrying test
  2026-07-14  7:57       ` [LTP] [PATCH v5] " Wake Liu via ltp
  2026-07-14  8:06         ` [LTP] " linuxtestproject.agent
@ 2026-07-14  8:09         ` Andrea Cervesato via ltp
  1 sibling, 0 replies; 16+ messages in thread
From: Andrea Cervesato via ltp @ 2026-07-14  8:09 UTC (permalink / raw)
  To: Wake Liu; +Cc: Wake Liu, ltp

Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

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

end of thread, other threads:[~2026-07-14  8:09 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08 10:42 [LTP] [PATCH] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries Wake Liu via ltp
2026-07-08 12:25 ` [LTP] " linuxtestproject.agent
2026-07-09 16:05 ` [LTP] [PATCH] " Andrea Cervesato via ltp
2026-07-10  2:15 ` [LTP] [PATCH v2] " Wake Liu via ltp
2026-07-10  4:56   ` [LTP] " linuxtestproject.agent
2026-07-10  6:52   ` [LTP] [PATCH v2] " Andrea Cervesato via ltp
2026-07-13  1:18     ` Wake Liu via ltp
2026-07-13  1:25   ` Wake Liu via ltp
2026-07-13  1:25   ` [LTP] [PATCH v3] madvise09: Reset cgroup limits before retrying test Wake Liu via ltp
2026-07-13  2:44     ` [LTP] " linuxtestproject.agent
2026-07-13 11:23       ` Andrea Cervesato via ltp
2026-07-13 11:42     ` [LTP] [PATCH v4] " Wake Liu via ltp
2026-07-13 14:09       ` [LTP] " linuxtestproject.agent
2026-07-14  7:57       ` [LTP] [PATCH v5] " Wake Liu via ltp
2026-07-14  8:06         ` [LTP] " linuxtestproject.agent
2026-07-14  8:09         ` [LTP] [PATCH v5] " Andrea Cervesato via ltp

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.