[CRIU] [PATCH 3/3] restore: use breakpoints instead of tracing syscalls

[Andrew Vagin]

On Wed, Sep 17, 2014 at 07:35:41PM +0400, Pavel Emelyanov wrote:
> On 09/17/2014 07:12 PM, Andrey Vagin wrote:
> > Currently CRIU traces syscalls to catch a moment, when sigreturn() is
> > called. Now we trace recv(cmd), close(logfd), close(cmdfd), sigreturn().
> > 
> > We can reduce a number of steps by using hw breakpoints. A breakpoint is
> > set before sigreturn, so we will need to trace only it.
> > 
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  cr-restore.c       | 10 ++++++++--
> >  include/restorer.h |  1 +
> >  include/rst_info.h |  2 ++
> >  pie/restorer.c     | 10 ++++++++--
> >  4 files changed, 19 insertions(+), 4 deletions(-)
> > 
> > diff --git a/cr-restore.c b/cr-restore.c
> > index 4d5ccd5..3be32cf 100644
> > --- a/cr-restore.c
> > +++ b/cr-restore.c
> > @@ -1532,7 +1532,7 @@ static int attach_to_tasks(bool root_seized)
> >  
> >  	for_each_pstree_item(item) {
> >  		pid_t pid = item->pid.real;
> > -		int status, i;
> > +		int status, i, ret;
> >  
> >  		if (!task_alive(item))
> >  			continue;
> > @@ -1565,7 +1565,12 @@ static int attach_to_tasks(bool root_seized)
> >  				return -1;
> >  			}
> >  
> > -			if (ptrace(PTRACE_SYSCALL, pid, NULL, NULL)) {
> > +			ret = ptrace_set_breakpoint(pid, item->rst->breakpoint);
> > +			if (ret < 0)
> > +				return -1;
> > +
> > +			/* A breakpoint was not set */
> > +			if (ret == 0 && ptrace(PTRACE_SYSCALL, pid, NULL, NULL)) {
> 
> This is not symmetrical to parasite unload.
> 
> For parasite:
> 
> 	wake_up_socket()	
> 
> 	if (set_breakpoint())
> 		wait()

I think you are going to apply "[CRIU] [PATCH] ptrace: Skip GETREGS on
exits from syscalls when possible". So we need to call
parasite_stop_on_syscall(1, smth) only after PTRACE_SYSCALL

> 
> 	PTRACE_SYSCALL
> 	while (1) {
> 		wait()
> 		PTRACE_SYSCALL
> 	}
> 
> For restorer
> 
> 	if (set_breakpoint())
> 		/* nothing */
> 	else
> 		PTRACE_SYSCALL
> 
> 	wake_up_futex()
> 
> 	while (1) {
> 		wait()
> 		PTRACE_SYSCALL
> 	}
> 
> Why?
> 
> >  				pr_perror("Unable to start %d", pid);
> >  				return -1;
> >  			}
> > @@ -2674,6 +2679,7 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core)
> >  	if (rst_mem_remap(mem))
> >  		goto err;
> >  
> > +	task_args->breakpoint = &current->rst->breakpoint;
> >  	task_args->task_entries = rst_mem_remap_ptr(task_entries_pos, RM_SHREMAP);
> >  
> >  	task_args->rst_mem = mem;
> > diff --git a/include/restorer.h b/include/restorer.h
> > index a690341..1dd35f9 100644
> > --- a/include/restorer.h
> > +++ b/include/restorer.h
> > @@ -155,6 +155,7 @@ struct task_restore_args {
> >  	struct vdso_symtable		vdso_sym_rt;		/* runtime vdso symbols */
> >  	unsigned long			vdso_rt_parked_at;	/* safe place to keep vdso */
> >  #endif
> > +	void				**breakpoint;
> >  } __aligned(64);
> >  
> >  #define RESTORE_ALIGN_STACK(start, size)	\
> > diff --git a/include/rst_info.h b/include/rst_info.h
> > index e89be02..6b00c50 100644
> > --- a/include/rst_info.h
> > +++ b/include/rst_info.h
> > @@ -54,6 +54,8 @@ struct rst_info {
> >  	struct file_desc	*root;
> >  	bool			has_umask;
> >  	u32			umask;
> > +
> > +	void			*breakpoint;
> >  };
> >  
> >  #endif /* __CR_RST_INFO_H__ */
> > diff --git a/pie/restorer.c b/pie/restorer.c
> > index 11ad574..65bbf01 100644
> > --- a/pie/restorer.c
> > +++ b/pie/restorer.c
> > @@ -321,6 +321,11 @@ static int restore_thread_common(struct rt_sigframe *sigframe,
> >  	return 0;
> >  }
> >  
> > +static void noinline rst_sigreturn(unsigned long new_sp)
> > +{
> > +	ARCH_RT_SIGRETURN(new_sp);
> > +}
> > +
> >  /*
> >   * Threads restoration via sigreturn. Note it's locked
> >   * routine and calls for unlock at the end.
> > @@ -372,7 +377,7 @@ long __export_restore_thread(struct thread_restore_args *args)
> >  	futex_dec_and_wake(&thread_inprogress);
> >  
> >  	new_sp = (long)rt_sigframe + SIGFRAME_OFFSET;
> > -	ARCH_RT_SIGRETURN(new_sp);
> > +	rst_sigreturn(new_sp);
> >  
> >  core_restore_end:
> >  	pr_err("Restorer abnormal termination for %ld\n", sys_getpid());
> > @@ -729,6 +734,7 @@ long __export_restore_task(struct task_restore_args *args)
> >  #endif
> >  
> >  	task_entries = args->task_entries;
> > +	*args->breakpoint = rst_sigreturn;
> >  
> >  	ksigfillset(&act.rt_sa_mask);
> >  	act.rt_sa_handler = sigchld_handler;
> > @@ -1092,7 +1098,7 @@ long __export_restore_task(struct task_restore_args *args)
> >  	 * pure assembly since we don't need any additional
> >  	 * code insns from gcc.
> >  	 */
> > -	ARCH_RT_SIGRETURN(new_sp);
> > +	rst_sigreturn(new_sp);
> >  
> >  core_restore_end:
> >  	futex_abort_and_wake(&task_entries->nr_in_progress);
> > 
>