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

[Pavel Emelyanov]

On 09/17/2014 09:21 PM, Andrew Vagin wrote:
> 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

I know, i wrote the above snippets keeping this patch applied.

>>
>> 	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);
>>>
>>
> .
>