[CRIU] [PATCH 03/11] pstree/pid: add helper to get free pids in pidns and all it's ancestors

[Kirill Tkhai]

Not critical notes, but:

On 26.05.2017 20:02, Pavel Tikhomirov wrote:
> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> ---
>  criu/include/pstree.h |  2 +-
>  criu/namespaces.c     | 19 ++++---------------
>  criu/pstree.c         | 23 ++++++++++++++++++++++-
>  3 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/criu/include/pstree.h b/criu/include/pstree.h
> index 892a1cd..bae5ce6 100644
> --- a/criu/include/pstree.h
> +++ b/criu/include/pstree.h
> @@ -88,7 +88,7 @@ static inline bool task_alive(struct pstree_item *i)
>  	return is_alive_state(i->pid->state);
>  }
>  
> -extern int get_free_pid(struct ns_id *ns);
> +extern int get_free_pids(struct ns_id *ns, pid_t *pids);
>  extern void free_pstree_item(struct pstree_item *item);
>  extern void free_pstree(struct pstree_item *root_item);
>  extern struct pstree_item *__alloc_pstree_item(bool rst, int level);
> diff --git a/criu/namespaces.c b/criu/namespaces.c
> index 3473ab1..0c901e6 100644
> --- a/criu/namespaces.c
> +++ b/criu/namespaces.c
> @@ -2526,25 +2526,14 @@ int set_user_ns(u32 id)
>  static int do_reserve_pid_ns_helpers(struct ns_id *ns, void *oarg)
>  {
>  	struct pstree_item *helper;
> -	struct ns_id *iter = ns;
>  	pid_t pid[MAX_NS_NESTING], *p;
> -	int i, level;
> +	int level;
>  
> -	for (i = MAX_NS_NESTING-1; iter && i >= 0; i--, iter = iter->parent) {
> -		pid[i] = get_free_pid(iter);
> -		if (pid[i] < 0) {
> -			pr_err("Can't find free pid\n");
> -			return -1;
> -		}
> -	}
> -
> -	if (iter) {
> -		pr_err("Too many pids levels\n");
> +	level = get_free_pids(ns, pid);
> +	if (level < 0)

If we need this check, than we should add "level == 0", which is also error.

>  		return -1;
> -	}
>  
> -	p = &pid[++i];
> -	level = MAX_NS_NESTING - i;
> +	p = &pid[MAX_NS_NESTING - level];
>  	helper = lookup_create_item(p, level, ns->id);
>  	if (helper == NULL)
>  		return -1;
> diff --git a/criu/pstree.c b/criu/pstree.c
> index 83bb966..1fe5f2d 100644
> --- a/criu/pstree.c
> +++ b/criu/pstree.c
> @@ -833,7 +833,7 @@ static int read_pstree_image(pid_t *pid_max)
>  }
>  
>  #define RESERVED_PIDS		300
> -int get_free_pid(struct ns_id *ns)
> +static int get_free_pid(struct ns_id *ns)
>  {
>  	struct pid *prev, *next;
>  	struct ns_id *i = ns;
> @@ -863,6 +863,27 @@ int get_free_pid(struct ns_id *ns)
>  	return -1;
>  }
>  
> +int get_free_pids(struct ns_id *ns, pid_t *pids)
> +{
> +	struct ns_id *iter = ns;

We may use ns as only variable, as we we are not going to use original ns after cycle..

> +	int i;
> +
> +	for (i = MAX_NS_NESTING-1; iter && i >= 0; i--, iter = iter->parent) {
> +		pids[i] = get_free_pid(iter);
> +		if (pids[i] < 0) {
> +			pr_err("Can't find free pid\n");
> +			return -1;
> +		}
> +	}
> +
> +	if (iter) {
> +		pr_err("Too many pids levels\n");
> +		return -1;
> +	}
> +
> +	return MAX_NS_NESTING - i - 1;
> +}
> +
>  static int prepare_pstree_ids(void)
>  {
>  	struct pstree_item *item, *child, *helper, *tmp;
>