[CRIU] [RFC PATCH 07/12] lazy-pages: add handling of UFFD_EVENT_REMAP

[Mike Rapoport]

On Tue, Jan 10, 2017 at 05:07:01PM +0300, Pavel Emelyanov wrote:
> On 01/09/2017 11:23 AM, Mike Rapoport wrote:
> > When the restored process calls mremap(), we will see #PF's on the new
> > addresses and we have to create a correspondence between the addresses
> > found in the dump and the actual addresses the process uses. If the
> > mremap() call causes the mapping to grow, the additional part will receive
> > zero pages, as expected.
> > 
> > FIXME: is the mapping shrinks, we will try to fill the dropped part and
> > lazy-pages daemon will fail. It should be possible to track VMA changes in
> > the lazy-pages daemon, but simpler, and, apparently, more correct would be
> > to add UFFD_EVENT_MUNMAP to the kernel.
> > 
> > Signed-off-by: Mike Rapoport <rppt at linux.vnet.ibm.com>
> > ---
> >  criu/uffd.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 46 insertions(+), 1 deletion(-)
> > 
> > diff --git a/criu/uffd.c b/criu/uffd.c
> > index 3bc7bc1..8bf4a14 100644
> > --- a/criu/uffd.c
> > +++ b/criu/uffd.c
> > @@ -53,7 +53,12 @@ struct lazy_iovec {
> >  	unsigned long len;
> >  };
> >  
> > -struct lazy_pages_info;
> > +struct lazy_remap {
> > +	struct list_head l;
> > +	unsigned long from;
> > +	unsigned long to;
> > +	unsigned long len;
> > +};
> >  
> >  struct pf_info {
> >  	unsigned long addr;
> > @@ -65,6 +70,7 @@ struct lazy_pages_info {
> >  
> >  	struct list_head iovs;
> >  	struct list_head pfs;
> > +	struct list_head remaps;
> >  
> >  	struct page_read pr;
> >  
> > @@ -92,6 +98,7 @@ static struct lazy_pages_info *lpi_init(void)
> >  	memset(lpi, 0, sizeof(*lpi));
> >  	INIT_LIST_HEAD(&lpi->iovs);
> >  	INIT_LIST_HEAD(&lpi->pfs);
> > +	INIT_LIST_HEAD(&lpi->remaps);
> >  	INIT_LIST_HEAD(&lpi->l);
> >  	lpi->lpfd.revent = handle_uffd_event;
> >  
> > @@ -101,12 +108,15 @@ static struct lazy_pages_info *lpi_init(void)
> >  static void lpi_fini(struct lazy_pages_info *lpi)
> >  {
> >  	struct lazy_iovec *p, *n;
> > +	struct lazy_remap *p1, *n1;
> >  
> >  	if (!lpi)
> >  		return;
> >  	free(lpi->buf);
> >  	list_for_each_entry_safe(p, n, &lpi->iovs, l)
> >  		xfree(p);
> > +	list_for_each_entry_safe(p1, n1, &lpi->remaps, l)
> > +		xfree(p1);
> >  	if (lpi->lpfd.fd > 0)
> >  		close(lpi->lpfd.fd);
> >  	if (lpi->pr.close)
> > @@ -515,9 +525,17 @@ out:
> >  static int uffd_copy(struct lazy_pages_info *lpi, __u64 address, int nr_pages)
> >  {
> >  	struct uffdio_copy uffdio_copy;
> > +	struct lazy_remap *r;
> >  	unsigned long len = nr_pages * page_size();
> >  	int rc;
> >  
> > +	list_for_each_entry(r, &lpi->remaps, l) {
> > +		if (address >= r->from && address < r->from + r->len) {
> > +			address += (r->to - r->from);
> > +			break;
> > +		}
> > +	}
> > +
> >  	uffdio_copy.dst = address;
> >  	uffdio_copy.src = (unsigned long)lpi->buf;
> >  	uffdio_copy.len = len;
> > @@ -670,9 +688,27 @@ static int handle_madv_dontneed(struct lazy_pages_info *lpi,
> >  	return 0;
> >  }
> >  
> > +static int handle_remap(struct lazy_pages_info *lpi, struct uffd_msg *msg)
> > +{
> > +	struct lazy_remap *remap;
> > +
> > +	remap = xmalloc(sizeof(*remap));
> > +	if (!remap)
> > +		return -1;
> > +
> > +	INIT_LIST_HEAD(&remap->l);
> > +	remap->from = msg->arg.remap.from;
> > +	remap->to = msg->arg.remap.to;
> > +	remap->len = msg->arg.remap.len;
> > +	list_add_tail(&remap->l, &lpi->remaps);
> 
> Shouldn't we punch a hole in original region? Not to handle #PF there?

I don't quite follow you here. The #PF's are delivered to uffd only if the
VMA covering the range is registered with uffd. For moving remaps, the
original address range will not be covered by a VMA with uffd. The only
issue it the shrinking remaps (see changelog).

> Also, what if one region is remapped somewhere, then remapped back? Will
> we have two "patches" for it?
> Also, what if we swap two regs with remap (a->c, b->a, c->b) what kind of
> mess will we have here? %)

You forgot a combination of fork() and mremap() events in arbitrary order ;-)
This makes the things even more funny.

I can try to "remap" iovecs, but I remember that something bothered me when
I first started doing it, just cannot remember what exactly...

> 
> > +
> > +	return 0;
> > +}
> > +
> >  static int handle_page_fault(struct lazy_pages_info *lpi, struct uffd_msg *msg)
> >  {
> >  	struct pf_info *pf;
> > +	struct lazy_remap *r;
> >  	__u64 address;
> >  	int ret;
> >  
> > @@ -695,6 +731,13 @@ static int handle_page_fault(struct lazy_pages_info *lpi, struct uffd_msg *msg)
> >  	}
> >  #endif
> >  
> > +	list_for_each_entry(r, &lpi->remaps, l) {
> > +		if (address >= r->to && address < r->to + r->len) {
> > +			address -= (r->to - r->from);
> > +			break;
> > +		}
> > +	}
> > +
> >  	list_for_each_entry(pf, &lpi->pfs, l)
> >  		if (pf->addr == address)
> >  			return 0;
> > @@ -742,6 +785,8 @@ static int handle_uffd_event(struct epoll_rfd *lpfd)
> >  		return handle_page_fault(lpi, &msg);
> >  	case UFFD_EVENT_MADVDONTNEED:
> >  		return handle_madv_dontneed(lpi, &msg);
> > +	case UFFD_EVENT_REMAP:
> > +		return handle_remap(lpi, &msg);
> >  	default:
> >  		pr_err("unexpected uffd event %u\n", msg.event);
> >  		return -1;
> > 
>