[CRIU] p.haul page statistics

[Mike Rapoport]

On Tue, Sep 13, 2016 at 06:12:28PM +0300, Pavel Emelyanov wrote:
> On 09/12/2016 03:08 PM, Mike Rapoport wrote:
> > On Mon, Sep 12, 2016 at 11:02:34AM +0200, Adrian Reber wrote:
> >> On Thu, Sep 08, 2016 at 08:39:06AM +0300, Mike Rapoport wrote:
> >>> On Tue, Sep 06, 2016 at 11:35:23AM +0200, Adrian Reber wrote:
> >>>> Using the latest criu with p.haul the information about the pages dumped
> >>>> looks a bit wrong. From p.haul I get:
> > 
> > [ snip ]
> >  
> >>>> So p.haul is kind of right as the stats file says that only 13 pages
> >>>> have been written. But all the lazy pages have also been written.
> >>>>
> >>>> Should pages_written include the lazy pages or should p.haul add
> >>>> pages_written and pages_lazy to get the actual number of pages written?
> >>>
> >>> I think p.haul should add pages_written and pages_lazy.
> >>
> >> I thought some more about this and I am not convinced. 'pages_written'
> >> sounds like all pages written. Unrelated if the pages are lazy or not.
> >  
> > Agree. How about the patch below?
> > 
> >> Are there any advantages I do not see if the lazy pages are not included
> >> in 'pages_written'? In the future when p.haul might know how to combine
> >> pre-copy and post-copy the variable 'pages_lazy' will become important
> >> but right now it feels wrong to read it to decide if additional pre-copy
> >> runs should be performed.
> >>
> >> 		Adrian
> >>
> > 
> >>From 898ab269a6b4a47ed8019445b688dfd3d41d0581 Mon Sep 17 00:00:00 2001
> > From: Mike Rapoport <rppt at linux.vnet.ibm.com>
> > Date: Mon, 12 Sep 2016 15:05:13 +0300
> > Subject: [CRIU][PATCH] criu: mem: count all pages actually written to image as
> >  "pages_written"
> > 
> > When potentially lazy pages are written to image add their count to
> > "pages_written" stats counter.
> 
> Erm... I saw Adrian's thoughts on counting lazy pages as written, but now I
> disagree with that. If we treat pages_written as "pages that went into the
> image file", then this definition is clean and understandable. But how to
> define pages_written if we count lazy pages there as well?
> 
> Can we better leave pages_written as "pages that are in the images" and, if
> we need it, introduce pages_dumped to count all the pages that are to be
> taken with us regardless whether they are in the image or lazy?

Well, PAGES_LAZY have broken notion of pages_written as "pages that
are in the images". With this patch it's restored:
- if dump writes potentially lazy pages to the image, they are counted as
  written
- if dump will go to server mode to transfer pages on demand, the lazy
  pages are not written to images and CNT_PAGES_WRITTEN does not count
them 
 
> -- Pavel
> 
> > Signed-off-by: Mike Rapoport <rppt at linux.vnet.ibm.com>
> > ---
> >  criu/mem.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/criu/mem.c b/criu/mem.c
> > index 597bf29..b9b0bf4 100644
> > --- a/criu/mem.c
> > +++ b/criu/mem.c
> > @@ -192,7 +192,8 @@ static int generate_iovs(struct vma_area *vma, struct page_pipe *pp, u64 *map, u
> >  	cnt_add(CNT_PAGES_ZERO, pages[0]);
> >  	cnt_add(CNT_PAGES_SKIPPED_PARENT, pages[1]);
> >  	cnt_add(CNT_PAGES_LAZY, pages[2]);
> > -	cnt_add(CNT_PAGES_WRITTEN, pages[3]);
> > +	cnt_add(CNT_PAGES_WRITTEN,
> > +		opts.lazy_pages ? pages[3] : pages[2] + pages[3]);
> >  
> >  	pr_info("Pagemap generated: %lu pages (%lu lazy) %lu holes %lu zeros\n",
> >  		pages[3] + pages[2], pages[2], pages[1], pages[0]);
> > 
>