[Devel] Re: [PATCH] c/r: threads sync on restart (fix regression from commit afbe522c...)

[Serge E. Hallyn]

Quoting Oren Laadan (orenl at librato.com):
> Initialize the new member signal->restart_count in copy_signal().
> 
> The lack of which in commit afbe522ccf6b1deb5edfc01a00becfe6ee325d0c,
> c/r: let entire thread group in sys_restart before restoring a thread,
> can cause run-pthreads1.sh test failure.
> 
> In this patch are two other small cleanups:
> 
> 1) In post_restore_task() test if current->checkpoint_data is valid,
> and return immediately if not (meaning restart failed early).
> 
> 2) Replace two instances of DECLARE_WAIT_QUEUE_HEAD(waitq) with
> DECLARE_WAIT_QUEUE_HEAD_ONSTACK(waitq), to please the lockdep checked.
> 
> Signed-off-by: Oren Laadan <orenl at cs.columbia.edu>

Tested-by: Serge Hallyn <serue at us.ibm.com>

thanks,
-serge

> ---
>  checkpoint/process.c |    4 ++++
>  checkpoint/restart.c |    4 ++--
>  kernel/fork.c        |    4 ++++
>  3 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/checkpoint/process.c b/checkpoint/process.c
> index c2832fe..424f688 100644
> --- a/checkpoint/process.c
> +++ b/checkpoint/process.c
> @@ -847,6 +847,10 @@ int pre_restore_task(struct ckpt_ctx *ctx)
>  /* pre_restore_task - prepare the task for restore */
>  void post_restore_task(struct ckpt_ctx *ctx)
>  {
> +	/* can happen if restart failed early */
> +	if (!current->checkpoint_data)
> +		return;
> +
>  	/* only now is it safe to unblock the restored task's signals */
>  	sigprocmask(SIG_SETMASK, &current->checkpoint_data->blocked, NULL);
> 
> diff --git a/checkpoint/restart.c b/checkpoint/restart.c
> index b2dc139..3a58a76 100644
> --- a/checkpoint/restart.c
> +++ b/checkpoint/restart.c
> @@ -821,7 +821,7 @@ static int wait_task_sync(struct ckpt_ctx *ctx)
>  /* grabs a reference to the @ctx on success; caller should free */
>  static struct ckpt_ctx *wait_checkpoint_ctx(void)
>  {
> -	DECLARE_WAIT_QUEUE_HEAD(waitq);
> +	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(waitq);
>  	struct ckpt_ctx *ctx;
>  	int ret;
> 
> @@ -912,7 +912,7 @@ static int wait_sync_threads(void)
>  			wake_up_process(p);
>  		read_unlock(&tasklist_lock);
>  	} else {
> -		DECLARE_WAIT_QUEUE_HEAD(waitq);
> +		DECLARE_WAIT_QUEUE_HEAD_ONSTACK(waitq);
>  		ret = wait_event_interruptible(waitq, !atomic_read(count));
>  	}
> 
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 0e226f5..04dffe9 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -865,6 +865,10 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)
> 
>  	tty_audit_fork(sig);
> 
> +#ifdef CONFIG_CHECKPOINT
> +	atomic_set(&sig->restart_count, 0);
> +#endif
> +
>  	return 0;
>  }
> 
> -- 
> 1.6.0.4
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers