Re: [PATCH] pid_namespace: make zap_pid_ns_processes() freezable

[Oleg Nesterov]

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@xxxxxxxxxxxxxxx # 6.1+
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Aviv Vaknin <vaknins33@xxxxxxxxx>

OK, since Rafael agrees with this patch

Acked-by: Oleg Nesterov <oleg@xxxxxxxxxx>


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.