[CRIU] Re: [PATCH cr] restore: add helper for getting a temporary address of vma

[Pavel Emelyanov]

On 10/18/2012 07:26 PM, Andrey Vagin wrote:
>> For me it will be OK if you just use some #define or static inline helper
>> titled e.g. vma_premmaped_start(vma) that will look like "return vma->shimd".
> 
> Pavel, is it what you want?

Yes, thanks.

> It will not be commit as this. This patch will be merged in other patches.
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> ---
>  cr-restore.c    |   33 +++++++++++++++++++--------------
>  include/image.h |    1 +
>  restorer.c      |    4 ++--
>  3 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index c8cb690..2f6541f 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -208,7 +208,7 @@ static int map_private_vma(pid_t pid, struct vma_area *vma,
>  		    p->vma.start == vma->vma.start) {
>  			pr_info("COW 0x%016lx-0x%016lx 0x%016lx vma\n",
>  				vma->vma.start, vma->vma.end, vma->vma.pgoff);
> -			vma->vma.shmid = p->vma.shmid;
> +			vma->vma.shmid = vma_entry_tmp_addr(&p->vma);
>  			break;
>  		}
>  
> @@ -216,7 +216,7 @@ static int map_private_vma(pid_t pid, struct vma_area *vma,
>  
>  	*pvma = list_entry(p->list.prev, struct vma_area, list);
>  
> -	if (!vma->vma.shmid) {
> +	if (!vma_entry_tmp_addr(&vma->vma)) {
>  		void *addr;
>  
>  		pr_info("Map 0x%016lx-0x%016lx 0x%016lx vma\n",
> @@ -287,7 +287,8 @@ static int restore_anon_vma_content(pid_t pid)
>  			return -1;
>  		}
>  
> -		p = (void *) (va - vma->vma.start + vma->vma.shmid);
> +		p = (void *) (va - vma->vma.start +
> +					vma_entry_tmp_addr(&vma->vma));
>  		if (memcmp(p, buf, PAGE_SIZE) == 0)
>  			continue;
>  
> @@ -298,6 +299,7 @@ static int restore_anon_vma_content(pid_t pid)
>  	/* Remove pages, which were not shared with a child */
>  	list_for_each_entry(vma, &vma_list, list) {
>  		unsigned long size, i = 0;
> +		void *addr = (void *) vma_entry_tmp_addr(&vma->vma);
>  
>  		if (vma->ppage_bitmap == NULL)
>  			continue;
> @@ -310,7 +312,7 @@ static int restore_anon_vma_content(pid_t pid)
>  			if ( i >= size)
>  				break;
>  
> -			ret = madvise((void *)(vma->vma.shmid + PAGE_SIZE * i),
> +			ret = madvise(addr + PAGE_SIZE * i,
>  						PAGE_SIZE, MADV_DONTNEED);
>  			if (ret < 0) {
>  				pr_perror("madvise failed\n");
> @@ -1476,6 +1478,7 @@ static int mark_target_vmas(struct list_head *self_vma_list)
>  	/* Some VMA may be merged, so they are should be splitted */
>  	while (&vma->list != self_vma_list) {
>  		struct vma_area *c, *n;
> +		unsigned long caddr;
>  
>  		c = NULL;
>  		len++;
> @@ -1483,6 +1486,7 @@ static int mark_target_vmas(struct list_head *self_vma_list)
>  		/* Try to find a first target vma */
>  		list_for_each_entry(n, &vma_list, list) {
>  			unsigned long start = vma->vma.start;
> +			unsigned long naddr = vma_entry_tmp_addr(&n->vma);
>  
>  			if (!vma_priv(&n->vma))
>  				continue;
> @@ -1490,10 +1494,11 @@ static int mark_target_vmas(struct list_head *self_vma_list)
>  			if (n->vma.flags & MAP_GROWSDOWN)
>  				start -= PAGE_SIZE; /* guard page */
>  
> -			if (n->vma.shmid >= start &&
> -			    n->vma.shmid < vma->vma.end &&
> -			    (c == NULL || c->vma.shmid > n->vma.shmid))
> +			if (naddr >= start && naddr < vma->vma.end &&
> +			    (c == NULL || caddr > naddr)) {
>  				c = n;
> +				caddr = naddr;
> +			}
>  		}
>  
>  		if (c == NULL) {
> @@ -1504,20 +1509,20 @@ static int mark_target_vmas(struct list_head *self_vma_list)
>  		}
>  
>  		pr_debug("Mark VMA 0x%016lx-0x%016lx inside 0x%016lx-0x%016lx\n",
> -				c->vma.shmid, c->vma.shmid + vma_area_len(c),
> +				caddr, caddr + vma_area_len(c),
>  				vma->vma.start, vma->vma.end);
>  
>  		/* A first part isn't a target VMA */
> -		if (c->vma.shmid > vma->vma.start) {
> +		if (caddr > vma->vma.start) {
>  			BUG_ON(c->vma.flags & MAP_GROWSDOWN);
>  			new = alloc_vma_area();
>  			if (new == NULL)
>  				return -1;
>  			memcpy(new, vma, sizeof(*vma));
>  
> -			vma->vma.start = c->vma.shmid;
> +			vma->vma.start = caddr;
>  
> -			new->vma.end = c->vma.shmid;
> +			new->vma.end = caddr;
>  			list_add(&new->list, &new_list);
>  			len++;
>  		}
> @@ -1526,15 +1531,15 @@ static int mark_target_vmas(struct list_head *self_vma_list)
>  			vma->vma.start -= PAGE_SIZE; /* guard page */
>  
>  		/* Split a target VMA and handle a remaining part again */
> -		if (c->vma.shmid + vma_area_len(c) < vma->vma.end) {
> +		if (caddr + vma_area_len(c) < vma->vma.end) {
>  			new = alloc_vma_area();
>  			if (new == NULL)
>  				return -1;
>  			memcpy(new, vma, sizeof(*vma));
>  
> -			new->vma.start = c->vma.shmid + vma_area_len(c);
> +			new->vma.start = caddr + vma_area_len(c);
>  
> -			vma->vma.end = c->vma.shmid + vma_area_len(c);
> +			vma->vma.end = caddr + vma_area_len(c);
>  			vma->vma.status |= VMA_AREA_DEST;
>  			list_move_tail(&vma->list, &new_list);
>  
> diff --git a/include/image.h b/include/image.h
> index 4fb4eab..616badc 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -95,6 +95,7 @@
>  
>  #define VMA_AREA_DEST		(1 <<  11)
>  
> +#define vma_entry_tmp_addr(vma) ((vma)->shmid)
>  #define vma_entry_is(vma, s)	(((vma)->status & (s)) == (s))
>  #define vma_entry_len(vma)	((vma)->end - (vma)->start)
>  
> diff --git a/restorer.c b/restorer.c
> index 48aef03..3d21f83 100644
> --- a/restorer.c
> +++ b/restorer.c
> @@ -340,12 +340,12 @@ long __export_restore_task(struct task_restore_core_args *args)
>  		if (!vma_priv(vma_entry))
>  			continue;
>  
> -		va = sys_mremap(vma_entry->shmid,
> +		va = sys_mremap(vma_entry_tmp_addr(vma_entry),
>  				vma_entry_len(vma_entry), vma_entry_len(vma_entry),
>  				MREMAP_MAYMOVE | MREMAP_FIXED, vma_entry->start);
>  		if (va != vma_entry->start) {
>  			pr_err("Unable to remap %lx -> %lx mapping with %lx\n",
> -				vma_entry->shmid, vma_entry->start, va);
> +				vma_entry_tmp_addr(vma_entry), vma_entry->start, va);
>  			goto core_restore_end;
>  		}
>  	}
>