[CRIU] [PATCH 08/15] restore: don't unmap premmapped private vma-s (v2)

[Pavel Emelyanov]

On 11/02/2012 05:32 PM, Andrey Vagin wrote:
> Private vma-s are mapped before forking children, then they are
> remapped to corrected places in restorer.c.
> 
> In restorer all unneeded vma-s are unmaped. VMA-s from premmapped
> regions should not be unmaped.
> 
> v2: replace guard pages on arithmetic in restorer
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> ---
>  cr-restore.c       |  2 ++
>  include/restorer.h |  2 ++
>  restorer.c         | 28 +++++++++++++++++++++++++---
>  3 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index 0597e6b..7d05c31 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -1603,6 +1603,8 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core)
>  
>  	mem += self_vmas_len;
>  	task_args->tgt_vmas = vma_list_remap(mem, vmas_len, &rst_vma_list);
> +	task_args->premmapped_addr = (unsigned long) premmapped_addr;
> +	task_args->premmapped_len = premmapped_len;
>  	if (!task_args->tgt_vmas)
>  		goto err;
>  
> diff --git a/include/restorer.h b/include/restorer.h
> index 8fa6c1d..377afff 100644
> --- a/include/restorer.h
> +++ b/include/restorer.h
> @@ -99,6 +99,8 @@ struct task_restore_core_args {
>  	struct task_entries		*task_entries;
>  	VmaEntry			*self_vmas;
>  	VmaEntry			*tgt_vmas;
> +	unsigned long			premmapped_addr;
> +	unsigned long			premmapped_len;
>  	rt_sigaction_t			sigchld_act;
>  
>  	struct itimerval		itimers[3];
> diff --git a/restorer.c b/restorer.c
> index 369adb9..6c0d888 100644
> --- a/restorer.c
> +++ b/restorer.c
> @@ -327,6 +327,7 @@ long __export_restore_task(struct task_restore_core_args *args)
>  	long ret = -1;
>  	VmaEntry *vma_entry;
>  	u64 va;
> +	unsigned long premmapped_end = args->premmapped_addr + args->premmapped_len;
>  
>  	struct rt_sigframe *rt_sigframe;
>  	unsigned long new_sp;
> @@ -344,12 +345,33 @@ long __export_restore_task(struct task_restore_core_args *args)
>  	pr_info("Switched to the restorer %d\n", my_pid);
>  
>  	for (vma_entry = args->self_vmas; vma_entry->start != 0; vma_entry++) {
> +		unsigned long addr = vma_entry->start;
> +		unsigned long len;
> +
>  		if (!vma_entry_is(vma_entry, VMA_AREA_REGULAR))
>  			continue;
>  
> -		if (sys_munmap((void *)vma_entry->start, vma_entry_len(vma_entry))) {
> -			pr_err("Munmap fail for %lx\n", vma_entry->start);
> -			goto core_restore_end;
> +		pr_debug("Examine %lx-%lx\n", vma_entry->start, vma_entry->end);
> +
> +		if (addr < args->premmapped_addr) {
> +			if (vma_entry->end >= args->premmapped_addr)
> +				len = args->premmapped_addr - addr;
> +			else
> +				len = vma_entry->end - vma_entry->start;
> +			if (sys_munmap((void *) addr, len)) {
> +				pr_err("munmap fail for %lx - %lx\n", addr, addr + len);
> +				goto core_restore_end;
> +			}
> +		}
> +
> +		if (vma_entry->end > premmapped_end) {
> +			if (vma_entry->start < premmapped_end)
> +				addr = premmapped_end;
> +			len = vma_entry->end - addr;
> +			if (sys_munmap((void *) addr, len)) {
> +				pr_err("munmap fail for %lx - %lx\n", addr, addr + len);
> +				goto core_restore_end;
> +			}

This isn't nice. Plz, rewrite it to look like

for () {
	if (/* skip_this_map */)
		continue;
	sys_munmap()
}

not

for () {
	if (/* map is OK left */)
		sys_munmap();
	if (/* map is OK right */)
		sys_munmap();
}

>  		}
>  	}
>  
>