[Devel] Re: [PATCH 3/6] Check 'may_checkpoint()' early

[Sukadev Bhattiprolu]

Oren, 

I see that patch 3/6 (deny external checkpoint unless frozen) in this
set was merged in v14, but do you have any plans/objections to merging
this optimization patch ? Serge had acked an earlier version of this
patch.

Suka

Sukadev Bhattiprolu [sukadev at linux.vnet.ibm.com] wrote:
| 
| From: Sukadev Bhattiprolu <sukadev at linux.vnet.ibm.com>
| Date: Thu, 12 Mar 2009 14:19:54 -0700
| Subject: [PATCH 3/6] 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 at linux.vnet.ibm.com>
| ---
|  checkpoint/checkpoint.c |    9 +++++++++
|  1 files changed, 9 insertions(+), 0 deletions(-)
| 
| diff --git a/checkpoint/checkpoint.c b/checkpoint/checkpoint.c
| index ed4fd3d..dae9b97 100644
| --- a/checkpoint/checkpoint.c
| +++ b/checkpoint/checkpoint.c
| @@ -341,11 +341,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)
| @@ -401,6 +408,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 at lists.linux-foundation.org
| https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers