[CRIU] [PATCH] compel: pass regs by pointer in get_task_regs()

Andrei Vagin avagin at virtuozzo.com
Thu Mar 9 08:31:26 PST 2017


Applied

On Tue, Mar 07, 2017 at 03:55:29PM +0300, Dmitry Safonov wrote:
> CID 73371 (#1 of 1): Big parameter passed by value (PASS_BY_VALUE)
> pass_by_value: Passing parameter regs of type user_regs_struct_t
> (size 224 bytes) by value.
> 
> Suggesting to do this until compel is released and API is cut in stone.
> 
> Signed-off-by: Dmitry Safonov <dsafonov at virtuozzo.com>
> ---
>  compel/arch/aarch64/src/lib/infect.c |  6 +++---
>  compel/arch/arm/src/lib/infect.c     | 16 ++++++++--------
>  compel/arch/ppc64/src/lib/infect.c   |  6 +++---
>  compel/arch/x86/src/lib/infect.c     | 16 ++++++++--------
>  compel/include/infect-priv.h         |  2 +-
>  compel/src/lib/infect.c              |  4 ++--
>  6 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/compel/arch/aarch64/src/lib/infect.c b/compel/arch/aarch64/src/lib/infect.c
> index 4f5534b75530..41600e0914ef 100644
> --- a/compel/arch/aarch64/src/lib/infect.c
> +++ b/compel/arch/aarch64/src/lib/infect.c
> @@ -56,7 +56,7 @@ int sigreturn_prep_fpu_frame_plain(struct rt_sigframe *sigframe,
>  	return 0;
>  }
>  
> -int get_task_regs(pid_t pid, user_regs_struct_t regs, save_regs_t save, void *arg)
> +int get_task_regs(pid_t pid, user_regs_struct_t *regs, save_regs_t save, void *arg)
>  {
>  	struct iovec iov;
>  	user_fpregs_struct_t fpsimd;
> @@ -64,7 +64,7 @@ int get_task_regs(pid_t pid, user_regs_struct_t regs, save_regs_t save, void *ar
>  
>  	pr_info("Dumping GP/FPU registers for %d\n", pid);
>  
> -	iov.iov_base = &regs;
> +	iov.iov_base = regs;
>  	iov.iov_len = sizeof(user_regs_struct_t);
>  	if ((ret = ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov))) {
>  		pr_perror("Failed to obtain CPU registers for %d", pid);
> @@ -78,7 +78,7 @@ int get_task_regs(pid_t pid, user_regs_struct_t regs, save_regs_t save, void *ar
>  		goto err;
>  	}
>  
> -	ret = save(arg, &regs, &fpsimd);
> +	ret = save(arg, regs, &fpsimd);
>  err:
>  	return ret;
>  }
> diff --git a/compel/arch/arm/src/lib/infect.c b/compel/arch/arm/src/lib/infect.c
> index ad085ff98e07..a78108dff30f 100644
> --- a/compel/arch/arm/src/lib/infect.c
> +++ b/compel/arch/arm/src/lib/infect.c
> @@ -65,7 +65,7 @@ int sigreturn_prep_fpu_frame_plain(struct rt_sigframe *sigframe,
>  }
>  
>  #define PTRACE_GETVFPREGS 27
> -int get_task_regs(pid_t pid, user_regs_struct_t regs, save_regs_t save, void *arg)
> +int get_task_regs(pid_t pid, user_regs_struct_t *regs, save_regs_t save, void *arg)
>  {
>  	user_fpregs_struct_t vfp;
>  	int ret = -1;
> @@ -78,23 +78,23 @@ int get_task_regs(pid_t pid, user_regs_struct_t regs, save_regs_t save, void *ar
>  	}
>  
>  	/* Did we come from a system call? */
> -	if ((int)regs.ARM_ORIG_r0 >= 0) {
> +	if ((int)regs->ARM_ORIG_r0 >= 0) {
>  		/* Restart the system call */
> -		switch ((long)(int)regs.ARM_r0) {
> +		switch ((long)(int)regs->ARM_r0) {
>  		case -ERESTARTNOHAND:
>  		case -ERESTARTSYS:
>  		case -ERESTARTNOINTR:
> -			regs.ARM_r0 = regs.ARM_ORIG_r0;
> -			regs.ARM_pc -= 4;
> +			regs->ARM_r0 = regs->ARM_ORIG_r0;
> +			regs->ARM_pc -= 4;
>  			break;
>  		case -ERESTART_RESTARTBLOCK:
> -			regs.ARM_r0 = __NR_restart_syscall;
> -			regs.ARM_pc -= 4;
> +			regs->ARM_r0 = __NR_restart_syscall;
> +			regs->ARM_pc -= 4;
>  			break;
>  		}
>  	}
>  
> -	ret = save(arg, &regs, &vfp);
> +	ret = save(arg, regs, &vfp);
>  err:
>  	return ret;
>  }
> diff --git a/compel/arch/ppc64/src/lib/infect.c b/compel/arch/ppc64/src/lib/infect.c
> index 11154d6580fd..f3f1aacec854 100644
> --- a/compel/arch/ppc64/src/lib/infect.c
> +++ b/compel/arch/ppc64/src/lib/infect.c
> @@ -369,16 +369,16 @@ static int __get_task_regs(pid_t pid, user_regs_struct_t *regs,
>  	return 0;
>  }
>  
> -int get_task_regs(pid_t pid, user_regs_struct_t regs, save_regs_t save, void *arg)
> +int get_task_regs(pid_t pid, user_regs_struct_t *regs, save_regs_t save, void *arg)
>  {
>  	user_fpregs_struct_t fpregs;
>  	int ret;
>  
> -	ret = __get_task_regs(pid, &regs, &fpregs);
> +	ret = __get_task_regs(pid, regs, &fpregs);
>  	if (ret)
>  		return ret;
>  
> -	return save(arg, &regs, &fpregs);
> +	return save(arg, regs, &fpregs);
>  }
>  
>  int compel_syscall(struct parasite_ctl *ctl, int nr, long *ret,
> diff --git a/compel/arch/x86/src/lib/infect.c b/compel/arch/x86/src/lib/infect.c
> index f1b216650ec0..84ff21b15729 100644
> --- a/compel/arch/x86/src/lib/infect.c
> +++ b/compel/arch/x86/src/lib/infect.c
> @@ -225,7 +225,7 @@ int sigreturn_prep_fpu_frame_plain(struct rt_sigframe *sigframe,
>  	((user_regs_native(pregs)) ? (int64_t)((pregs)->native.name) :	\
>  				(int32_t)((pregs)->compat.name))
>  
> -int get_task_regs(pid_t pid, user_regs_struct_t regs, save_regs_t save, void *arg)
> +int get_task_regs(pid_t pid, user_regs_struct_t *regs, save_regs_t save, void *arg)
>  {
>  	user_fpregs_struct_t xsave	= {  }, *xs = NULL;
>  
> @@ -233,21 +233,21 @@ int get_task_regs(pid_t pid, user_regs_struct_t regs, save_regs_t save, void *ar
>  	int ret = -1;
>  
>  	pr_info("Dumping general registers for %d in %s mode\n", pid,
> -			user_regs_native(&regs) ? "native" : "compat");
> +			user_regs_native(regs) ? "native" : "compat");
>  
>  	/* Did we come from a system call? */
> -	if (get_signed_user_reg(&regs, orig_ax) >= 0) {
> +	if (get_signed_user_reg(regs, orig_ax) >= 0) {
>  		/* Restart the system call */
> -		switch (get_signed_user_reg(&regs, ax)) {
> +		switch (get_signed_user_reg(regs, ax)) {
>  		case -ERESTARTNOHAND:
>  		case -ERESTARTSYS:
>  		case -ERESTARTNOINTR:
> -			set_user_reg(&regs, ax, get_user_reg(&regs, orig_ax));
> -			set_user_reg(&regs, ip, get_user_reg(&regs, ip) - 2);
> +			set_user_reg(regs, ax, get_user_reg(regs, orig_ax));
> +			set_user_reg(regs, ip, get_user_reg(regs, ip) - 2);
>  			break;
>  		case -ERESTART_RESTARTBLOCK:
>  			pr_warn("Will restore %d with interrupted system call\n", pid);
> -			set_user_reg(&regs, ax, -EINTR);
> +			set_user_reg(regs, ax, -EINTR);
>  			break;
>  		}
>  	}
> @@ -279,7 +279,7 @@ int get_task_regs(pid_t pid, user_regs_struct_t regs, save_regs_t save, void *ar
>  
>  	xs = &xsave;
>  out:
> -	ret = save(arg, &regs, xs);
> +	ret = save(arg, regs, xs);
>  err:
>  	return ret;
>  }
> diff --git a/compel/include/infect-priv.h b/compel/include/infect-priv.h
> index cf0ce3a927fb..0f80895a7be8 100644
> --- a/compel/include/infect-priv.h
> +++ b/compel/include/infect-priv.h
> @@ -58,7 +58,7 @@ extern void *remote_mmap(struct parasite_ctl *ctl,
>  		void *addr, size_t length, int prot,
>  		int flags, int fd, off_t offset);
>  extern bool arch_can_dump_task(struct parasite_ctl *ctl);
> -extern int get_task_regs(pid_t pid, user_regs_struct_t regs, save_regs_t save, void *arg);
> +extern int get_task_regs(pid_t pid, user_regs_struct_t *regs, save_regs_t save, void *arg);
>  extern int sigreturn_prep_regs_plain(struct rt_sigframe *sigframe,
>  				     user_regs_struct_t *regs,
>  				     user_fpregs_struct_t *fpregs);
> diff --git a/compel/src/lib/infect.c b/compel/src/lib/infect.c
> index 78c9655efb7b..c02425036e63 100644
> --- a/compel/src/lib/infect.c
> +++ b/compel/src/lib/infect.c
> @@ -664,7 +664,7 @@ static int parasite_start_daemon(struct parasite_ctl *ctl)
>  	 * while in daemon it is not such.
>  	 */
>  
> -	if (get_task_regs(pid, ctl->orig.regs, ictx->save_regs, ictx->regs_arg)) {
> +	if (get_task_regs(pid, &ctl->orig.regs, ictx->save_regs, ictx->regs_arg)) {
>  		pr_err("Can't obtain regs for thread %d\n", pid);
>  		return -1;
>  	}
> @@ -1556,7 +1556,7 @@ k_rtsigset_t *compel_task_sigmask(struct parasite_ctl *ctl)
>  
>  int compel_get_thread_regs(struct parasite_thread_ctl *tctl, save_regs_t save, void * arg)
>  {
> -	return get_task_regs(tctl->tid, tctl->th.regs, save, arg);
> +	return get_task_regs(tctl->tid, &tctl->th.regs, save, arg);
>  }
>  
>  struct infect_ctx *compel_infect_ctx(struct parasite_ctl *ctl)
> -- 
> 2.11.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu


More information about the CRIU mailing list