* [PATCH 0/2] selftests/cgroup: test_zswap: fix single-core flakes
@ 2026-08-04 4:20 Wilson Felipe Pereira
2026-08-04 4:20 ` [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback Wilson Felipe Pereira
2026-08-04 4:20 ` [PATCH 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass Wilson Felipe Pereira
0 siblings, 2 replies; 5+ messages in thread
From: Wilson Felipe Pereira @ 2026-08-04 4:20 UTC (permalink / raw)
To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou,
Tejun Heo, Michal Koutný, Shuah Khan
Cc: linux-mm, cgroups, linux-kselftest, Wilson Felipe Pereira
When running the cgroup zswap selftests on a single-core VM (-smp 1) with
4GB of RAM, test_zswap_writeback and test_no_kmem_bypass reliably fail on
the initial run after boot due to timing and asynchronous tasks not
completed.
This 2-patch series fixes both initial-run failures:
* Patch 1 fixes an EBUSY race in test_zswap_writeback() where writing
"+memory" to cgroup.subtree_control collides with the kernel's
asynchronous RCU removal of dead child tasks from cgroup.procs.
* Patch 2 fixes an implicit unsigned integer promotion bug in
test_no_kmem_bypass() where a small negative delta becomes large when
compared to an unsigned size_t threshold.
Both tests now reliably pass on the initial run after cold boot on -smp 1
VMs.
Wilson Felipe Pereira (2):
selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in
test_zswap_writeback
selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in
test_no_kmem_bypass
tools/testing/selftests/cgroup/test_zswap.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
--
2.55.0.571.g244d577d93-goog
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback 2026-08-04 4:20 [PATCH 0/2] selftests/cgroup: test_zswap: fix single-core flakes Wilson Felipe Pereira @ 2026-08-04 4:20 ` Wilson Felipe Pereira [not found] ` <CAO9r8zM9H8iMOE2FE-H47CMUAExf3VcCf04anT=_Ja=m=sbr4g@mail.gmail.com> 2026-08-04 4:20 ` [PATCH 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass Wilson Felipe Pereira 1 sibling, 1 reply; 5+ messages in thread From: Wilson Felipe Pereira @ 2026-08-04 4:20 UTC (permalink / raw) To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou, Tejun Heo, Michal Koutný, Shuah Khan Cc: linux-mm, cgroups, linux-kselftest, Wilson Felipe Pereira When running test_zswap on a single-core VM (-smp 1) with 4GB of RAM, test_zswap_writeback reliably fails on the initial run after boot. In test_zswap_writeback(), after waitpid() reaps the child process created by test_zswap_writeback_one(), writing "+memory" to cgroup.subtree_control can fail with -EBUSY. Under Cgroup v2, enabling domain subtree controllers is forbidden while any tasks remain in cgroup.procs. When a child process exits, waitpid() reaps the zombie PID immediately, but the removal of struct task_struct from the cgroup task list is performed asynchronously via an RCU callback (release_task). On single-core systems, this RCU callback is delayed behind CPU softirqs, causing "+memory" to fail if written immediately after waitpid() returns. Fix this by adding an EBUSY retry loop with usleep(1000) around cg_write(), matching the existing cgroup cleanup pattern in cg_destroy(). Signed-off-by: Wilson Felipe Pereira <wfelipe@google.com> --- tools/testing/selftests/cgroup/test_zswap.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c index 49b36ee791606..312d51887a1d8 100644 --- a/tools/testing/selftests/cgroup/test_zswap.c +++ b/tools/testing/selftests/cgroup/test_zswap.c @@ -407,8 +407,11 @@ static int test_zswap_writeback(const char *root, bool wb) * Thus, the parent's setting shall be what's in effect. */ if (cg_write(test_group, "memory.zswap.max", "max")) goto out; - if (cg_write(test_group, "cgroup.subtree_control", "+memory")) - goto out; + while (cg_write(test_group, "cgroup.subtree_control", "+memory")) { + if (errno != EBUSY) + goto out; + usleep(1000); + } test_group_child = cg_name(test_group, "zswap_writeback_test_child"); if (!test_group_child) -- 2.55.0.571.g244d577d93-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
[parent not found: <CAO9r8zM9H8iMOE2FE-H47CMUAExf3VcCf04anT=_Ja=m=sbr4g@mail.gmail.com>]
* Re: [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback [not found] ` <CAO9r8zM9H8iMOE2FE-H47CMUAExf3VcCf04anT=_Ja=m=sbr4g@mail.gmail.com> @ 2026-08-06 8:33 ` Michal Koutný 0 siblings, 0 replies; 5+ messages in thread From: Michal Koutný @ 2026-08-06 8:33 UTC (permalink / raw) To: Yosry Ahmed Cc: Wilson Felipe Pereira, Johannes Weiner, Nhat Pham, Chengming Zhou, Tejun Heo, Shuah Khan, linux-mm, cgroups, linux-kselftest [-- Attachment #1: Type: text/plain, Size: 1535 bytes --] On Mon, Aug 03, 2026 at 11:39:14PM -0700, Yosry Ahmed <yosry@kernel.org> wrote: > > diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c > > index 49b36ee791606..312d51887a1d8 100644 > > --- a/tools/testing/selftests/cgroup/test_zswap.c > > +++ b/tools/testing/selftests/cgroup/test_zswap.c > > @@ -407,8 +407,11 @@ static int test_zswap_writeback(const char *root, bool wb) > > * Thus, the parent's setting shall be what's in effect. */ > > if (cg_write(test_group, "memory.zswap.max", "max")) > > goto out; > > - if (cg_write(test_group, "cgroup.subtree_control", "+memory")) > > - goto out; > > + while (cg_write(test_group, "cgroup.subtree_control", "+memory")) { > > + if (errno != EBUSY) > > + goto out; > > + usleep(1000); > > + } > > I would honestly rather create a new cgroup here instead of reusing > the leaf cgroup as a parent, to avoid any subtleties like this in the > future, or even split test_zswap_writeback() into two test cases. I > don't feel strongly though if others think the simple wait here is > enough. I somewhat agree, I'm not sure whether the recycling brings any signal to the test. In the case, the recycling and (re-)enablement is needed, I'd suggest an explicit check for emptiness, the best of utils arsenal seems: cg_read_strcmp_wait(test_cgroup, "cgroup.events", "populated 0\n") Thanks, Michal [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 265 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass 2026-08-04 4:20 [PATCH 0/2] selftests/cgroup: test_zswap: fix single-core flakes Wilson Felipe Pereira 2026-08-04 4:20 ` [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback Wilson Felipe Pereira @ 2026-08-04 4:20 ` Wilson Felipe Pereira 2026-08-06 8:32 ` Michal Koutný 1 sibling, 1 reply; 5+ messages in thread From: Wilson Felipe Pereira @ 2026-08-04 4:20 UTC (permalink / raw) To: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou, Tejun Heo, Michal Koutný, Shuah Khan Cc: linux-mm, cgroups, linux-kselftest, Wilson Felipe Pereira In test_no_kmem_bypass(), delta (stored_pages * page_size - zswapped) is checked against stored_pages * page_size / 4 to verify that the pages pushed to zswap belong to the test memory cgroup. Due to slight stat update timing differences, delta can evaluate to a small negative number (e.g. -5MB out of 1GB). Because delta is declared as a signed int and stored_pages is an unsigned size_t, C's usual arithmetic conversions implicitly promote a negative delta to a large unsigned 64-bit integer, causing `delta < stored_pages * page_size / 4` to falsely evaluate to 0 and fail the test. Fix this by checking abs(delta), ensuring the test correctly compares the absolute difference between system zswap and cgroup zswapped bytes. Signed-off-by: Wilson Felipe Pereira <wfelipe@google.com> --- tools/testing/selftests/cgroup/test_zswap.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/cgroup/test_zswap.c b/tools/testing/selftests/cgroup/test_zswap.c index 312d51887a1d8..7bc2519862f7e 100644 --- a/tools/testing/selftests/cgroup/test_zswap.c +++ b/tools/testing/selftests/cgroup/test_zswap.c @@ -624,7 +624,8 @@ static int test_no_kmem_bypass(const char *root) if (stored_pages > stored_pages_threshold) { int zswapped = cg_read_key_long(test_group, "memory.stat", "zswapped "); int delta = stored_pages * page_size - zswapped; - int result_ok = delta < stored_pages * page_size / 4; + int result_ok = abs(delta) < + stored_pages * page_size / 4; ret = result_ok ? KSFT_PASS : KSFT_FAIL; break; -- 2.55.0.571.g244d577d93-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass 2026-08-04 4:20 ` [PATCH 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass Wilson Felipe Pereira @ 2026-08-06 8:32 ` Michal Koutný 0 siblings, 0 replies; 5+ messages in thread From: Michal Koutný @ 2026-08-06 8:32 UTC (permalink / raw) To: Wilson Felipe Pereira Cc: Johannes Weiner, Yosry Ahmed, Nhat Pham, Chengming Zhou, Tejun Heo, Shuah Khan, linux-mm, cgroups, linux-kselftest [-- Attachment #1: Type: text/plain, Size: 1316 bytes --] On Tue, Aug 04, 2026 at 04:20:45AM +0000, Wilson Felipe Pereira <wfelipe@google.com> wrote: > In test_no_kmem_bypass(), delta (stored_pages * page_size - zswapped) is > checked against stored_pages * page_size / 4 to verify that the pages > pushed to zswap belong to the test memory cgroup. > > Due to slight stat update timing differences, delta can evaluate to a small > negative number (e.g. -5MB out of 1GB). Because delta is declared as a > signed int and stored_pages is an unsigned size_t, C's usual arithmetic > conversions implicitly promote a negative delta to a large unsigned 64-bit > integer, causing `delta < stored_pages * page_size / 4` to falsely evaluate > to 0 and fail the test. Thanks for the breakdown. > Fix this by checking abs(delta), ensuring the test correctly compares the > absolute difference between system zswap and cgroup zswapped bytes. I still think the delta quantity has some meaning here and taking the abs() changes the semantics of the checked inequality. What about making both zswapped and delta `long`s? (Under similar reasoning, there's already a possible loss after squashing cg_read_key_long() result into the `int`.) In any way, also Fixes: a549f9f31561a ("selftests: cgroup: add test_zswap with no kmem bypass test") Thanks, Michal [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 265 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-06 8:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-04 4:20 [PATCH 0/2] selftests/cgroup: test_zswap: fix single-core flakes Wilson Felipe Pereira
2026-08-04 4:20 ` [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback Wilson Felipe Pereira
[not found] ` <CAO9r8zM9H8iMOE2FE-H47CMUAExf3VcCf04anT=_Ja=m=sbr4g@mail.gmail.com>
2026-08-06 8:33 ` Michal Koutný
2026-08-04 4:20 ` [PATCH 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass Wilson Felipe Pereira
2026-08-06 8:32 ` Michal Koutný
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox