Re: [PATCH 4/5] Check 'may_checkpoint()' early

["Serge E. Hallyn" <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

Quoting Sukadev Bhattiprolu (sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org):
> 
> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Subject: [PATCH 4/5] Check 'may_checkpoint()' early
> 
> We currently check if a task is checkpointable when writing the task
> information to checkpoint file. The small downside of doing this check
> late is that we may have processed several processes in the tree before
> hitting one that cannot be checkpointed.
> 
> We anyway walk the process tree we are checkpointing earlier, while
> counting the tasks. We could check if all processes in the tree are
> checkpointable at that time and fail early if any of them are not.
> Since the process tree should be frozen, checking earlier should not
> matter ?
> 
> For now, the patch leaves the existing check in cr_write_pids(). We
> can remove that later.
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

Yes this seems like a good idea to me.

Acked-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

> ---
>  checkpoint/checkpoint.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/checkpoint/checkpoint.c b/checkpoint/checkpoint.c
> index fbcd9eb..9189abb 100644
> --- a/checkpoint/checkpoint.c
> +++ b/checkpoint/checkpoint.c
> @@ -346,11 +346,18 @@ static int cr_tree_count_tasks(struct cr_ctx *ctx)
>  	struct task_struct **tasks_arr = ctx->tasks_arr;
>  	int tasks_nr = ctx->tasks_nr;
>  	int nr = 0;
> +	int ret;
> 
>  	read_lock(&tasklist_lock);
> 
>  	/* count tasks via DFS scan of the tree */
>  	while (1) {
> +		ret = cr_may_checkpoint_task(task, ctx);
> +		if (ret < 0) {
> +			nr = ret;
> +			break;
> +		}
> +
>  		if (tasks_arr) {
>  			/* unlikely, but ... */
>  			if (nr == tasks_nr)
> @@ -406,6 +413,8 @@ static int cr_build_tree(struct cr_ctx *ctx)
> 
>  	/* count tasks (no side effects) */
>  	n = cr_tree_count_tasks(ctx);
> +	if (n < 0)
> +		return n;
> 
>  	ctx->tasks_nr = n;
>  	ctx->tasks_arr = kzalloc(n * sizeof(*ctx->tasks_arr), GFP_KERNEL);
> -- 
> 1.5.2.5
> 
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers