* [PATCH] pid_namespace: make zap_pid_ns_processes() freezable
@ 2026-10-01 12:12 Aviv Vaknin
2026-10-01 13:03 ` Rafael J. Wysocki (Intel)
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Aviv Vaknin @ 2026-10-01 12:12 UTC (permalink / raw)
To: Christian Brauner, Oleg Nesterov
Cc: Rafael J. Wysocki, Pavel Tikhomirov, Eric W. Biederman, linux-pm,
linux-kernel, stable
When the init of a pid namespace exits, zap_pid_ns_processes() waits
until every other pid in the namespace has been freed. That wait has no
upper bound: a tracer outside the namespace that doesn't reap its traced
zombies keeps their pids allocated (see the comment above the final
loop). While a task waits there, every suspend attempt fails:
Freezing user space processes failed after 20.006 seconds (2 tasks refusing to freeze, wq_busy=0):
task:pidns-freeze-re state:S stack:14200 pid:104 tgid:104 ppid:99 task_flags:0x40014c flags:0x00080000
task:pidns-freeze-re state:S stack:14160 pid:109 tgid:105 ppid:104 task_flags:0x40044c flags:0x00080802
and keeps failing until the tracer reaps or exits, because neither wait
can be frozen:
- The first loop sleeps in kernel_wait4(). The freezer's fake signal
ends that wait with -ERESTARTSYS, then the loop clears TIF_SIGPENDING
and waits again without ever calling try_to_freeze().
- The final loop sleeps in TASK_INTERRUPTIBLE without TASK_FREEZABLE.
Worse, nothing clears the TIF_SIGPENDING that the fake signal set, so
from the first suspend attempt on schedule() returns at once and the
task spins at 100% CPU for as long as the wait lasts.
Neither loop has ever been freezable. The spin came with
commit b9a985db9896 ("pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns_processes"),
which replaced TASK_UNINTERRUPTIBLE to keep such long waits from
triggering the hung task detector.
This happens in practice with Chromium-based browsers, whose renderers
are pid namespace inits. If a renderer crashes while the browser shuts
down, crashpad (outside the namespace) ptrace-attaches its threads to
dump it. When the renderer is SIGKILLed mid-dump, crashpad keeps the
dead threads as unreaped traced zombies, and it can then block in
waitpid() on the renderer's last thread, which is itself waiting in
zap_pid_ns_processes() for those zombies. One such renderer blocks
suspend system-wide until crashpad is killed or the machine reboots.
The userspace side is being reported to Chromium separately.
Call try_to_freeze() after kernel_wait4() in the first loop, and sleep
in TASK_IDLE | TASK_FREEZABLE in the final loop, as coredump_task_exit()
already does on the same exit path. Like TASK_INTERRUPTIBLE, TASK_IDLE
keeps the hung task detector quiet and adds no load, but it is not
woken by signals, so a pending signal can no longer turn the wait into
a busy loop. TASK_FREEZABLE lets the freezer freeze the task in place;
a free_pid() wakeup that arrives while it is frozen is kept in
->saved_state and acted on at thaw.
Tested on v7.3-rc5 in QEMU with PROVE_LOCKING, DEBUG_ATOMIC_SLEEP and
DETECT_HUNG_TASK (10 s timeout), using a reproducer in which a tracer
PTRACE_SEIZEs two threads of a nested pid namespace init and doesn't
reap them (it works unprivileged too, from a user namespace), and using
the real browser and crashpad trigger:
- before: freezing fails after 20 s, and afterwards the final loop
spins (500 ticks of system time per 5 s).
- after: freezing completes in 0.001 s, the waiting task sits in state
I without using CPU, no hung task or lockdep report appears over
30-45 s, and the task exits as soon as the zombies are reaped.
Fixes: 3eb07c8c8adb ("pid namespaces: destroy pid namespace on init's death")
Fixes: b9a985db9896 ("pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns_processes")
Cc: stable@vger.kernel.org # 6.1+
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Aviv Vaknin <vaknins33@gmail.com>
---
Based on vfs kernel-7.4.signal (also in next-20260930), on top of
5c0985dd3c32 ("pid_namespace: prevent TIF_NOTIFY_SIGNAL from
interrupting the reaper"). That commit keeps TIF_NOTIFY_SIGNAL from
waking the reaper; this one is about the freezer, whose fake signal
sets TIF_SIGPENDING, which the guard doesn't cover. Retested on this
base with the reproducer below (same debug config): freezing completes
in 0.000 s, no spin, no warnings. A version for v7.3-rc5 is available
if wanted.
Reproducer (run as root in a VM, then follow its instructions):
// SPDX-License-Identifier: GPL-2.0
/*
* pidns-freeze-repro: a task waiting in zap_pid_ns_processes() blocks suspend.
*
* main (tracer, root pid ns)
* `- A: pid 1 of pid ns A, single-threaded
* `- B: pid 1 of nested pid ns B, multithreaded
*
* main PTRACE_SEIZEs non-leader threads of B, then A exits. zap(A) kills B;
* the seized threads stay traced zombies until main reaps them, keeping ns B's
* pids allocated. B's last thread waits in zap's final loop, A in zap's first
* loop (kernel_wait4() for B). While "stuck" is shown, run as root:
* echo freezer > /sys/power/pm_test; echo mem > /sys/power/state
* Unpatched, freezing fails after 20 s ("2 tasks refusing to freeze") and B's
* thread then busy-loops in zap. At the end main reaps, ending the hang.
*
* Build: gcc -O2 -Wall -pthread -o pidns-freeze-repro pidns-freeze-repro.c
* Usage: ./pidns-freeze-repro [hold_seconds] (root, in a VM; default: Enter)
* With -DUSERNS, A also gets a new user ns, so no privilege is needed.
*/
#define _GNU_SOURCE
#include <dirent.h>
#include <pthread.h>
#include <sched.h>
#include <signal.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ptrace.h>
#include <sys/wait.h>
#include <unistd.h>
#ifdef USERNS
#define NS_FLAGS (CLONE_NEWUSER | CLONE_NEWPID)
#else
#define NS_FLAGS CLONE_NEWPID
#endif
#define NTHREADS 4 /* B's threads besides its leader */
#define NSEIZE 2 /* how many of them main seizes */
static int ready[2], go[2]; /* B -> main: threads are up; main -> A: exit */
static char stack_a[256 << 10], stack_b[256 << 10];
static void *idle_thread(void *arg) { for (;;) pause(); return arg; }
static int init_b(void *arg) /* pid 1 of ns B */
{
pthread_t t;
for (int i = 0; i < NTHREADS; i++)
pthread_create(&t, NULL, idle_thread, arg);
if (write(ready[1], "r", 1) != 1)
_exit(1);
for (;;) pause();
}
static int init_a(void *arg) /* pid 1 of ns A */
{
close(go[1]); /* only main may hold the write end */
if (clone(init_b, stack_b + sizeof(stack_b), CLONE_NEWPID | SIGCHLD, arg) < 0)
_exit(1);
while (read(go[0], &arg, 1) > 0) /* EOF: main has seized */
;
_exit(0); /* -> zap_pid_ns_processes(A) */
}
/* Read a small /proc file into buf ("" if it is gone). */
static char *slurp(char *buf, size_t len, const char *fmt, ...)
{
char path[300];
va_list ap;
FILE *f;
va_start(ap, fmt);
vsnprintf(path, sizeof(path), fmt, ap);
va_end(ap);
buf[0] = 0;
if ((f = fopen(path, "r"))) {
buf[fread(buf, 1, len - 1, f)] = 0;
fclose(f);
}
return buf;
}
/* Global pid of A's child B: the process whose PPid is A. */
static pid_t child_of(pid_t parent)
{
DIR *d = opendir("/proc");
struct dirent *e;
char buf[512], *p;
int ppid;
while (d && (e = readdir(d))) {
p = strrchr(slurp(buf, sizeof(buf), "/proc/%s/stat", e->d_name), ')');
if (p && sscanf(p + 2, "%*c %d", &ppid) == 1 && ppid == parent)
return atoi(e->d_name); /* (leaks d; fine here) */
}
return -1;
}
/* Print a task's state and wchan; return whether it is exiting (PF_EXITING). */
static int show(const char *what, pid_t pid, pid_t tid)
{
char st[512], wchan[64], *p, c = '?';
unsigned int flags = 0;
p = strrchr(slurp(st, sizeof(st), "/proc/%d/task/%d/stat", pid, tid), ')');
if (p)
sscanf(p + 2, "%c %*d %*d %*d %*d %*d %u", &c, &flags);
slurp(wchan, sizeof(wchan), "/proc/%d/task/%d/wchan", pid, tid);
printf(" %s tid %d: state %c, flags %#x, wchan %s\n", what, tid, c, flags, wchan);
return flags & 0x4;
}
int main(int argc, char **argv)
{
pid_t a, b, tid, seized[NSEIZE];
int n = 0, status;
struct dirent *e;
char buf[64];
DIR *d;
if (pipe(ready) || pipe(go))
return perror("pipe"), 1;
if ((a = clone(init_a, stack_a + sizeof(stack_a), NS_FLAGS | SIGCHLD, NULL)) < 0)
return perror("clone (run as root)"), 1;
if (read(ready[0], buf, 1) != 1 || (b = child_of(a)) < 0)
return fprintf(stderr, "setup failed\n"), 1;
/* From outside both namespaces, seize non-leader threads of B. */
snprintf(buf, sizeof(buf), "/proc/%d/task", b);
for (d = opendir(buf); d && (e = readdir(d)) && n < NSEIZE; )
if ((tid = atoi(e->d_name)) > 0 && tid != b && !ptrace(PTRACE_SEIZE, tid, 0, 0))
seized[n++] = tid;
printf("A=%d B=%d: seized %d threads of B\n", a, b, n);
close(go[1]); /* A exits now */
sleep(2);
if (!n || waitpid(a, &status, WNOHANG) != 0 || !show("A", a, a))
return printf("not reproduced\n"), 1;
for (rewinddir(d); (e = readdir(d)); )
if ((tid = atoi(e->d_name)) > 0)
show("B", b, tid);
printf("stuck: A and B can't finish exiting until main reaps its %d traced threads\n", n);
printf("now run: echo freezer > /sys/power/pm_test; echo mem > /sys/power/state\n");
fflush(stdout);
if (argc > 1)
sleep(atoi(argv[1]));
else
getchar();
/* Reap the traced zombies: ns B empties, then B and A finish exiting. */
for (int i = 0, left = n; left && i < 500; i++, usleep(10000))
for (int k = 0; k < n; k++)
if (seized[k] && waitpid(seized[k], &status, __WALL | WNOHANG) == seized[k])
seized[k] = 0, left--;
for (int i = 0; i < 500; i++, usleep(10000))
if (waitpid(a, &status, WNOHANG) == a)
return printf("reaped the zombies: hang resolved\n"), 0;
printf("still stuck after reaping\n");
return 2;
}
kernel/pid_namespace.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 8bc9edb40..abdc05462 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -23,6 +23,7 @@
#include <linux/sched/task.h>
#include <linux/sched/signal.h>
#include <linux/idr.h>
+#include <linux/freezer.h>
#include <linux/nstree.h>
#include <uapi/linux/wait.h>
#include "pid_sysctl.h"
@@ -243,6 +244,11 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
do {
clear_thread_flag(TIF_SIGPENDING);
rc = kernel_wait4(-1, NULL, __WALL, NULL);
+ /*
+ * The freezer's fake signal ends the wait with -ERESTARTSYS;
+ * freeze here, or one stuck pid namespace blocks suspend.
+ */
+ try_to_freeze();
} while (rc != -ECHILD);
/*
@@ -269,7 +275,13 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
* free_pid() will awaken this task.
*/
for (;;) {
- set_current_state(TASK_INTERRUPTIBLE);
+ /*
+ * TASK_IDLE: no hung task warning or load for a wait that can
+ * last as long as a tracer keeps a zombie, and a pending signal
+ * (e.g. the freezer's fake one) can't turn it into a busy loop.
+ * TASK_FREEZABLE: let the freezer freeze us while we wait.
+ */
+ set_current_state(TASK_IDLE | TASK_FREEZABLE);
if (pid_ns->pid_allocated == init_pids)
break;
schedule();
base-commit: ed14a591175bb5f56c2936b082cffb9e4b935e6d
--
2.55.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pid_namespace: make zap_pid_ns_processes() freezable
2026-10-01 12:12 [PATCH] pid_namespace: make zap_pid_ns_processes() freezable Aviv Vaknin
@ 2026-10-01 13:03 ` Rafael J. Wysocki (Intel)
2026-10-01 15:35 ` Oleg Nesterov
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-10-01 13:03 UTC (permalink / raw)
To: Aviv Vaknin
Cc: Christian Brauner, Oleg Nesterov, Rafael J. Wysocki,
Pavel Tikhomirov, Eric W. Biederman, linux-pm, linux-kernel,
stable
On Thu, Oct 1, 2026 at 2:13 PM Aviv Vaknin <vaknins33@gmail.com> wrote:
>
> When the init of a pid namespace exits, zap_pid_ns_processes() waits
> until every other pid in the namespace has been freed. That wait has no
> upper bound: a tracer outside the namespace that doesn't reap its traced
> zombies keeps their pids allocated (see the comment above the final
> loop). While a task waits there, every suspend attempt fails:
>
> Freezing user space processes failed after 20.006 seconds (2 tasks refusing to freeze, wq_busy=0):
> task:pidns-freeze-re state:S stack:14200 pid:104 tgid:104 ppid:99 task_flags:0x40014c flags:0x00080000
> task:pidns-freeze-re state:S stack:14160 pid:109 tgid:105 ppid:104 task_flags:0x40044c flags:0x00080802
>
> and keeps failing until the tracer reaps or exits, because neither wait
> can be frozen:
>
> - The first loop sleeps in kernel_wait4(). The freezer's fake signal
> ends that wait with -ERESTARTSYS, then the loop clears TIF_SIGPENDING
> and waits again without ever calling try_to_freeze().
>
> - The final loop sleeps in TASK_INTERRUPTIBLE without TASK_FREEZABLE.
> Worse, nothing clears the TIF_SIGPENDING that the fake signal set, so
> from the first suspend attempt on schedule() returns at once and the
> task spins at 100% CPU for as long as the wait lasts.
>
> Neither loop has ever been freezable. The spin came with
> commit b9a985db9896 ("pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns_processes"),
> which replaced TASK_UNINTERRUPTIBLE to keep such long waits from
> triggering the hung task detector.
>
> This happens in practice with Chromium-based browsers, whose renderers
> are pid namespace inits. If a renderer crashes while the browser shuts
> down, crashpad (outside the namespace) ptrace-attaches its threads to
> dump it. When the renderer is SIGKILLed mid-dump, crashpad keeps the
> dead threads as unreaped traced zombies, and it can then block in
> waitpid() on the renderer's last thread, which is itself waiting in
> zap_pid_ns_processes() for those zombies. One such renderer blocks
> suspend system-wide until crashpad is killed or the machine reboots.
> The userspace side is being reported to Chromium separately.
>
> Call try_to_freeze() after kernel_wait4() in the first loop, and sleep
> in TASK_IDLE | TASK_FREEZABLE in the final loop, as coredump_task_exit()
> already does on the same exit path. Like TASK_INTERRUPTIBLE, TASK_IDLE
> keeps the hung task detector quiet and adds no load, but it is not
> woken by signals, so a pending signal can no longer turn the wait into
> a busy loop. TASK_FREEZABLE lets the freezer freeze the task in place;
> a free_pid() wakeup that arrives while it is frozen is kept in
> ->saved_state and acted on at thaw.
>
> Tested on v7.3-rc5 in QEMU with PROVE_LOCKING, DEBUG_ATOMIC_SLEEP and
> DETECT_HUNG_TASK (10 s timeout), using a reproducer in which a tracer
> PTRACE_SEIZEs two threads of a nested pid namespace init and doesn't
> reap them (it works unprivileged too, from a user namespace), and using
> the real browser and crashpad trigger:
>
> - before: freezing fails after 20 s, and afterwards the final loop
> spins (500 ticks of system time per 5 s).
> - after: freezing completes in 0.001 s, the waiting task sits in state
> I without using CPU, no hung task or lockdep report appears over
> 30-45 s, and the task exits as soon as the zombies are reaped.
>
> Fixes: 3eb07c8c8adb ("pid namespaces: destroy pid namespace on init's death")
> Fixes: b9a985db9896 ("pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns_processes")
> Cc: stable@vger.kernel.org # 6.1+
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Aviv Vaknin <vaknins33@gmail.com>
Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
> ---
> Based on vfs kernel-7.4.signal (also in next-20260930), on top of
> 5c0985dd3c32 ("pid_namespace: prevent TIF_NOTIFY_SIGNAL from
> interrupting the reaper"). That commit keeps TIF_NOTIFY_SIGNAL from
> waking the reaper; this one is about the freezer, whose fake signal
> sets TIF_SIGPENDING, which the guard doesn't cover. Retested on this
> base with the reproducer below (same debug config): freezing completes
> in 0.000 s, no spin, no warnings. A version for v7.3-rc5 is available
> if wanted.
>
> Reproducer (run as root in a VM, then follow its instructions):
>
> // SPDX-License-Identifier: GPL-2.0
> /*
> * pidns-freeze-repro: a task waiting in zap_pid_ns_processes() blocks suspend.
> *
> * main (tracer, root pid ns)
> * `- A: pid 1 of pid ns A, single-threaded
> * `- B: pid 1 of nested pid ns B, multithreaded
> *
> * main PTRACE_SEIZEs non-leader threads of B, then A exits. zap(A) kills B;
> * the seized threads stay traced zombies until main reaps them, keeping ns B's
> * pids allocated. B's last thread waits in zap's final loop, A in zap's first
> * loop (kernel_wait4() for B). While "stuck" is shown, run as root:
> * echo freezer > /sys/power/pm_test; echo mem > /sys/power/state
> * Unpatched, freezing fails after 20 s ("2 tasks refusing to freeze") and B's
> * thread then busy-loops in zap. At the end main reaps, ending the hang.
> *
> * Build: gcc -O2 -Wall -pthread -o pidns-freeze-repro pidns-freeze-repro.c
> * Usage: ./pidns-freeze-repro [hold_seconds] (root, in a VM; default: Enter)
> * With -DUSERNS, A also gets a new user ns, so no privilege is needed.
> */
> #define _GNU_SOURCE
> #include <dirent.h>
> #include <pthread.h>
> #include <sched.h>
> #include <signal.h>
> #include <stdarg.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/ptrace.h>
> #include <sys/wait.h>
> #include <unistd.h>
>
> #ifdef USERNS
> #define NS_FLAGS (CLONE_NEWUSER | CLONE_NEWPID)
> #else
> #define NS_FLAGS CLONE_NEWPID
> #endif
> #define NTHREADS 4 /* B's threads besides its leader */
> #define NSEIZE 2 /* how many of them main seizes */
>
> static int ready[2], go[2]; /* B -> main: threads are up; main -> A: exit */
> static char stack_a[256 << 10], stack_b[256 << 10];
>
> static void *idle_thread(void *arg) { for (;;) pause(); return arg; }
>
> static int init_b(void *arg) /* pid 1 of ns B */
> {
> pthread_t t;
>
> for (int i = 0; i < NTHREADS; i++)
> pthread_create(&t, NULL, idle_thread, arg);
> if (write(ready[1], "r", 1) != 1)
> _exit(1);
> for (;;) pause();
> }
>
> static int init_a(void *arg) /* pid 1 of ns A */
> {
> close(go[1]); /* only main may hold the write end */
> if (clone(init_b, stack_b + sizeof(stack_b), CLONE_NEWPID | SIGCHLD, arg) < 0)
> _exit(1);
> while (read(go[0], &arg, 1) > 0) /* EOF: main has seized */
> ;
> _exit(0); /* -> zap_pid_ns_processes(A) */
> }
>
> /* Read a small /proc file into buf ("" if it is gone). */
> static char *slurp(char *buf, size_t len, const char *fmt, ...)
> {
> char path[300];
> va_list ap;
> FILE *f;
>
> va_start(ap, fmt);
> vsnprintf(path, sizeof(path), fmt, ap);
> va_end(ap);
> buf[0] = 0;
> if ((f = fopen(path, "r"))) {
> buf[fread(buf, 1, len - 1, f)] = 0;
> fclose(f);
> }
> return buf;
> }
>
> /* Global pid of A's child B: the process whose PPid is A. */
> static pid_t child_of(pid_t parent)
> {
> DIR *d = opendir("/proc");
> struct dirent *e;
> char buf[512], *p;
> int ppid;
>
> while (d && (e = readdir(d))) {
> p = strrchr(slurp(buf, sizeof(buf), "/proc/%s/stat", e->d_name), ')');
> if (p && sscanf(p + 2, "%*c %d", &ppid) == 1 && ppid == parent)
> return atoi(e->d_name); /* (leaks d; fine here) */
> }
> return -1;
> }
>
> /* Print a task's state and wchan; return whether it is exiting (PF_EXITING). */
> static int show(const char *what, pid_t pid, pid_t tid)
> {
> char st[512], wchan[64], *p, c = '?';
> unsigned int flags = 0;
>
> p = strrchr(slurp(st, sizeof(st), "/proc/%d/task/%d/stat", pid, tid), ')');
> if (p)
> sscanf(p + 2, "%c %*d %*d %*d %*d %*d %u", &c, &flags);
> slurp(wchan, sizeof(wchan), "/proc/%d/task/%d/wchan", pid, tid);
> printf(" %s tid %d: state %c, flags %#x, wchan %s\n", what, tid, c, flags, wchan);
> return flags & 0x4;
> }
>
> int main(int argc, char **argv)
> {
> pid_t a, b, tid, seized[NSEIZE];
> int n = 0, status;
> struct dirent *e;
> char buf[64];
> DIR *d;
>
> if (pipe(ready) || pipe(go))
> return perror("pipe"), 1;
> if ((a = clone(init_a, stack_a + sizeof(stack_a), NS_FLAGS | SIGCHLD, NULL)) < 0)
> return perror("clone (run as root)"), 1;
> if (read(ready[0], buf, 1) != 1 || (b = child_of(a)) < 0)
> return fprintf(stderr, "setup failed\n"), 1;
>
> /* From outside both namespaces, seize non-leader threads of B. */
> snprintf(buf, sizeof(buf), "/proc/%d/task", b);
> for (d = opendir(buf); d && (e = readdir(d)) && n < NSEIZE; )
> if ((tid = atoi(e->d_name)) > 0 && tid != b && !ptrace(PTRACE_SEIZE, tid, 0, 0))
> seized[n++] = tid;
> printf("A=%d B=%d: seized %d threads of B\n", a, b, n);
>
> close(go[1]); /* A exits now */
> sleep(2);
> if (!n || waitpid(a, &status, WNOHANG) != 0 || !show("A", a, a))
> return printf("not reproduced\n"), 1;
> for (rewinddir(d); (e = readdir(d)); )
> if ((tid = atoi(e->d_name)) > 0)
> show("B", b, tid);
> printf("stuck: A and B can't finish exiting until main reaps its %d traced threads\n", n);
> printf("now run: echo freezer > /sys/power/pm_test; echo mem > /sys/power/state\n");
> fflush(stdout);
> if (argc > 1)
> sleep(atoi(argv[1]));
> else
> getchar();
>
> /* Reap the traced zombies: ns B empties, then B and A finish exiting. */
> for (int i = 0, left = n; left && i < 500; i++, usleep(10000))
> for (int k = 0; k < n; k++)
> if (seized[k] && waitpid(seized[k], &status, __WALL | WNOHANG) == seized[k])
> seized[k] = 0, left--;
> for (int i = 0; i < 500; i++, usleep(10000))
> if (waitpid(a, &status, WNOHANG) == a)
> return printf("reaped the zombies: hang resolved\n"), 0;
> printf("still stuck after reaping\n");
> return 2;
> }
>
> kernel/pid_namespace.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> index 8bc9edb40..abdc05462 100644
> --- a/kernel/pid_namespace.c
> +++ b/kernel/pid_namespace.c
> @@ -23,6 +23,7 @@
> #include <linux/sched/task.h>
> #include <linux/sched/signal.h>
> #include <linux/idr.h>
> +#include <linux/freezer.h>
> #include <linux/nstree.h>
> #include <uapi/linux/wait.h>
> #include "pid_sysctl.h"
> @@ -243,6 +244,11 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
> do {
> clear_thread_flag(TIF_SIGPENDING);
> rc = kernel_wait4(-1, NULL, __WALL, NULL);
> + /*
> + * The freezer's fake signal ends the wait with -ERESTARTSYS;
> + * freeze here, or one stuck pid namespace blocks suspend.
> + */
> + try_to_freeze();
> } while (rc != -ECHILD);
>
> /*
> @@ -269,7 +275,13 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
> * free_pid() will awaken this task.
> */
> for (;;) {
> - set_current_state(TASK_INTERRUPTIBLE);
> + /*
> + * TASK_IDLE: no hung task warning or load for a wait that can
> + * last as long as a tracer keeps a zombie, and a pending signal
> + * (e.g. the freezer's fake one) can't turn it into a busy loop.
> + * TASK_FREEZABLE: let the freezer freeze us while we wait.
> + */
> + set_current_state(TASK_IDLE | TASK_FREEZABLE);
> if (pid_ns->pid_allocated == init_pids)
> break;
> schedule();
>
> base-commit: ed14a591175bb5f56c2936b082cffb9e4b935e6d
> --
> 2.55.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pid_namespace: make zap_pid_ns_processes() freezable
2026-10-01 12:12 [PATCH] pid_namespace: make zap_pid_ns_processes() freezable Aviv Vaknin
2026-10-01 13:03 ` Rafael J. Wysocki (Intel)
@ 2026-10-01 15:35 ` Oleg Nesterov
2026-10-01 16:53 ` Bradley Morgan
2026-10-02 14:26 ` Oleg Nesterov
3 siblings, 0 replies; 9+ messages in thread
From: Oleg Nesterov @ 2026-10-01 15:35 UTC (permalink / raw)
To: Aviv Vaknin
Cc: Christian Brauner, Rafael J. Wysocki, Pavel Tikhomirov,
Eric W. Biederman, linux-pm, linux-kernel, stable
Hi Aviv,
Thanks, at first glance your patch makes sense... but for some reasons
I can't read the source code today and perhaps tomorrow.
let me ask a couple of questions right now.
On 10/01, Aviv Vaknin wrote:
>
> @@ -243,6 +244,11 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
> do {
> clear_thread_flag(TIF_SIGPENDING);
> rc = kernel_wait4(-1, NULL, __WALL, NULL);
> + /*
> + * The freezer's fake signal ends the wait with -ERESTARTSYS;
> + * freeze here, or one stuck pid namespace blocks suspend.
> + */
> + try_to_freeze();
Well, I don't really like this another callsite of try_to_freeze(). Perhaps
we can change the kernel_wait4() paths to use schedule(INTERRUPTIBLE | FREEZABLE)
instead? Not sure, please recheck...
> for (;;) {
> - set_current_state(TASK_INTERRUPTIBLE);
> + /*
> + * TASK_IDLE: no hung task warning or load for a wait that can
> + * last as long as a tracer keeps a zombie, and a pending signal
> + * (e.g. the freezer's fake one) can't turn it into a busy loop.
> + * TASK_FREEZABLE: let the freezer freeze us while we wait.
> + */
> + set_current_state(TASK_IDLE | TASK_FREEZABLE);
> if (pid_ns->pid_allocated == init_pids)
> break;
> schedule();
Looks "obviously good", but I need to recall why this code uses TASK_INTERRUPTIBLE,
not TASK_IDLE.
Oleg.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pid_namespace: make zap_pid_ns_processes() freezable
2026-10-01 12:12 [PATCH] pid_namespace: make zap_pid_ns_processes() freezable Aviv Vaknin
2026-10-01 13:03 ` Rafael J. Wysocki (Intel)
2026-10-01 15:35 ` Oleg Nesterov
@ 2026-10-01 16:53 ` Bradley Morgan
2026-10-02 14:26 ` Oleg Nesterov
3 siblings, 0 replies; 9+ messages in thread
From: Bradley Morgan @ 2026-10-01 16:53 UTC (permalink / raw)
To: vaknins33
Cc: brauner, ebiederm, linux-kernel, linux-pm, oleg, ptikhomirov,
rafael, stable
On 1 October 2026 13:12:54 BST, Aviv Vaknin <vaknins33@gmail.com> wrote:
>When the init of a pid namespace exits, zap_pid_ns_processes() waits
>until every other pid in the namespace has been freed. That wait has no
>upper bound: a tracer outside the namespace that doesn't reap its traced
>zombies keeps their pids allocated (see the comment above the final
>loop). While a task waits there, every suspend attempt fails:
>
> Freezing user space processes failed after 20.006 seconds (2 tasks refusing to freeze, wq_busy=0):
> task:pidns-freeze-re state:S stack:14200 pid:104 tgid:104 ppid:99 task_flags:0x40014c flags:0x00080000
> task:pidns-freeze-re state:S stack:14160 pid:109 tgid:105 ppid:104 task_flags:0x40044c flags:0x00080802
>
>and keeps failing until the tracer reaps or exits, because neither wait
>can be frozen:
>
>- The first loop sleeps in kernel_wait4(). The freezer's fake signal
> ends that wait with -ERESTARTSYS, then the loop clears TIF_SIGPENDING
> and waits again without ever calling try_to_freeze().
>
>- The final loop sleeps in TASK_INTERRUPTIBLE without TASK_FREEZABLE.
> Worse, nothing clears the TIF_SIGPENDING that the fake signal set, so
> from the first suspend attempt on schedule() returns at once and the
> task spins at 100% CPU for as long as the wait lasts.
>
>Neither loop has ever been freezable. The spin came with
>commit b9a985db9896 ("pid_ns: Sleep in TASK_INTERRUPTIBLE in
>zap_pid_ns_processes"),
>which replaced TASK_UNINTERRUPTIBLE to keep such long waits from
>triggering the hung task detector.
>
>This happens in practice with Chromium-based browsers, whose renderers
>are pid namespace inits. If a renderer crashes while the browser shuts
>down, crashpad (outside the namespace) ptrace-attaches its threads to
>dump it. When the renderer is SIGKILLed mid-dump, crashpad keeps the
>dead threads as unreaped traced zombies, and it can then block in
>waitpid() on the renderer's last thread, which is itself waiting in
>zap_pid_ns_processes() for those zombies. One such renderer blocks
>suspend system-wide until crashpad is killed or the machine reboots.
>The userspace side is being reported to Chromium separately.
>
>Call try_to_freeze() after kernel_wait4() in the first loop, and sleep
>in TASK_IDLE | TASK_FREEZABLE in the final loop, as coredump_task_exit()
>already does on the same exit path. Like TASK_INTERRUPTIBLE, TASK_IDLE
>keeps the hung task detector quiet and adds no load, but it is not
>woken by signals, so a pending signal can no longer turn the wait into
>a busy loop. TASK_FREEZABLE lets the freezer freeze the task in place;
>a free_pid() wakeup that arrives while it is frozen is kept in
>->saved_state and acted on at thaw.
>
>Tested on v7.3-rc5 in QEMU with PROVE_LOCKING, DEBUG_ATOMIC_SLEEP and
>DETECT_HUNG_TASK (10 s timeout), using a reproducer in which a tracer
>PTRACE_SEIZEs two threads of a nested pid namespace init and doesn't
>reap them (it works unprivileged too, from a user namespace), and using
>the real browser and crashpad trigger:
>
>- before: freezing fails after 20 s, and afterwards the final loop
> spins (500 ticks of system time per 5 s).
>- after: freezing completes in 0.001 s, the waiting task sits in state
> I without using CPU, no hung task or lockdep report appears over
> 30-45 s, and the task exits as soon as the zombies are reaped.
>
>Fixes: 3eb07c8c8adb ("pid namespaces: destroy pid namespace on init's death")
>Fixes: b9a985db9896 ("pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns_processes")
>Cc: stable@vger.kernel.org # 6.1+
>Assisted-by: Claude:claude-opus-5-5
>Signed-off-by: Aviv Vaknin <vaknins33@gmail.com>
>---
>Based on vfs kernel-7.4.signal (also in next-20260930), on top of
>5c0985dd3c32 ("pid_namespace: prevent TIF_NOTIFY_SIGNAL from
>interrupting the reaper"). That commit keeps TIF_NOTIFY_SIGNAL from
>waking the reaper; this one is about the freezer, whose fake signal
>sets TIF_SIGPENDING, which the guard doesn't cover. Retested on this
>base with the reproducer below (same debug config): freezing completes
>in 0.000 s, no spin, no warnings. A version for v7.3-rc5 is available
>if wanted.
>
>Reproducer (run as root in a VM, then follow its instructions):
>
> // SPDX-License-Identifier: GPL-2.0
> /*
> * pidns-freeze-repro: a task waiting in zap_pid_ns_processes() blocks suspend.
> *
> * main (tracer, root pid ns)
> * `- A: pid 1 of pid ns A, single-threaded
> * `- B: pid 1 of nested pid ns B, multithreaded
> *
> * main PTRACE_SEIZEs non-leader threads of B, then A exits. zap(A) kills B;
> * the seized threads stay traced zombies until main reaps them, keeping ns B's
> * pids allocated. B's last thread waits in zap's final loop, A in zap's first
> * loop (kernel_wait4() for B). While "stuck" is shown, run as root:
> * echo freezer > /sys/power/pm_test; echo mem > /sys/power/state
> * Unpatched, freezing fails after 20 s ("2 tasks refusing to freeze") and B's
> * thread then busy-loops in zap. At the end main reaps, ending the hang.
> *
> * Build: gcc -O2 -Wall -pthread -o pidns-freeze-repro pidns-freeze-repro.c
> * Usage: ./pidns-freeze-repro [hold_seconds] (root, in a VM; default: Enter)
> * With -DUSERNS, A also gets a new user ns, so no privilege is needed.
> */
> #define _GNU_SOURCE
> #include <dirent.h>
> #include <pthread.h>
> #include <sched.h>
> #include <signal.h>
> #include <stdarg.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/ptrace.h>
> #include <sys/wait.h>
> #include <unistd.h>
>
> #ifdef USERNS
> #define NS_FLAGS (CLONE_NEWUSER | CLONE_NEWPID)
> #else
> #define NS_FLAGS CLONE_NEWPID
> #endif
> #define NTHREADS 4 /* B's threads besides its leader */
> #define NSEIZE 2 /* how many of them main seizes */
>
> static int ready[2], go[2]; /* B -> main: threads are up; main -> A: exit */
> static char stack_a[256 << 10], stack_b[256 << 10];
>
> static void *idle_thread(void *arg) { for (;;) pause(); return arg; }
>
> static int init_b(void *arg) /* pid 1 of ns B */
> {
> pthread_t t;
>
> for (int i = 0; i < NTHREADS; i++)
> pthread_create(&t, NULL, idle_thread, arg);
> if (write(ready[1], "r", 1) != 1)
> _exit(1);
> for (;;) pause();
> }
>
> static int init_a(void *arg) /* pid 1 of ns A */
> {
> close(go[1]); /* only main may hold the write end */
> if (clone(init_b, stack_b + sizeof(stack_b), CLONE_NEWPID | SIGCHLD, arg) < 0)
> _exit(1);
> while (read(go[0], &arg, 1) > 0) /* EOF: main has seized */
> ;
> _exit(0); /* -> zap_pid_ns_processes(A) */
> }
>
> /* Read a small /proc file into buf ("" if it is gone). */
> static char *slurp(char *buf, size_t len, const char *fmt, ...)
> {
> char path[300];
> va_list ap;
> FILE *f;
>
> va_start(ap, fmt);
> vsnprintf(path, sizeof(path), fmt, ap);
> va_end(ap);
> buf[0] = 0;
> if ((f = fopen(path, "r"))) {
> buf[fread(buf, 1, len - 1, f)] = 0;
> fclose(f);
> }
> return buf;
> }
>
> /* Global pid of A's child B: the process whose PPid is A. */
> static pid_t child_of(pid_t parent)
> {
> DIR *d = opendir("/proc");
> struct dirent *e;
> char buf[512], *p;
> int ppid;
>
> while (d && (e = readdir(d))) {
> p = strrchr(slurp(buf, sizeof(buf), "/proc/%s/stat", e->d_name), ')');
> if (p && sscanf(p + 2, "%*c %d", &ppid) == 1 && ppid == parent)
> return atoi(e->d_name); /* (leaks d; fine here) */
> }
> return -1;
> }
>
> /* Print a task's state and wchan; return whether it is exiting (PF_EXITING). */
> static int show(const char *what, pid_t pid, pid_t tid)
> {
> char st[512], wchan[64], *p, c = '?';
> unsigned int flags = 0;
>
> p = strrchr(slurp(st, sizeof(st), "/proc/%d/task/%d/stat", pid, tid), ')');
> if (p)
> sscanf(p + 2, "%c %*d %*d %*d %*d %*d %u", &c, &flags);
> slurp(wchan, sizeof(wchan), "/proc/%d/task/%d/wchan", pid, tid);
> printf(" %s tid %d: state %c, flags %#x, wchan %s\n", what, tid, c, flags, wchan);
> return flags & 0x4;
> }
>
> int main(int argc, char **argv)
> {
> pid_t a, b, tid, seized[NSEIZE];
> int n = 0, status;
> struct dirent *e;
> char buf[64];
> DIR *d;
>
> if (pipe(ready) || pipe(go))
> return perror("pipe"), 1;
> if ((a = clone(init_a, stack_a + sizeof(stack_a), NS_FLAGS | SIGCHLD, NULL)) < 0)
> return perror("clone (run as root)"), 1;
> if (read(ready[0], buf, 1) != 1 || (b = child_of(a)) < 0)
> return fprintf(stderr, "setup failed\n"), 1;
>
> /* From outside both namespaces, seize non-leader threads of B. */
> snprintf(buf, sizeof(buf), "/proc/%d/task", b);
> for (d = opendir(buf); d && (e = readdir(d)) && n < NSEIZE; )
> if ((tid = atoi(e->d_name)) > 0 && tid != b && !ptrace(PTRACE_SEIZE, tid, 0, 0))
> seized[n++] = tid;
> printf("A=%d B=%d: seized %d threads of B\n", a, b, n);
>
> close(go[1]); /* A exits now */
> sleep(2);
> if (!n || waitpid(a, &status, WNOHANG) != 0 || !show("A", a, a))
> return printf("not reproduced\n"), 1;
> for (rewinddir(d); (e = readdir(d)); )
> if ((tid = atoi(e->d_name)) > 0)
> show("B", b, tid);
> printf("stuck: A and B can't finish exiting until main reaps its %d traced threads\n", n);
> printf("now run: echo freezer > /sys/power/pm_test; echo mem > /sys/power/state\n");
> fflush(stdout);
> if (argc > 1)
> sleep(atoi(argv[1]));
> else
> getchar();
>
> /* Reap the traced zombies: ns B empties, then B and A finish exiting. */
> for (int i = 0, left = n; left && i < 500; i++, usleep(10000))
> for (int k = 0; k < n; k++)
> if (seized[k] && waitpid(seized[k], &status, __WALL | WNOHANG) == seized[k])
> seized[k] = 0, left--;
> for (int i = 0; i < 500; i++, usleep(10000))
> if (waitpid(a, &status, WNOHANG) == a)
> return printf("reaped the zombies: hang resolved\n"), 0;
> printf("still stuck after reaping\n");
> return 2;
> }
>
> kernel/pid_namespace.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
>diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
>index 8bc9edb40..abdc05462 100644
>--- a/kernel/pid_namespace.c
>+++ b/kernel/pid_namespace.c
>@@ -23,6 +23,7 @@
> #include <linux/sched/task.h>
> #include <linux/sched/signal.h>
> #include <linux/idr.h>
>+#include <linux/freezer.h>
> #include <linux/nstree.h>
> #include <uapi/linux/wait.h>
> #include "pid_sysctl.h"
>@@ -243,6 +244,11 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
> do {
> clear_thread_flag(TIF_SIGPENDING);
> rc = kernel_wait4(-1, NULL, __WALL, NULL);
>+ /*
>+ * The freezer's fake signal ends the wait with -ERESTARTSYS;
>+ * freeze here, or one stuck pid namespace blocks suspend.
>+ */
>+ try_to_freeze();
> } while (rc != -ECHILD);
>
> /*
>@@ -269,7 +275,13 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
> * free_pid() will awaken this task.
> */
> for (;;) {
>- set_current_state(TASK_INTERRUPTIBLE);
>+ /*
>+ * TASK_IDLE: no hung task warning or load for a wait that can
>+ * last as long as a tracer keeps a zombie, and a pending signal
>+ * (e.g. the freezer's fake one) can't turn it into a busy loop.
>+ * TASK_FREEZABLE: let the freezer freeze us while we wait.
>+ */
>+ set_current_state(TASK_IDLE | TASK_FREEZABLE);
> if (pid_ns->pid_allocated == init_pids)
> break;
> schedule();
>
>base-commit: ed14a591175bb5f56c2936b082cffb9e4b935e6d
>
Fair enough:
Reviewed-by: Bradley Morgan <brads@mainlining.org>
--- Thanks!
"I'm not a very positive person" - Linus torvalds
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pid_namespace: make zap_pid_ns_processes() freezable
2026-10-01 12:12 [PATCH] pid_namespace: make zap_pid_ns_processes() freezable Aviv Vaknin
` (2 preceding siblings ...)
2026-10-01 16:53 ` Bradley Morgan
@ 2026-10-02 14:26 ` Oleg Nesterov
2026-10-02 16:24 ` Rafael J. Wysocki (Intel)
3 siblings, 1 reply; 9+ messages in thread
From: Oleg Nesterov @ 2026-10-02 14:26 UTC (permalink / raw)
To: Aviv Vaknin, Rafael J. Wysocki
Cc: Christian Brauner, Pavel Tikhomirov, Eric W. Biederman, linux-pm,
linux-kernel, stable
On 10/01, Aviv Vaknin wrote:
>
> Fixes: 3eb07c8c8adb ("pid namespaces: destroy pid namespace on init's death")
> Fixes: b9a985db9896 ("pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns_processes")
> Cc: stable@vger.kernel.org # 6.1+
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Aviv Vaknin <vaknins33@gmail.com>
OK, since Rafael agrees with this patch
Acked-by: Oleg Nesterov <oleg@redhat.com>
but see below...
> @@ -243,6 +244,11 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
> do {
> clear_thread_flag(TIF_SIGPENDING);
> rc = kernel_wait4(-1, NULL, __WALL, NULL);
> + /*
> + * The freezer's fake signal ends the wait with -ERESTARTSYS;
> + * freeze here, or one stuck pid namespace blocks suspend.
> + */
> + try_to_freeze();
> } while (rc != -ECHILD);
Somehow I still think it would be better to change do_wait() to use
TASK_INTERRUPTIBLE | TASK_FREEZABLE. Slightly less robust in theory,
but I think should work in practice...
Rafael, what do you think?
> for (;;) {
> - set_current_state(TASK_INTERRUPTIBLE);
> + /*
> + * TASK_IDLE: no hung task warning or load for a wait that can
> + * last as long as a tracer keeps a zombie, and a pending signal
> + * (e.g. the freezer's fake one) can't turn it into a busy loop.
> + * TASK_FREEZABLE: let the freezer freeze us while we wait.
> + */
> + set_current_state(TASK_IDLE | TASK_FREEZABLE);
This looks like overdocumentation to me. I guess it was added by AI. Other users
of IDLE/FREEZABLE do not try to document the meaning of these task states.
But this is subjective, I won't insist.
Oleg.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pid_namespace: make zap_pid_ns_processes() freezable
2026-10-02 14:26 ` Oleg Nesterov
@ 2026-10-02 16:24 ` Rafael J. Wysocki (Intel)
2026-10-02 18:06 ` Oleg Nesterov
0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-10-02 16:24 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Aviv Vaknin, Rafael J. Wysocki, Christian Brauner,
Pavel Tikhomirov, Eric W. Biederman, linux-pm, linux-kernel,
stable
On Fri, Oct 2, 2026 at 4:26 PM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 10/01, Aviv Vaknin wrote:
> >
> > Fixes: 3eb07c8c8adb ("pid namespaces: destroy pid namespace on init's death")
> > Fixes: b9a985db9896 ("pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns_processes")
> > Cc: stable@vger.kernel.org # 6.1+
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Aviv Vaknin <vaknins33@gmail.com>
>
> OK, since Rafael agrees with this patch
>
> Acked-by: Oleg Nesterov <oleg@redhat.com>
>
>
> but see below...
>
> > @@ -243,6 +244,11 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
> > do {
> > clear_thread_flag(TIF_SIGPENDING);
> > rc = kernel_wait4(-1, NULL, __WALL, NULL);
> > + /*
> > + * The freezer's fake signal ends the wait with -ERESTARTSYS;
> > + * freeze here, or one stuck pid namespace blocks suspend.
> > + */
> > + try_to_freeze();
> > } while (rc != -ECHILD);
>
> Somehow I still think it would be better to change do_wait() to use
> TASK_INTERRUPTIBLE | TASK_FREEZABLE. Slightly less robust in theory,
> but I think should work in practice...
>
> Rafael, what do you think?
Well, why exactly do you think that TASK_INTERRUPTIBLE would be better
than TASK_IDLE here?
> > for (;;) {
> > - set_current_state(TASK_INTERRUPTIBLE);
> > + /*
> > + * TASK_IDLE: no hung task warning or load for a wait that can
> > + * last as long as a tracer keeps a zombie, and a pending signal
> > + * (e.g. the freezer's fake one) can't turn it into a busy loop.
> > + * TASK_FREEZABLE: let the freezer freeze us while we wait.
> > + */
> > + set_current_state(TASK_IDLE | TASK_FREEZABLE);
>
> This looks like overdocumentation to me. I guess it was added by AI. Other users
> of IDLE/FREEZABLE do not try to document the meaning of these task states.
It looks a bit like a note for self TBH.
> But this is subjective, I won't insist.
Same here.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pid_namespace: make zap_pid_ns_processes() freezable
2026-10-02 16:24 ` Rafael J. Wysocki (Intel)
@ 2026-10-02 18:06 ` Oleg Nesterov
2026-10-02 18:13 ` Rafael J. Wysocki (Intel)
0 siblings, 1 reply; 9+ messages in thread
From: Oleg Nesterov @ 2026-10-02 18:06 UTC (permalink / raw)
To: Rafael J. Wysocki (Intel)
Cc: Aviv Vaknin, Christian Brauner, Pavel Tikhomirov,
Eric W. Biederman, linux-pm, linux-kernel, stable
On 10/02, Rafael J. Wysocki (Intel) wrote:
>
> On Fri, Oct 2, 2026 at 4:26 PM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > > @@ -243,6 +244,11 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
> > > do {
> > > clear_thread_flag(TIF_SIGPENDING);
> > > rc = kernel_wait4(-1, NULL, __WALL, NULL);
> > > + /*
> > > + * The freezer's fake signal ends the wait with -ERESTARTSYS;
> > > + * freeze here, or one stuck pid namespace blocks suspend.
> > > + */
> > > + try_to_freeze();
> > > } while (rc != -ECHILD);
> >
> > Somehow I still think it would be better to change do_wait() to use
> > TASK_INTERRUPTIBLE | TASK_FREEZABLE. Slightly less robust in theory,
> > but I think should work in practice...
> >
> > Rafael, what do you think?
>
> Well, why exactly do you think that TASK_INTERRUPTIBLE would be better
> than TASK_IDLE here?
Hmm... it seems we don't understand each other. At least I certainly don't
understand your "than TASK_IDLE here".
I don't see TASK_IDLE "here", I see that this patch adds try_to_freeze()
"here". And this is fine. But, I was thinking about this change instead:
--- x/kernel/exit.c
+++ x/kernel/exit.c
@@ -1729,7 +1729,7 @@ static long do_wait(struct wait_opts *wo
add_wait_queue(¤t->signal->wait_chldexit, &wo->child_wait);
do {
- set_current_state(TASK_INTERRUPTIBLE);
+ set_current_state(TASK_INTERRUPTIBLE | TASK_FREEZABLE);
retval = __do_wait(wo);
if (retval != -ERESTARTSYS)
break;
because IMO it makes sense anyway. This way try_to_freeze_tasks() -> __freeze_task()
path can freeze the tasks sleeping in do_wait() without waiting until they react to
fake_signal_wake_up() and call get_signal() -> try_to_freeze().
Plus this looks simpler, zap_pid_ns_processes() doesn't need another try_to_freeze().
Sorry if I missed your point...
> > > for (;;) {
> > > - set_current_state(TASK_INTERRUPTIBLE);
> > > + /*
> > > + * TASK_IDLE: no hung task warning or load for a wait that can
> > > + * last as long as a tracer keeps a zombie, and a pending signal
> > > + * (e.g. the freezer's fake one) can't turn it into a busy loop.
> > > + * TASK_FREEZABLE: let the freezer freeze us while we wait.
> > > + */
> > > + set_current_state(TASK_IDLE | TASK_FREEZABLE);
> >
> > This looks like overdocumentation to me. I guess it was added by AI. Other users
> > of IDLE/FREEZABLE do not try to document the meaning of these task states.
>
> It looks a bit like a note for self TBH.
True. But...
Ok, firstly the comment about TASK_IDLE looks a bit misleading to me. I mean the
"as long as a tracer keeps a zombie" part. At this point the descedants traced from
the parent namespace have already gone. The huge comment above this loop tries to
explain the reasons for this "wait for pid_allocated == init_pids" in more details.
Secondly. The comment about TASK_FREEZABLE looks fine. But why should the user,
zap_pid_ns_processes(), explain the semantics of TASK_FREEZABLE? Probably this
documentation makes sense, but then it should be moved to sched.h. Just my IMHO.
> > But this is subjective, I won't insist.
>
> Same here.
OK ;)
Oleg.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pid_namespace: make zap_pid_ns_processes() freezable
2026-10-02 18:06 ` Oleg Nesterov
@ 2026-10-02 18:13 ` Rafael J. Wysocki (Intel)
2026-10-02 20:28 ` Aviv Vaknin
0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-10-02 18:13 UTC (permalink / raw)
To: Oleg Nesterov, Aviv Vaknin
Cc: Christian Brauner, Pavel Tikhomirov, Eric W. Biederman, linux-pm,
linux-kernel, stable
On Fri, Oct 2, 2026 at 8:06 PM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 10/02, Rafael J. Wysocki (Intel) wrote:
> >
> > On Fri, Oct 2, 2026 at 4:26 PM Oleg Nesterov <oleg@redhat.com> wrote:
> > >
> > > > @@ -243,6 +244,11 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
> > > > do {
> > > > clear_thread_flag(TIF_SIGPENDING);
> > > > rc = kernel_wait4(-1, NULL, __WALL, NULL);
> > > > + /*
> > > > + * The freezer's fake signal ends the wait with -ERESTARTSYS;
> > > > + * freeze here, or one stuck pid namespace blocks suspend.
> > > > + */
> > > > + try_to_freeze();
> > > > } while (rc != -ECHILD);
> > >
> > > Somehow I still think it would be better to change do_wait() to use
> > > TASK_INTERRUPTIBLE | TASK_FREEZABLE. Slightly less robust in theory,
> > > but I think should work in practice...
> > >
> > > Rafael, what do you think?
> >
> > Well, why exactly do you think that TASK_INTERRUPTIBLE would be better
> > than TASK_IDLE here?
>
> Hmm... it seems we don't understand each other. At least I certainly don't
> understand your "than TASK_IDLE here".
>
> I don't see TASK_IDLE "here", I see that this patch adds try_to_freeze()
> "here". And this is fine. But, I was thinking about this change instead:
>
> --- x/kernel/exit.c
> +++ x/kernel/exit.c
> @@ -1729,7 +1729,7 @@ static long do_wait(struct wait_opts *wo
> add_wait_queue(¤t->signal->wait_chldexit, &wo->child_wait);
>
> do {
> - set_current_state(TASK_INTERRUPTIBLE);
> + set_current_state(TASK_INTERRUPTIBLE | TASK_FREEZABLE);
> retval = __do_wait(wo);
> if (retval != -ERESTARTSYS)
> break;
>
> because IMO it makes sense anyway. This way try_to_freeze_tasks() -> __freeze_task()
> path can freeze the tasks sleeping in do_wait() without waiting until they react to
> fake_signal_wake_up() and call get_signal() -> try_to_freeze().
>
> Plus this looks simpler, zap_pid_ns_processes() doesn't need another try_to_freeze().
>
> Sorry if I missed your point...
No worries and thanks for the explanation!
Yes, it looks simpler and it should actually work.
Aviv, what do you think?
> > > > for (;;) {
> > > > - set_current_state(TASK_INTERRUPTIBLE);
> > > > + /*
> > > > + * TASK_IDLE: no hung task warning or load for a wait that can
> > > > + * last as long as a tracer keeps a zombie, and a pending signal
> > > > + * (e.g. the freezer's fake one) can't turn it into a busy loop.
> > > > + * TASK_FREEZABLE: let the freezer freeze us while we wait.
> > > > + */
> > > > + set_current_state(TASK_IDLE | TASK_FREEZABLE);
> > >
> > > This looks like overdocumentation to me. I guess it was added by AI. Other users
> > > of IDLE/FREEZABLE do not try to document the meaning of these task states.
> >
> > It looks a bit like a note for self TBH.
>
> True. But...
>
> Ok, firstly the comment about TASK_IDLE looks a bit misleading to me. I mean the
> "as long as a tracer keeps a zombie" part. At this point the descedants traced from
> the parent namespace have already gone. The huge comment above this loop tries to
> explain the reasons for this "wait for pid_allocated == init_pids" in more details.
>
> Secondly. The comment about TASK_FREEZABLE looks fine. But why should the user,
> zap_pid_ns_processes(), explain the semantics of TASK_FREEZABLE? Probably this
> documentation makes sense, but then it should be moved to sched.h. Just my IMHO.
>
> > > But this is subjective, I won't insist.
> >
> > Same here.
>
> OK ;)
>
> Oleg.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pid_namespace: make zap_pid_ns_processes() freezable
2026-10-02 18:13 ` Rafael J. Wysocki (Intel)
@ 2026-10-02 20:28 ` Aviv Vaknin
0 siblings, 0 replies; 9+ messages in thread
From: Aviv Vaknin @ 2026-10-02 20:28 UTC (permalink / raw)
To: Rafael J. Wysocki, Oleg Nesterov
Cc: Christian Brauner, Pavel Tikhomirov, Eric W. Biederman,
Bradley Morgan, linux-pm, linux-kernel, stable
On Fri, Oct 2, 2026 at 8:13 PM Rafael J. Wysocki (Intel) <rafael@kernel.org> wrote:
> Yes, it looks simpler and it should actually work.
>
> Aviv, what do you think?
Agreed, it's simpler and avoids the extra try_to_freeze(). I've sent v2
with that change and without the comments:
https://lore.kernel.org/r/20261002202820.3433318-1-vaknins33@gmail.com
Thanks Oleg and Rafael for the review.
Aviv
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-02 20:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-01 12:12 [PATCH] pid_namespace: make zap_pid_ns_processes() freezable Aviv Vaknin
2026-10-01 13:03 ` Rafael J. Wysocki (Intel)
2026-10-01 15:35 ` Oleg Nesterov
2026-10-01 16:53 ` Bradley Morgan
2026-10-02 14:26 ` Oleg Nesterov
2026-10-02 16:24 ` Rafael J. Wysocki (Intel)
2026-10-02 18:06 ` Oleg Nesterov
2026-10-02 18:13 ` Rafael J. Wysocki (Intel)
2026-10-02 20:28 ` Aviv Vaknin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®