RE: [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Li, Liang Z]

> On Mon, Apr 25, 2016 at 03:11:05AM +0000, Li, Liang Z wrote:
> > > On Fri, Apr 22, 2016 at 10:48:38AM +0100, Dr. David Alan Gilbert wrote:
> > > > * Michael S. Tsirkin (mst@xxxxxxxxxx) wrote:
> > > > > On Tue, Apr 19, 2016 at 03:02:09PM +0000, Li, Liang Z wrote:
> > > > > > > On Tue, 2016-04-19 at 22:34 +0800, Liang Li wrote:
> > > > > > > > The free page bitmap will be sent to QEMU through virtio
> > > > > > > > interface and used for live migration optimization.
> > > > > > > > Drop the cache before building the free page bitmap can
> > > > > > > > get more free pages. Whether dropping the cache is decided by
> user.
> > > > > > > >
> > > > > > >
> > > > > > > How do you prevent the guest from using those recently-freed
> > > > > > > pages for something else, between when you build the bitmap
> > > > > > > and the live migration completes?
> > > > > >
> > > > > > Because the dirty page logging is enabled before building the
> > > > > > bitmap, there is no need to prevent the guest from using the
> > > > > > recently-
> > > freed pages ...
> > > > > >
> > > > > > Liang
> > > > >
> > > > > Well one point of telling host that page is free is so that it
> > > > > can mark it clean even if it was dirty previously.
> > > > > So I think you must pass the pages to guest under the lock.
> > > > > This will allow host optimizations such as marking these pages
> > > > > MADV_DONTNEED or MADV_FREE.
> > > > > Otherwise it's all too tied up to a specific usecase - you
> > > > > aren't telling host that a page is free, you are telling it that
> > > > > a page was free in the past.
> > > >
> > > > But doing it under lock sounds pretty expensive, especially given
> > > > how long the userspace side is going to take to work through the
> > > > bitmap and device what to do.
> > > >
> > > > Dave
> > >
> > > We need to make it as fast as we can since the VCPU is stopped on
> > > exit anyway. This just means e.g. sizing the bitmap reasonably -
> > > don't always try to fit all memory in a single bitmap.
> >
> > Then we should pause the whole VM when using the bitmap, too
> expensive?
> 
> Why should we? I don't get it. Just make sure that at the point when you give
> a page to host, it's not in use. Host can clear the dirty bitmap, discard the
> page, or whatever.
> 
I did not know you mean to put the page into balloon. 
There is no need to pause the VM if you do in that way.

> > > Really, if the page can in fact be in use when you tell host it's
> > > free, then it's rather hard to explain what does it mean from
> > > host/guest interface point of view.
> > >
> >
> > How about rename the interface to a more appropriate name other than
> 'free page' ?
> >
> > Liang.
> 
> Maybe. But start with a description.
> 
> The way I figured is passing a page to host meant putting it in the balloon and
> immediately taking it out again. this allows things like discarding it since while
> page is in the balloon, it is owned by the balloon.
> 
> This aligns well with how balloon works today.
>
 > 
> If not that, then what can it actually mean?
> 
> Without a lock, the only thing we can make it mean is that the page is in the
> balloon at some point after the report is requested and before it's passed to
> balloon.
> 
> This happens to work if you only have one page in the balloon, but to make it
> asynchronous you really have to pass in a request ID, and then return it back
> with the bitmap. This way we can say "this page was free sometime after
> host sent request with this ID and before it received response with the same
> ID".
> 
> And then, what host is supposed to do for pre-copy, copy the dirty bitmap
> before sending request, then on response we clear bit in this bitmap copy,
> then we set bits received from kvm (or another backend) afterwards.
> 
> Of course just not retrieving the bitmap from kvm until we get a response
> also works (this is what your patches did) and then you do not need a copy,
> but that's inelegant because this means guest can defer completing
> migration.

My RFC version patch did like this, but this version I changed the behavior,
now there is no waiting before starting live migration.

> 
> So this works for migration but not for discarding pages.
> 
> For this reason I think as a first step, we should focus on the simpler
> approach where we keep the lock.  Then add a feature bit that allows
> dropping the lock.
> 
> 

I got you this time,  but I still don't think put the free page in the balloon is a good
idea for live migration optimization. There is no need to do extra things which increases
the guest's overhead, it's not worth the candle.

We can do something this to optimize the current virtio-balloon's performance. 
but not for live migration, the efficiency should be the first thing we consider
about, or we run the risk of blocking user from using this new feature.
 
Liang
> 
> 
> > > It probably can be defined but the interface seems very complex.
> > >
> > > Let's start with a simple thing instead unless it can be shown that
> > > there's a performance problem.
> > >
> > >
> > > > >
> > > > > --
> > > > > MST
> > > > --
> > > > Dr. David Alan Gilbert / dgilbert@xxxxxxxxxx / Manchester, UK