[CRIU] [PATCH] mem: Don't do unneeded mprotects

[Andrei Vagin]

Applied, thanks
On Mon, May 15, 2017 at 05:14:31PM +0300, Pavel Emelyanov wrote:
> When a vma we restore doesn't have any pages in pagemaps there's
> not need to enforce PROT_WRITE bit on it.
> 
> This only applies to non-premmaped vmas.
> 
> Signed-off-by: Pavel Emelyanov <xemul at virtuozzo.com>
> ---
>  criu/include/image.h |  1 +
>  criu/mem.c           | 27 ++++++++++++++++++++++++++-
>  criu/pie/restorer.c  |  6 ++++--
>  3 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/criu/include/image.h b/criu/include/image.h
> index 24fb828..1dcd18c 100644
> --- a/criu/include/image.h
> +++ b/criu/include/image.h
> @@ -89,6 +89,7 @@
>  #define VMA_AREA_VVAR		(1 <<  12)
>  #define VMA_AREA_AIORING	(1 <<  13)
>  
> +#define VMA_NO_PROT_WRITE	(1 <<  29)
>  #define VMA_PREMMAPED		(1 <<  30)
>  #define VMA_UNSUPP		(1 <<  31)
>  
> diff --git a/criu/mem.c b/criu/mem.c
> index 942d5d9..db7279a 100644
> --- a/criu/mem.c
> +++ b/criu/mem.c
> @@ -690,6 +690,13 @@ static int premap_private_vma(struct pstree_item *t, struct vma_area *vma, void
>  			}
>  		}
>  
> +		/*
> +		 * All mappings here get PROT_WRITE regardless of whether we
> +		 * put any data into it or not, because this area will get
> +		 * mremap()-ed (branch below) so we MIGHT need to have WRITE 
> +		 * bits there. Ideally we'd check for the whole COW-chain
> +		 * having any data in.
> +		 */
>  		addr = mmap(*tgt_addr, size,
>  				vma->e->prot | PROT_WRITE,
>  				vma->e->flags | MAP_FIXED | flag,
> @@ -786,14 +793,26 @@ static int premap_priv_vmas(struct pstree_item *t, struct vm_area_list *vmas,
>  		if (!vma_area_is_private(vma, kdat.task_size))
>  			continue;
>  
> -		if (vma->pvma == NULL && pr->pieok && !vma_force_premap(vma, &vmas->h))
> +		if (vma->pvma == NULL && pr->pieok && !vma_force_premap(vma, &vmas->h)) {
>  			/*
>  			 * VMA in question is not shared with anyone. We'll
>  			 * restore it with its contents in restorer.
> +			 * Now let's check whether we need to map it with
> +			 * PROT_WRITE or not.
>  			 */
> +			do {
> +				if (pr->pe->vaddr + pr->pe->nr_pages * PAGE_SIZE <= vma->e->start)
> +					continue;
> +				if (pr->pe->vaddr > vma->e->end)
> +					vma->e->status |= VMA_NO_PROT_WRITE;
> +				break;
> +			} while (pr->advance(pr));
> +
>  			continue;
> +		}
>  
>  		ret = premap_private_vma(t, vma, at);
> +
>  		if (ret < 0)
>  			break;
>  	}
> @@ -879,6 +898,12 @@ static int restore_priv_vma_content(struct pstree_item *t, struct page_read *pr)
>  						(nr_pages - i) * PAGE_SIZE,
>  						vma->e->end - va);
>  
> +				if (vma->e->status & VMA_NO_PROT_WRITE) {
> +					pr_debug("VMA 0x%"PRIx64":0x%"PRIx64" RO %#lx:%lu IO\n",
> +							vma->e->start, vma->e->end, va, nr_pages);
> +					BUG();
> +				}
> +
>  				if (pagemap_enqueue_iovec(pr, (void *)va, len, vma_io))
>  					return -1;
>  
> diff --git a/criu/pie/restorer.c b/criu/pie/restorer.c
> index 98c81f3..1758883 100644
> --- a/criu/pie/restorer.c
> +++ b/criu/pie/restorer.c
> @@ -621,7 +621,8 @@ static unsigned long restore_mapping(VmaEntry *vma_entry)
>  		flags |= MAP_ANONYMOUS;
>  
>  	/* A mapping of file with MAP_SHARED is up to date */
> -	if (vma_entry->fd == -1 || !(vma_entry->flags & MAP_SHARED))
> +	if ((vma_entry->fd == -1 || !(vma_entry->flags & MAP_SHARED)) &&
> +			!(vma_entry->status & VMA_NO_PROT_WRITE))
>  		prot |= PROT_WRITE;
>  
>  	pr_debug("\tmmap(%"PRIx64" -> %"PRIx64", %x %x %d)\n",
> @@ -1348,7 +1349,8 @@ long __export_restore_task(struct task_restore_args *args)
>  		if (!(vma_entry_is(vma_entry, VMA_AREA_REGULAR)))
>  			continue;
>  
> -		if (vma_entry->prot & PROT_WRITE)
> +		if ((vma_entry->prot & PROT_WRITE) ||
> +				(vma_entry->status & VMA_NO_PROT_WRITE))
>  			continue;
>  
>  		sys_mprotect(decode_pointer(vma_entry->start),
> -- 
> 2.1.4
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu