[CRIU] [PATCH 7/8] sig: Optimize sigactions restore

[Andrew Vagin]

On Wed, Aug 06, 2014 at 06:37:12PM +0400, Pavel Emelyanov wrote:
> On 08/06/2014 05:01 PM, Andrew Vagin wrote:
> > On Wed, Aug 06, 2014 at 04:25:38PM +0400, Pavel Emelyanov wrote:
> >> Most of the sigactions are the same across the tasks in the image.
> >> Nonetheless existing code always calls a syscall to restore them 
> >> and spends 64 calls per-task.
> >>
> >> Let's restore signals before forking children and let them inherit
> >> sigactions. Tune one only if it differs from the parent's.
> > 
> > I like the idea. Do we have test, which uses different signal actions
> > for parent and child?
> 
> All timers tests are such. The pdeath_sig one is. The file_fown.

Acked-by: Andrew Vagin <avagin at parallels.com>

> 
> >>
> >> Signed-off-by: Pavel Emelyanov <xemul at parallels.com>
> >> ---
> >>  cr-restore.c | 32 ++++++++++++++++++++++++++++----
> >>  1 file changed, 28 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/cr-restore.c b/cr-restore.c
> >> index afdafd8..0408aa3 100644
> >> --- a/cr-restore.c
> >> +++ b/cr-restore.c
> >> @@ -614,6 +614,22 @@ static int open_vmas(int pid)
> >>  }
> >>  
> >>  static rt_sigaction_t sigchld_act;
> >> +static rt_sigaction_t parent_act[SIGMAX];
> >> +
> >> +static bool sa_inherited(int sig, rt_sigaction_t *sa)
> >> +{
> >> +	rt_sigaction_t *pa;
> >> +
> >> +	if (current == root_item)
> >> +		return false; /* XXX -- inherit from CRIU? */
> >> +
> >> +	pa = &parent_act[sig];
> >> +	return pa->rt_sa_handler == sa->rt_sa_handler &&
> >> +		pa->rt_sa_flags == sa->rt_sa_flags &&
> >> +		pa->rt_sa_restorer == sa->rt_sa_restorer &&
> >> +		pa->rt_sa_mask.sig[0] == sa->rt_sa_mask.sig[0];
> >> +}
> >> +
> >>  static int prepare_sigactions(void)
> >>  {
> >>  	int pid = current->pid.virt;
> >> @@ -621,7 +637,7 @@ static int prepare_sigactions(void)
> >>  	int fd_sigact;
> >>  	SaEntry *e;
> >>  	int sig;
> >> -	int ret = -1;
> >> +	int ret = 0;
> >>  
> >>  	fd_sigact = open_image(CR_FD_SIGACT, O_RSTR, pid);
> >>  	if (fd_sigact < 0)
> >> @@ -655,6 +671,10 @@ static int prepare_sigactions(void)
> >>  			sigchld_act = act;
> >>  			continue;
> >>  		}
> >> +
> >> +		if (sa_inherited(sig, &act))
> >> +			continue;
> >> +
> >>  		/*
> >>  		 * A pure syscall is used, because glibc
> >>  		 * sigaction overwrites se_restorer.
> >> @@ -664,6 +684,8 @@ static int prepare_sigactions(void)
> >>  			pr_err("%d: Can't restore sigaction: %m\n", pid);
> >>  			goto err;
> >>  		}
> >> +
> >> +		parent_act[sig] = act;
> >>  	}
> >>  
> >>  err:
> >> @@ -716,9 +738,6 @@ static int restore_one_alive_task(int pid, CoreEntry *core)
> >>  	if (prepare_file_locks(pid))
> >>  		return -1;
> >>  
> >> -	if (prepare_sigactions())
> >> -		return -1;
> >> -
> >>  	if (open_vmas(pid))
> >>  		return -1;
> >>  
> >> @@ -1346,6 +1367,9 @@ static int restore_task_with_children(void *_arg)
> >>  	if (prepare_task_cgroup(current) < 0)
> >>  		return -1;
> >>  
> >> +	if (prepare_sigactions() < 0)
> >> +		return -1;
> >> +
> >>  	if (create_children_and_session())
> >>  		goto err;
> >>  
> >> -- 
> >> 1.8.4.2
> >>
> >>
> >> _______________________________________________
> >> CRIU mailing list
> >> CRIU at openvz.org
> >> https://lists.openvz.org/mailman/listinfo/criu
> > .
> > 
>