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

[Andrey Vagin]

On Tue, Oct 30, 2012 at 10:56:13PM +0400, Pavel Emelyanov wrote:
> >>
> >>> +			break;
> >>> +		}
> >>> +
> >>> +	}
> >>> +
> >>> +	*pvma = list_entry(p->list.prev, struct vma_area, list);
> >>
> >> prev? Why prev? We should move forward the parent list. No?
> > 
> > list_for_each_entry_continue() starts from a next element, so we need to
> > save prev here.
> 
> Indeed. Then the initialization of the pvma with _first_ element is wrong,
> you should start with the list head instead.

Yes, thanks.
> 
> >>
> >>> +
> >>> +		list_del(&p->list);
> >>> +		xfree(p);
> >>
> >> Why do we remove parent vma from its list?
> > 
> > Then all unused parent's vma-s will be unmapped.
> 
> What do you mean by "unused"?

This vma-s are present in parent and they are not present in a child.

> 
> >>
> >>> +	}
> >>> +
> >>> +	if (vma_entry_is(&vma->vma, VMA_FILE_PRIVATE))
> >>> +		close(vma->vma.fd);
> >>
> >> Plz, explain this close.
> > 
> > This fd was opened upper and a vma was mapped and this fd isn't needed more.
> 
> Upper? Where is it?

static int map_private_vma(pid_t pid, struct vma_area *vma, void
*tgt_addr,
                        struct vma_area **pvma, struct list_head
*pvma_list)
{       
        int ret;
        void *addr;
        unsigned long nr_pages;
        struct vma_area *p = *pvma;
                
        if (vma_entry_is(&vma->vma, VMA_FILE_PRIVATE)) {
                ret = get_filemap_fd(pid, &vma->vma);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> >>
> >>> +
> >>> +		addr = (void *) vma_premmaped_start(&vma->vma);
> >>> +		len = vma_area_len(vma);
> >>> +
> >>> +		if (!addr)
> >>> +			continue;
> >>> +
> >>> +		if (munmap(addr, len)) {
> >>
> >> What is this munmap for?
> > 
> > Why do we need this vma-s, they were inherited from a parent.
> > And one more thing is that all this vma-s will be inherited by
> > children of this process and so on...
> 
> So this is unmap of all parent's maps, right? They were mapped with
> one big chunk, why not unmapping them the same way?
Yes, we can.