Re: [RFC][PATCH] 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>
> Date: Mon, 16 Feb 2009 12:05:50 -0800
> Subject: [PATCH] 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

Yes we'll at least need to add checks that there is no running task
sharing an mm or files_struct with a checkpointed task, else some of
our checks will definately be racy (and are even now).

> can remove that later.
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@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 64155de..2bbb409 100644
> --- a/checkpoint/checkpoint.c
> +++ b/checkpoint/checkpoint.c
> @@ -347,11 +347,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 so then try again later */
>  			if (nr == tasks_nr) {
> @@ -409,6 +416,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