[CRIU] Re: [PATCH cr 07/16] restore: map private vma-s before forking children

[Andrew Vagin]

> >  static int read_vmas(int pid)
> >  {
> >  	int fd, ret = 0;
> >  	LIST_HEAD(old);
> > -	struct vma_area *vma;
> > +	struct vma_area *pvma, *vma;
> > +	unsigned long priv_size = 0;
> > +	void *addr;
> >  
> >  	list_replace_init(&vma_list, &old);
> >  	INIT_LIST_HEAD(&vma_list);
> >  
> > +	pvma = list_first_entry(&old, struct vma_area, list);
> 
> Plz, move below, closer to the place it's really required.
> 
> > +
> >  	/* Skip errors, because a zombie doesn't have an image of vmas */
> >  	fd = open_image_ro(CR_FD_VMAS, pid);
> >  	if (fd < 0)
> > @@ -211,14 +273,54 @@ static int read_vmas(int pid)
> >  
> >  		vma->vma = *e;
> >  		vma_entry__free_unpacked(e, NULL);
> > +
> > +		if (!vma_priv(&vma->vma))
> > +			continue;
> > +
> > +		priv_size += vma_area_len(vma);
> > +	}
> > +
> > +	/* Reserve a place for mapping private vma-s one by one */
> > +	addr = mmap(NULL, priv_size, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> > +	if (addr == MAP_FAILED) {
> > +		pr_perror("Unable to reserve memory");
> > +		return -1;
> > +	}
> > +
> > +	list_for_each_entry(vma, &vma_list, list) {
> > +		if (!vma_priv(&vma->vma))
> > +			continue;
> > +
> > +		ret = map_private_vma(pid, vma, addr, &pvma, &old);
> > +		if (ret < 0)
> > +			break;
> > +
> > +		addr += vma_area_len(vma);
> 
> Both addr and pvma propagation should be done in one place for better code readability

I disagee with you here, becuase they are completly different.
pvma is a cursor on a parent vma.
addr is a target address.
I try to move this line in vma_area_len, this looks bad for me...