From 46e0a0f97a5377986fa34ae8cdcef202f9ca33ba Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 12 Dec 2024 17:29:34 -0800 Subject: [PATCH 1/2] freeze_processes: fix nr_attempts calculation Commit 9fae23fbe2 grossly (by 1000x) miscalculated the number of attempts required, as a result, we are seeing something like this: > (00.000340) freezing processes: 100000 attempts with 100 ms steps > (00.000351) freezer.state=THAWED > (00.000358) freezer.state=FREEZING > (00.100446) freezer.state=FREEZING > ...close to 100 lines skipped... > (09.915110) freezer.state=FREEZING > (10.000432) Error (criu/cr-dump.c:1467): Timeout reached. Try to interrupt: 0 > (10.000563) freezer.state=FREEZING For 10s with 100ms steps we only need 100 attempts, not 100000. While at it, add an error print in case we hit the timeout earlier than i reaches nr_attempts. Signed-off-by: Kir Kolyshkin --- criu/seize.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/criu/seize.c b/criu/seize.c index 9bd1832d9b..7ade0b1fe8 100644 --- a/criu/seize.c +++ b/criu/seize.c @@ -542,7 +542,7 @@ static int freeze_processes(void) enum freezer_state state = THAWED; static const unsigned long step_ms = 100; - unsigned long nr_attempts = (opts.timeout * 1000000) / step_ms; + unsigned long nr_attempts = (opts.timeout * 1000) / step_ms; unsigned long i = 0; const struct timespec req = { @@ -555,7 +555,7 @@ static int freeze_processes(void) * If timeout is turned off, lets * wait for at least 10 seconds. */ - nr_attempts = (10 * 1000000) / step_ms; + nr_attempts = (10 * 1000) / step_ms; } pr_debug("freezing processes: %lu attempts with %lu ms steps\n", nr_attempts, step_ms); @@ -594,8 +594,10 @@ static int freeze_processes(void) if (state == FROZEN) break; - if (alarm_timeouted()) + if (alarm_timeouted()) { + pr_err("Unable to freeze cgroup %s (timed out)\n", opts.freeze_cgroup); goto err; + } nanosleep(&req, NULL); } From 2d3fb7bb4fe638cdeb18aa106ae14ac5a63ce32a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 12 Dec 2024 17:34:17 -0800 Subject: [PATCH 2/2] freeze_processes: implement kludges for cgroup v1 Cgroup v1 freezer has always been problematic, failing to freeze a cgroup. In runc, we have implemented a few kludges to increase the chance of succeeding, but those are used when runc freezes a cgroup for its own purposes (for "runc pause" and to modify device properties for cgroup v1). When criu is used, it fails to freeze a cgroup from time to time (see [1], [2]). Let's try adding kludges similar to ones in runc. Alas, I have absolutely no way to test this, so please review carefully. [1]: https://github.com/opencontainers/runc/issues/4273 [2]: https://github.com/opencontainers/runc/issues/4457 Signed-off-by: Kir Kolyshkin --- criu/seize.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/criu/seize.c b/criu/seize.c index 7ade0b1fe8..c9d23365d4 100644 --- a/criu/seize.c +++ b/criu/seize.c @@ -599,6 +599,34 @@ static int freeze_processes(void) goto err; } nanosleep(&req, NULL); + + if (cgroup_v2) + continue; + + /* As per older kernel docs (freezer-subsystem.txt before + * the kernel commit ef9fe980c6fcc1821), if FREEZING is seen, + * userspace should either retry or thaw. While current + * kernel cgroup v1 docs no longer mention a need to retry, + * even recent kernels can't reliably freeze a cgroup v1. + * + * Let's keep asking the kernel to freeze from time to time. + * In addition, do occasional thaw/sleep/freeze. + * + * This is still a game of chances (the real fix belongs to the kernel) + * but these kludges might improve the probability of success. + * + * Cgroup v2 does not have this problem. + */ + switch (i%32) { + case 30: + freezer_write_state(fd, THAWED); + break; + case 9: + case 20: + case 31: + freezer_write_state(fd, FROZEN); + break; + } } if (i > nr_attempts) {