[CRIU] [PATCH 4/6] pstree: resort code about abandoned tasks

[Pavel Emelyanov]

On 03/15/2016 09:37 AM, Andrey Vagin wrote:
> From: Andrew Vagin <avagin at virtuozzo.com>
> 
> Currently we enumirate all children of the init task and
> if a task isn't a session leader, we create a helper,
> collect all children with this sid to the children list of this helper.
> 
> When all children of the init task has been enumirated, we try to find
> a session leader for each helper.
> 
> We use this way to enumirate all tasks only once.
> 
> Now we are going to collect all tasks in rbtree, so we can find
> a sessial leader when we need it. It will not affect performance,
> because without searching a session leader, we can't add a helper
> to the tree.
> 
> Signed-off-by: Andrew Vagin <avagin at virtuozzo.com>
> ---
>  criu/include/pstree.h |  2 --
>  criu/pstree.c         | 69 +++++++++++++++++++++------------------------------
>  2 files changed, 28 insertions(+), 43 deletions(-)
> 
> diff --git a/criu/include/pstree.h b/criu/include/pstree.h
> index a742a48..6ff8781 100644
> --- a/criu/include/pstree.h
> +++ b/criu/include/pstree.h
> @@ -64,8 +64,6 @@ static inline bool task_alive(struct pstree_item *i)
>  extern void free_pstree(struct pstree_item *root_item);
>  extern struct pstree_item *__alloc_pstree_item(bool rst);
>  #define alloc_pstree_item() __alloc_pstree_item(false)
> -#define alloc_pstree_item_with_rst() __alloc_pstree_item(true)
> -extern struct pstree_item *alloc_pstree_helper(void);
>  extern void init_pstree_helper(struct pstree_item *ret);
>  
>  extern struct pstree_item *lookup_create_item(pid_t pid);
> diff --git a/criu/pstree.c b/criu/pstree.c
> index 8e3c0c9..3274f56 100644
> --- a/criu/pstree.c
> +++ b/criu/pstree.c
> @@ -24,6 +24,8 @@ static struct rb_root pid_root_rb;
>  
>  #define CLONE_ALLNS     (CLONE_NEWPID | CLONE_NEWNET | CLONE_NEWIPC | CLONE_NEWUTS | CLONE_NEWNS | CLONE_NEWUSER)
>  
> +#define alloc_pstree_item_with_rst() __alloc_pstree_item(true)
> +
>  void core_entry_free(CoreEntry *core)
>  {
>  	if (core->tc && core->tc->timers)
> @@ -230,17 +232,6 @@ void init_pstree_helper(struct pstree_item *ret)
>  	task_entries->nr_helpers++;
>  }
>  
> -struct pstree_item *alloc_pstree_helper(void)
> -{
> -	struct pstree_item *ret;
> -
> -	ret = alloc_pstree_item_with_rst();
> -	if (ret)
> -		init_pstree_helper(ret);
> -
> -	return ret;
> -}
> -
>  /* Deep first search on children */
>  struct pstree_item *pstree_item_next(struct pstree_item *item)
>  {
> @@ -586,6 +577,7 @@ static int prepare_pstree_ids(void)
>  	 * reparented to init.
>  	 */
>  	list_for_each_entry(item, &root_item->children, sibling) {
> +		struct pstree_item *leader;
>  
>  		/*
>  		 * If a child belongs to the root task's session or it's
> @@ -595,15 +587,31 @@ static int prepare_pstree_ids(void)
>  		if (item->sid == root_item->sid || item->sid == item->pid.virt)
>  			continue;
>  
> -		helper = alloc_pstree_helper();
> -		if (helper == NULL)
> -			return -1;
> -		helper->sid = item->sid;
> -		helper->pgid = item->sid;
> -		helper->pid.virt = item->sid;
> -		helper->parent = root_item;
> -		helper->ids = root_item->ids;
> -		list_add_tail(&helper->sibling, &helpers);
> +		leader = insert_item(item->sid);

There's no such call neither in this patch, nor in criu. At the same time
this line gets removed in patch #6 with commend "fix return code from insert_item".

Would you fix the set not to contain this?

> +		if (leader->pid.state != TASK_UNDEF) {
> +			helper = insert_item(++max_pid);
> +			if (helper == NULL)
> +				return -1;
> +
> +			pr_info("Session leader %d\n", item->sid);
> +
> +			helper->sid = item->sid;
> +			helper->pgid = leader->pgid;
> +			helper->ids = leader->ids;
> +			helper->parent = leader;
> +			list_add(&helper->sibling, &leader->children);
> +
> +			pr_info("Attach %d to the task %d\n",
> +					helper->pid.virt, leader->pid.virt);
> +		} else {
> +			helper = leader;
> +			helper->sid = item->sid;
> +			helper->pgid = item->sid;
> +			helper->parent = root_item;
> +			helper->ids = root_item->ids;
> +			list_add_tail(&helper->sibling, &helpers);
> +		}
> +		init_pstree_helper(helper);
>  
>  		pr_info("Add a helper %d for restoring SID %d\n",
>  				helper->pid.virt, helper->sid);
> @@ -663,27 +671,6 @@ static int prepare_pstree_ids(void)
>  
>  			continue;
>  		}
> -
> -		pr_info("Session leader %d\n", item->sid);
> -
> -		/* Try to find helpers, who should be connected to the leader */
> -		list_for_each_entry(child, &helpers, sibling) {
> -			if (child->pid.state != TASK_HELPER)
> -				continue;
> -
> -			if (child->sid != item->sid)
> -				continue;
> -
> -			child->pgid = item->pgid;
> -			child->pid.virt = ++max_pid;
> -			child->parent = item;
> -			list_move(&child->sibling, &item->children);
> -
> -			pr_info("Attach %d to the task %d\n",
> -					child->pid.virt, item->pid.virt);
> -
> -			break;
> -		}
>  	}
>  
>  	/* All other helpers are session leaders for own sessions */
>