[CRIU] [PATCH 5/7] restore: use less convoluted method to calculate the number of zombies

[Pavel Emelyanov]

On 06/23/2016 06:13 PM, Tycho Andersen wrote:
> Instead of doing this crazy thing where we pass the number of zombies back
> from the restorer blob to the main restore thread, let's just calculate the
> number in the main restore thread when we need it.
> 
> Also, note that this patch shifts the location of this calculation: we used
> to do a decrement of nr_in_progress when we waited() on a zombie, to sort
> of lie about the fact that they did go through CR_STATE_RESTORE_SIGCHLD,
> but since we're waiting on them anyway explicitly, there's no reason to do
> all this hackery. Let's just calculate the number once in the right place
> and be done with it.

Seems legit :) Applied, thanks.

> Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> ---
>  criu/cr-restore.c       | 14 +++++++-------
>  criu/include/rst_info.h |  1 -
>  criu/pie/restorer.c     |  3 ---
>  3 files changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index f3e2cfb..3651f54 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -1617,6 +1617,7 @@ static int restore_root_task(struct pstree_item *init)
>  	enum trace_flags flag = TRACE_ALL;
>  	int ret, fd, mnt_ns_fd = -1;
>  	int clean_remaps = 1, root_seized = 0;
> +	struct pstree_item *item;
>  
>  	ret = run_scripts(ACT_PRE_RESTORE);
>  	if (ret != 0) {
> @@ -1736,17 +1737,17 @@ static int restore_root_task(struct pstree_item *init)
>  	if (ret < 0)
>  		goto out_kill;
>  
> +	/* Zombies die after CR_STATE_RESTORE */
> +	for_each_pstree_item(item) {
> +		if (item->pid.state == TASK_DEAD)
> +			task_entries->nr_threads--;
> +	}
> +
>  	ret = restore_switch_stage(CR_STATE_RESTORE_SIGCHLD);
>  	if (ret < 0)
>  		goto out_kill;
>  
>  	/*
> -	 * The task_entries->nr_zombies is updated in the
> -	 * CR_STATE_RESTORE_SIGCHLD in pie code.
> -	 */
> -	task_entries->nr_threads -= atomic_read(&task_entries->nr_zombies);
> -
> -	/*
>  	 * There is no need to call try_clean_remaps() after this point,
>  	 * as restore went OK and all ghosts were removed by the openers.
>  	 */
> @@ -1877,7 +1878,6 @@ int prepare_task_entries(void)
>  	task_entries->nr_threads = 0;
>  	task_entries->nr_tasks = 0;
>  	task_entries->nr_helpers = 0;
> -	atomic_set(&task_entries->nr_zombies, 0);
>  	futex_set(&task_entries->start, CR_STATE_RESTORE_NS);
>  	mutex_init(&task_entries->userns_sync_lock);
>  
> diff --git a/criu/include/rst_info.h b/criu/include/rst_info.h
> index bf6a64f..2a35d88 100644
> --- a/criu/include/rst_info.h
> +++ b/criu/include/rst_info.h
> @@ -7,7 +7,6 @@
>  
>  struct task_entries {
>  	int nr_threads, nr_tasks, nr_helpers;
> -	atomic_t nr_zombies;
>  	futex_t nr_in_progress;
>  	futex_t start;
>  	atomic_t cr_err;
> diff --git a/criu/pie/restorer.c b/criu/pie/restorer.c
> index 5910fd0..2578f77 100644
> --- a/criu/pie/restorer.c
> +++ b/criu/pie/restorer.c
> @@ -1042,8 +1042,6 @@ static int wait_zombies(struct task_restore_args *task_args)
>  {
>  	int i;
>  
> -	atomic_add(task_args->zombies_n, &task_entries->nr_zombies);
> -
>  	for (i = 0; i < task_args->zombies_n; i++) {
>  		if (sys_waitid(P_PID, task_args->zombies[i], NULL, WNOWAIT | WEXITED, NULL) < 0) {
>  			pr_err("Wait on %d zombie failed\n", task_args->zombies[i]);
> @@ -1051,7 +1049,6 @@ static int wait_zombies(struct task_restore_args *task_args)
>  		}
>  		pr_debug("%ld: Collect a zombie with pid %d\n",
>  			sys_getpid(), task_args->zombies[i]);
> -		futex_dec_and_wake(&task_entries->nr_in_progress);
>  	}
>  
>  	return 0;
>