From: linuxtestproject.agent@gmail.com
To: Pavithra <pavrampu@linux.ibm.com>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] memcg/memcontrol05: add cgroup v2 task migration charge accounting test
Date: Tue, 4 Aug 2026 10:41:58 +0000 [thread overview]
Message-ID: <20260804104158.9052-1-linuxtestproject.agent@gmail.com> (raw)
In-Reply-To: <20260804095925.495813-1-pavrampu@linux.ibm.com>
Hi Pavithra,
On Aug 4, 2026, Pavithra wrote:
> memcg/memcontrol05: add cgroup v2 task migration charge accounting test
> buf1 = SAFE_MMAP(NULL, ALLOC_SIZE, PROT_READ | PROT_WRITE,
> MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);
>
> TST_CHECKPOINT_WAKE(WORKER_ALLOC_DONE);
> [...]
> buf2 = SAFE_MMAP(NULL, ALLOC_SIZE2, PROT_READ | PROT_WRITE,
> MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);
>
> TST_CHECKPOINT_WAKE(WORKER_ALLOC2_DONE);
Could these mappings be populated by explicitly writing one byte per page,
and could the test declare an appropriate .min_mem_avail requirement?
mmap(2) states that mmap() does not fail when MAP_POPULATE cannot populate
the whole mapping. Linux 7.2 also discards mm_populate() errors. Under memory
pressure, the parent can therefore report TFAIL for an incomplete allocation
even though cgroup migration accounting is correct. The test needs 80 MB, but
currently has no memory prerequisite to turn that condition into TCONF.
> pid_t pid;
> long baseline_b, after_migrate, after_alloc2, current_a;
> worker_resumed = false;
Could a blank line be added after the declarations? check-memcontrol05
reports:
memcontrol05.c:80: WARNING: Missing a blank line after declarations
> * Two child cgroups (group_a, group_b) are created under the test cgroup.
> * A worker process is placed in group_a and allocates a known amount of
> [...]
> * group_b/cgroup.procs. In cgroup v2 memory charges are never
> [...]
> * that allocated them. group_b/memory.current must not increase
Could identifiers and file names in this exported RST block use double
backticks as required for inline literals?
> .needs_root = 1,
Could the description also state why root is required? Root-only tests must
document the reason; here it is needed to create cgroups and move the worker
through cgroup.procs.
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
next prev parent reply other threads:[~2026-08-04 10:42 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-04 9:59 [LTP] [PATCH] memcg/memcontrol05: add cgroup v2 task migration charge accounting test Pavithra
2026-08-04 10:41 ` linuxtestproject.agent [this message]
-- strict thread matches above, loose matches on Subject: below --
2026-08-12 16:16 [LTP] [PATCH v2] " Pavithra
2026-08-12 17:07 ` [LTP] " linuxtestproject.agent
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260804104158.9052-1-linuxtestproject.agent@gmail.com \
--to=linuxtestproject.agent@gmail.com \
--cc=ltp@lists.linux.it \
--cc=pavrampu@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.