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

[Oleg Nesterov <oleg@redhat.com>]

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.