[CRIU] [PATCH RFC] seize: don't wory if a cgroup contains some extra tasks

[Pavel Emelyanov]

On 10/27/2015 07:19 PM, Andrey Vagin wrote:
> From: Andrew Vagin <avagin at openvz.org>
> 
> A freezer cgroup can contain tasks which will be not dumped,
> criu unfreezes the group, so we need to freeze all extra
> task with ptrace like we do for target tasks.
> 
> Currently we attache and send an interrupt signals to these tasks,
> but we don't call waitpid() for them, so then waitpid(-1, ...)
> returns these tasks where we don't expect to see them.
> 
> Signed-off-by: Andrew Vagin <avagin at openvz.org>
> ---
>  include/ptrace.h |  2 ++
>  ptrace.c         |  2 ++
>  seize.c          | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 68 insertions(+)
> 
> diff --git a/include/ptrace.h b/include/ptrace.h
> index 079ad63..a3852ce 100644
> --- a/include/ptrace.h
> +++ b/include/ptrace.h
> @@ -67,6 +67,8 @@ struct ptrace_peeksiginfo_args {
>  
>  #define SI_EVENT(_si_code)	(((_si_code) & 0xFFFF) >> 8)
>  
> +extern int frozen_processes;
> +
>  extern int seize_catch_task(pid_t pid);
>  extern int seize_wait_task(pid_t pid, pid_t ppid, struct proc_status_creds **creds);
>  extern int suspend_seccomp(pid_t pid);
> diff --git a/ptrace.c b/ptrace.c
> index b28824b..922fc87 100644
> --- a/ptrace.c
> +++ b/ptrace.c
> @@ -120,6 +120,8 @@ int seize_wait_task(pid_t pid, pid_t ppid, struct proc_status_creds **creds)
>  try_again:
>  
>  	ret = wait4(pid, &status, __WALL, NULL);
> +	if (ret > 0)
> +		frozen_processes--;

What if we goto try_again() below and wait4 the same task twice?

>  	wait_errno = errno;
>  
>  	ret2 = parse_pid_status(pid, &cr);
> diff --git a/seize.c b/seize.c
> index 20c0349..e3cd2b4 100644
> --- a/seize.c
> +++ b/seize.c
> @@ -84,6 +84,63 @@ int freeze_cgroup()
>  	return 0;
>  }
>  
> +/* A number of tasks in a freezer cgroup which are not going to be dumped */
> +int frozen_processes;
> +
> +/*
> + * A freezer cgroup can contain tasks which will not be dumped
> + * and we need to wait them, because the are interupted them by ptrace.
> + */
> +static int freezer_wait_processes()
> +{
> +	int i;
> +
> +	for (i = 0; i < frozen_processes; i++) {
> +		int status;
> +		pid_t pid;
> +
> +		/*
> +		 * Here we are going to skip tasks which are already traced.
> +		 * Ptraced tasks looks like children for us, so if
> +		 * a task isn't ptraced yet, waitpid() will return a error.
> +		 */
> +		pid = waitpid(-1, &status, 0);
> +		if (pid < 0) {
> +			pr_perror("Unable to wait processes");
> +			return -1;
> +		}
> +		pr_warn("Unexpected process %d in the freezer cgroup (status 0x%x)\n", pid, status);
> +	}
> +
> +	return 0;
> +}
> +
> +static int freezer_detach(void)
> +{
> +	char path[PATH_MAX];
> +	FILE *f;
> +
> +	if (!frozen_processes)
> +		return 0;
> +
> +	snprintf(path, sizeof(path), "%s/tasks", opts.freeze_cgroup);
> +	f = fopen(path, "r");
> +	if (f == NULL) {
> +		pr_perror("Unable to open %s", path);
> +		return -1;
> +	}
> +	while (fgets(path, sizeof(path), f)) {
> +		pid_t pid;
> +
> +		pid = atoi(path);
> +
> +		if (ptrace(PTRACE_DETACH, pid, NULL, NULL))
> +			pr_perror("Unable to detach from %d\n", pid);

Why do we need to manually detach from everybody?

> +	}
> +	fclose(f);
> +	return 0;
> +}
> +
>  static int freeze_processes(void)
>  {
>  	int i, ret, fd, exit_code = -1;
> @@ -159,6 +216,7 @@ static int freeze_processes(void)
>  				fclose(f);
>  				goto err;
>  			}
> +			frozen_processes++;

Why? Why does the ptrace attach failure result in frozen_processes++?

>  		}
>  		fclose(f);
>  
> @@ -324,6 +382,9 @@ static void pstree_wait(struct pstree_item *root_item)
>  			}
>  		}
>  	}
> +
> +	if (!opts.freeze_cgroup)
> +		freezer_detach();
>  	pid = wait4(-1, &status, __WALL, NULL);
>  	if (pid > 0) {
>  		pr_err("Unexpected child %d\n", pid);
> @@ -536,6 +597,9 @@ int collect_pstree(pid_t pid)
>  	if (ret < 0)
>  		goto err;
>  
> +	if (opts.freeze_cgroup && freezer_wait_processes())
> +		return -1;
> +
>  	timing_stop(TIME_FREEZING);
>  	timing_start(TIME_FROZEN);
>  
>