* [PATCH] selftests/cgroup: Fix intermittent test_cgfreezer_ptrace test failures
@ 2026-07-14 18:31 Waiman Long
2026-07-14 22:09 ` Tejun Heo
0 siblings, 1 reply; 4+ messages in thread
From: Waiman Long @ 2026-07-14 18:31 UTC (permalink / raw)
To: Tejun Heo, Johannes Weiner, Michal Koutný, Shuah Khan
Cc: cgroups, linux-kernel, linux-kselftest, Waiman Long
It is found that the test_cgfreezer_ptrace test of test_freezer can
intermittently fail on some architectures like arm64 and ppc64.
After further tracing of the mechanics of the test_cgfreezer_ptrace test,
it is found that the ptrace(PTRACE_DETACH) call temporaily unfreezes the
cgroup and then freezes it again afterward in the detaching process. The
reading of the frozen flag from cgroup.events is done from a different
process running maybe on a different CPU. As a result, racing is possible
and the intermediate unfrozen state can be read leading to occasional test
failures especially on architectures with a weak memory model like arm64.
Fix that by adding a short 1 ms delay before reading the frozen state
to ensure that the final frozen value will be read.
By running test_freezer 100 times in a loop, there were 28
test_cgfreezer_ptrace failures out of 100 on an arm64 test system before
the patch. After applying the patch, there was no test failure at all
in 100 runs of test_freezer.
Signed-off-by: Waiman Long <longman@redhat.com>
---
tools/testing/selftests/cgroup/test_freezer.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/testing/selftests/cgroup/test_freezer.c b/tools/testing/selftests/cgroup/test_freezer.c
index 0569e93fa6b0..8e2cca74d212 100644
--- a/tools/testing/selftests/cgroup/test_freezer.c
+++ b/tools/testing/selftests/cgroup/test_freezer.c
@@ -625,6 +625,18 @@ static int test_cgfreezer_ptrace(const char *root)
if (ptrace(PTRACE_DETACH, pid, NULL, NULL))
goto cleanup;
+ /*
+ * The ptrace(PTRACE_DETACH) call will temporaily unfreeze the cgroup
+ * and then freeze it again afterward in the detaching process. The
+ * reading of the frozen flag from cgroup.events is done from a
+ * different process running maybe on a different CPU. As a result,
+ * racing is possible and the intermediate unfrozen state can be read
+ * leading to occasional test failure especially on architectures with
+ * a weak memory model like arm64. This intermittent test failure can
+ * be avoided by adding a 1ms short delay before reading the frozen
+ * state.
+ */
+ usleep(1000);
if (cg_check_frozen(cgroup, true))
goto cleanup;
--
2.55.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] selftests/cgroup: Fix intermittent test_cgfreezer_ptrace test failures
2026-07-14 18:31 [PATCH] selftests/cgroup: Fix intermittent test_cgfreezer_ptrace test failures Waiman Long
@ 2026-07-14 22:09 ` Tejun Heo
2026-07-15 1:24 ` Tao Cui
2026-07-15 1:25 ` Waiman Long
0 siblings, 2 replies; 4+ messages in thread
From: Tejun Heo @ 2026-07-14 22:09 UTC (permalink / raw)
To: Waiman Long
Cc: Johannes Weiner, Michal Koutný, Shuah Khan, cgroups,
linux-kernel, linux-kselftest
Hello, Waiman.
> + usleep(1000);
> if (cg_check_frozen(cgroup, true))
> goto cleanup;
A fixed 1ms sleep only hides the race. On a loaded machine or a slow arch
(you mention ppc64) the refreeze can take longer than 1ms, and the test
reads the unfrozen state and fails anyway.
The freezer test already has a way to wait for this properly.
cg_prepare_for_wait() sets up an inotify watch on cgroup.events and
cg_wait_for() blocks until it changes. cg_enter_and_wait_for_frozen() shows
the pattern: loop cg_wait_for() then cg_check_frozen(). The cgroup always
ends up frozen, so looping until cg_check_frozen() reports frozen is
reliable and doesn't depend on timing.
Can you respin using that instead of usleep()?
Also, temporaily -> temporarily, in both the changelog and the comment.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] selftests/cgroup: Fix intermittent test_cgfreezer_ptrace test failures
2026-07-14 22:09 ` Tejun Heo
@ 2026-07-15 1:24 ` Tao Cui
2026-07-15 1:25 ` Waiman Long
1 sibling, 0 replies; 4+ messages in thread
From: Tao Cui @ 2026-07-15 1:24 UTC (permalink / raw)
To: Tejun Heo, Waiman Long
Cc: cui.tao, Johannes Weiner, Michal Koutný, Shuah Khan, cgroups,
linux-kernel, linux-kselftest
在 2026/7/15 06:09, Tejun Heo 写道:
> Hello, Waiman.
>
>> + usleep(1000);
>> if (cg_check_frozen(cgroup, true))
>> goto cleanup;
>
> A fixed 1ms sleep only hides the race. On a loaded machine or a slow arch
> (you mention ppc64) the refreeze can take longer than 1ms, and the test
> reads the unfrozen state and fails anyway.
>
> The freezer test already has a way to wait for this properly.
> cg_prepare_for_wait() sets up an inotify watch on cgroup.events and
> cg_wait_for() blocks until it changes. cg_enter_and_wait_for_frozen() shows
> the pattern: loop cg_wait_for() then cg_check_frozen(). The cgroup always
> ends up frozen, so looping until cg_check_frozen() reports frozen is
> reliable and doesn't depend on timing.
>
> Can you respin using that instead of usleep()?
>
> Also, temporaily -> temporarily, in both the changelog and the comment.
>
Hi Tejun, Waiman,
I ran into a similar issue a while back and can add a data point on
the reproducibility: when running the full cgroup selftest suite, a
few cases -- test_cgfreezer_ptrace and test_cgfreezer_stopped -- fail
intermittently (around 40-70% on VMs), yet each failing case passes
reliably when run on its own. That points at state carried across
tests rather than a per-test bug, which is also why a fixed sleep/retry
is fragile.
On the "unfrozen state" above, I traced where CGRP_FROZEN actually gets
cleared. When the frozen tracee is woken by PTRACE_INTERRUPT,
JOBCTL_TRAP_STOP takes priority over JOBCTL_TRAP_FREEZE in get_signal(),
so the task enters ptrace_stop() instead of going back to
do_freezer_trap(). With debug printk in cgroup_update_frozen_flag() and
cgroup_leave_frozen(), the cg_test_ptrace trace shows:
0 -> 1 nr_frozen=1 task_count=1 /* initial freeze */
1 -> 0 nr_frozen=0 task_count=1 /* dec, task still in cgroup */
0 -> 1 nr_frozen=0 task_count=0 /* task left cgroup */
The 1 -> 0 step (nr_frozen 1->0 while task_count is still 1) is
cgroup_dec_frozen_cnt(), called from cgroup_leave_frozen(true) at the
end of ptrace_stop() (signal.c:2479) when the tracee is woken by
PTRACE_DETACH. That clears CGRP_FROZEN until the task loops back into
do_freezer_trap() and re-enters the frozen state -- the transient
unfrozen window the test hits.
As a kernel-side attempt I changed cgroup_enter_frozen() so it no longer
returns early when current->frozen is already true: css_set_lock is
taken before the check and, if CGRP_FREEZE is still set,
cgroup_update_frozen() is called to re-verify the cgroup frozen state
when a frozen task is handed off to ptrace.
Due to other work I've had to pause this investigation for now, so I'm
sharing the above as a data point rather than a finished fix.
Thanks,
Tao
> Thanks.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] selftests/cgroup: Fix intermittent test_cgfreezer_ptrace test failures
2026-07-14 22:09 ` Tejun Heo
2026-07-15 1:24 ` Tao Cui
@ 2026-07-15 1:25 ` Waiman Long
1 sibling, 0 replies; 4+ messages in thread
From: Waiman Long @ 2026-07-15 1:25 UTC (permalink / raw)
To: Tejun Heo
Cc: Johannes Weiner, Michal Koutný, Shuah Khan, cgroups,
linux-kernel, linux-kselftest
On 7/14/26 6:09 PM, Tejun Heo wrote:
> Hello, Waiman.
>
>> + usleep(1000);
>> if (cg_check_frozen(cgroup, true))
>> goto cleanup;
> A fixed 1ms sleep only hides the race. On a loaded machine or a slow arch
> (you mention ppc64) the refreeze can take longer than 1ms, and the test
> reads the unfrozen state and fails anyway.
>
> The freezer test already has a way to wait for this properly.
> cg_prepare_for_wait() sets up an inotify watch on cgroup.events and
> cg_wait_for() blocks until it changes. cg_enter_and_wait_for_frozen() shows
> the pattern: loop cg_wait_for() then cg_check_frozen(). The cgroup always
> ends up frozen, so looping until cg_check_frozen() reports frozen is
> reliable and doesn't depend on timing.
>
> Can you respin using that instead of usleep()?
>
> Also, temporaily -> temporarily, in both the changelog and the comment.
>
> Thanks.
Thanks for the suggestion. Will revise the patch as suggested.
Cheers,
Longman
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-15 1:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 18:31 [PATCH] selftests/cgroup: Fix intermittent test_cgfreezer_ptrace test failures Waiman Long
2026-07-14 22:09 ` Tejun Heo
2026-07-15 1:24 ` Tao Cui
2026-07-15 1:25 ` Waiman Long
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox