[Qemu-devel] [PATCH kernel 0/2] speed up live migration by skipping free pages

[Qemu-devel] [PATCH kernel 0/2] speed up live migration by skipping free pages

[Liang Li]

Current QEMU live migration implementation mark all guest's RAM pages
as dirtied in the ram bulk stage, all these pages will be processed
and it consumes quite a lot of CPU cycles and network bandwidth.

>From guest's point of view, it doesn't care about the content in free
page. We can make use of this fact and skip processing the free
pages, this can save a lot CPU cycles and reduce the network traffic
significantly while speed up the live migration process obviously.

This patch set is the kernel side implementation.

The virtio-balloon driver is extended to send the free page bitmap
from guest to QEMU.

After getting the free page bitmap, QEMU can use it to filter out
guest's free pages. This make the live migration process much more
efficient.

In order to skip more free pages, we add an interface to let the user
decide whether dropping the cache in guest during live migration.

Liang Li (2):
  mm: add the related functions to build the free page bitmap
  virtio-balloon: extend balloon driver to support the new feature

 drivers/virtio/virtio_balloon.c     | 100 ++++++++++++++++++++++++++++++++++--
 fs/drop_caches.c                    |  22 +++++---
 include/linux/fs.h                  |   1 +
 include/uapi/linux/virtio_balloon.h |   1 +
 mm/page_alloc.c                     |  46 +++++++++++++++++
 5 files changed, 157 insertions(+), 13 deletions(-)

-- 
1.8.3.1

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

[Liang Li]

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.

Signed-off-by: Liang Li <liang.z.li@intel.com>
---
 fs/drop_caches.c   | 22 ++++++++++++++--------
 include/linux/fs.h |  1 +
 mm/page_alloc.c    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index d72d52b..f488086 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -50,14 +50,8 @@ int drop_caches_sysctl_handler(struct ctl_table *table, int write,
 	if (write) {
 		static int stfu;
 
-		if (sysctl_drop_caches & 1) {
-			iterate_supers(drop_pagecache_sb, NULL);
-			count_vm_event(DROP_PAGECACHE);
-		}
-		if (sysctl_drop_caches & 2) {
-			drop_slab();
-			count_vm_event(DROP_SLAB);
-		}
+		drop_cache(sysctl_drop_caches);
+
 		if (!stfu) {
 			pr_info("%s (%d): drop_caches: %d\n",
 				current->comm, task_pid_nr(current),
@@ -67,3 +61,15 @@ int drop_caches_sysctl_handler(struct ctl_table *table, int write,
 	}
 	return 0;
 }
+
+void drop_cache(int drop_ctl)
+{
+	if (drop_ctl & 1) {
+		iterate_supers(drop_pagecache_sb, NULL);
+		count_vm_event(DROP_PAGECACHE);
+	}
+	if (drop_ctl & 2) {
+		drop_slab();
+		count_vm_event(DROP_SLAB);
+	}
+}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 70e61b5..b8a0bc0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2864,6 +2864,7 @@ extern void drop_super(struct super_block *sb);
 extern void iterate_supers(void (*)(struct super_block *, void *), void *);
 extern void iterate_supers_type(struct file_system_type *,
 			        void (*)(struct super_block *, void *), void *);
+extern void drop_cache(int drop_ctl);
 
 extern int dcache_dir_open(struct inode *, struct file *);
 extern int dcache_dir_close(struct inode *, struct file *);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 59de90d..4799983 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -63,6 +63,7 @@
 #include <linux/sched/rt.h>
 #include <linux/page_owner.h>
 #include <linux/kthread.h>
+#include <linux/fs.h>
 
 #include <asm/sections.h>
 #include <asm/tlbflush.h>
@@ -4029,6 +4030,51 @@ void show_free_areas(unsigned int filter)
 	show_swap_cache_info();
 }
 
+static void mark_free_pages_bitmap(struct zone *zone,
+		unsigned long *bitmap, unsigned long len)
+{
+	unsigned long pfn, flags, i, limit;
+	unsigned int order, t;
+	struct list_head *curr;
+
+	if (zone_is_empty(zone))
+		return;
+
+	spin_lock_irqsave(&zone->lock, flags);
+
+	limit = min(len, max_pfn);
+	for_each_migratetype_order(order, t) {
+		list_for_each(curr, &zone->free_area[order].free_list[t]) {
+			pfn = page_to_pfn(list_entry(curr, struct page, lru));
+			for (i = 0; i < (1UL << order); i++) {
+				if ((pfn + i) < limit)
+					set_bit_le(pfn + i, bitmap);
+				else
+					break;
+			}
+		}
+	}
+
+	spin_unlock_irqrestore(&zone->lock, flags);
+}
+
+unsigned long get_max_pfn(void)
+{
+	return max_pfn;
+}
+EXPORT_SYMBOL(get_max_pfn);
+
+void get_free_pages(unsigned long *bitmap, unsigned long len, int drop)
+{
+	struct zone *zone;
+
+	drop_cache(drop);
+
+	for_each_populated_zone(zone)
+		mark_free_pages_bitmap(zone, bitmap, len);
+}
+EXPORT_SYMBOL(get_free_pages);
+
 static void zoneref_set_zone(struct zone *zone, struct zoneref *zoneref)
 {
 	zoneref->zone = zone;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Rik van Riel]

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?

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Li, Liang Z]

> 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

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Rik van Riel]

On Tue, 2016-04-19 at 15:02 +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 ...

Fair enough.

It would be good to have that mentioned in the
changelog.

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Li, Liang Z]

> On Tue, 2016-04-19 at 15:02 +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 ...
> 
> Fair enough.
> 
> It would be good to have that mentioned in the changelog.

Yes, I will. Thanks!

Liang

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Michael S. Tsirkin]

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.

-- 
MST

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Li, Liang Z]

> Cc: Rik van Riel; viro@zeniv.linux.org.uk; linux-kernel@vger.kernel.org;
> quintela@redhat.com; amit.shah@redhat.com; pbonzini@redhat.com;
> dgilbert@redhat.com; linux-mm@kvack.org; kvm@vger.kernel.org; qemu-
> devel@nongnu.org; agraf@suse.de; borntraeger@de.ibm.com
> Subject: Re: [PATCH kernel 1/2] mm: add the related functions to build the
> free page bitmap
> 
> 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.

Thanks! You mean save the free page bitmap in host pages?

> 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.
> 

Then we should prevent the guest from using those recently-freed pages, 
before doing the MADV_DONTNEED or MADV_FREE, or the pages in the
free page bitmap may be not free any more. In which case we will do something
like this? Balloon?

Liang


> --
> MST

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Michael S. Tsirkin]

On Wed, Apr 20, 2016 at 01:41:24AM +0000, Li, Liang Z wrote:
> > Cc: Rik van Riel; viro@zeniv.linux.org.uk; linux-kernel@vger.kernel.org;
> > quintela@redhat.com; amit.shah@redhat.com; pbonzini@redhat.com;
> > dgilbert@redhat.com; linux-mm@kvack.org; kvm@vger.kernel.org; qemu-
> > devel@nongnu.org; agraf@suse.de; borntraeger@de.ibm.com
> > Subject: Re: [PATCH kernel 1/2] mm: add the related functions to build the
> > free page bitmap
> > 
> > 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.
> 
> Thanks! You mean save the free page bitmap in host pages?

No, I literally mean don't release &zone->lock before you pass
the list of pages to host.

> > 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.
> > 
> 
> Then we should prevent the guest from using those recently-freed pages, 
> before doing the MADV_DONTNEED or MADV_FREE, or the pages in the
> free page bitmap may be not free any more. In which case we will do something
> like this? Balloon?
> 
> Liang
> 

Wouldn't keeping &zone->lock make sure these pages aren't used?


> > --
> > MST

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Li, Liang Z]

> On Wed, Apr 20, 2016 at 01:41:24AM +0000, Li, Liang Z wrote:
> > > Cc: Rik van Riel; viro@zeniv.linux.org.uk;
> > > linux-kernel@vger.kernel.org; quintela@redhat.com;
> > > amit.shah@redhat.com; pbonzini@redhat.com; dgilbert@redhat.com;
> > > linux-mm@kvack.org; kvm@vger.kernel.org; qemu- devel@nongnu.org;
> > > agraf@suse.de; borntraeger@de.ibm.com
> > > Subject: Re: [PATCH kernel 1/2] mm: add the related functions to
> > > build the free page bitmap
> > >
> > > 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.
> >
> > Thanks! You mean save the free page bitmap in host pages?
> 
> No, I literally mean don't release &zone->lock before you pass the list of
> pages to host.
> 
> > > 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.
> > >
> >
> > Then we should prevent the guest from using those recently-freed
> > pages, before doing the MADV_DONTNEED or MADV_FREE, or the pages in
> > the free page bitmap may be not free any more. In which case we will
> > do something like this? Balloon?
> >
> > Liang
> >
> 
> Wouldn't keeping &zone->lock make sure these pages aren't used?
> 
> 

Yes, keep the &zone->lock can ensure this, and it can make sure we get a real
free page bitmap, its more semantic correct.

But once we get a 'real' free page bitmap, the pages in the free page bitmap may 
became no free anymore before using it for some purposes, is there any mechanism
to prevent this?
If there is no strong reason, it's better to take the lock as short as possible.
Could you elaborate some use cases which require a 'real' free page bitmap? 

Liang

> > > --
> > > MST

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (mst@redhat.com) 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

> 
> -- 
> MST
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Michael S. Tsirkin]

On Fri, Apr 22, 2016 at 10:48:38AM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) 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.

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.

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@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Li, Liang Z]

> On Fri, Apr 22, 2016 at 10:48:38AM +0100, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (mst@redhat.com) 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?

> 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.
> 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@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Michael S. Tsirkin]

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@redhat.com) 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.

> > 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.


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.




> > 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@redhat.com / Manchester, UK

Re: [Qemu-devel] [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@redhat.com) 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@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 746 bytes --]

Hi,

[auto build test ERROR on v4.6-rc4]
[also build test ERROR on next-20160419]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Liang-Li/speed-up-live-migration-by-skipping-free-pages/20160419-224707
config: i386-tinyconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   mm/built-in.o: In function `get_free_pages':
>> (.text+0x3ff6): undefined reference to `drop_cache'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6274 bytes --]

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 904 bytes --]

Hi,

[auto build test ERROR on v4.6-rc4]
[also build test ERROR on next-20160419]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Liang-Li/speed-up-live-migration-by-skipping-free-pages/20160419-224707
config: arm-allnoconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   mm/built-in.o: In function `get_free_pages':
>> :(.text+0x3db4): undefined reference to `drop_cache'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 5508 bytes --]

Re: [Qemu-devel] [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]

Hi,

[auto build test ERROR on v4.6-rc4]
[also build test ERROR on next-20160419]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Liang-Li/speed-up-live-migration-by-skipping-free-pages/20160419-224707
config: parisc-allnoconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=parisc 

All errors (new ones prefixed by >>):

   mm/built-in.o: In function `get_free_pages':
>> (.text.get_free_pages+0x28): undefined reference to `drop_cache'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 4506 bytes --]

[Qemu-devel] [PATCH kernel 2/2] virtio-balloon: extend balloon driver to support the new feature

[Liang Li]

Extend the virtio balloon to support the new feature
VIRTIO_BALLOON_F_GET_FREE_PAGES, so that we can use it to send
the free page bitmap from guest to QEMU, the free page bitmap will
be used for live migration optimization.

Signed-off-by: Liang Li <liang.z.li@intel.com>
---
 drivers/virtio/virtio_balloon.c     | 100 ++++++++++++++++++++++++++++++++++--
 include/uapi/linux/virtio_balloon.h |   1 +
 2 files changed, 96 insertions(+), 5 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 7b6d74f..cf17694 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -45,9 +45,17 @@ static int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
 module_param(oom_pages, int, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
 
+extern void get_free_pages(unsigned long *free_page_bitmap,
+				unsigned long len, int drop);
+extern unsigned long get_max_pfn(void);
+
+struct cache_drop_ctrl {
+	u64 ctrl;
+};
+
 struct virtio_balloon {
 	struct virtio_device *vdev;
-	struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
+	struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_pages_vq;
 
 	/* The balloon servicing is delegated to a freezable workqueue. */
 	struct work_struct update_balloon_stats_work;
@@ -77,6 +85,10 @@ struct virtio_balloon {
 	unsigned int num_pfns;
 	u32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
 
+	unsigned long *free_pages;
+	unsigned long bmap_len;
+	struct cache_drop_ctrl cache_drop;
+
 	/* Memory statistics */
 	struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
 
@@ -256,6 +268,64 @@ static void update_balloon_stats(struct virtio_balloon *vb)
 				pages_to_bytes(available));
 }
 
+static void update_free_pages_stats(struct virtio_balloon *vb)
+{
+	unsigned long bitmap_bytes, max_pfn;
+
+	max_pfn = get_max_pfn();
+	bitmap_bytes = ALIGN(max_pfn, BITS_PER_LONG) / 8;
+
+	if (!vb->free_pages)
+		vb->free_pages = kzalloc(bitmap_bytes, GFP_KERNEL);
+	else {
+		if (bitmap_bytes < vb->bmap_len)
+			memset(vb->free_pages, 0, bitmap_bytes);
+		else {
+			kfree(vb->free_pages);
+			vb->free_pages = kzalloc(bitmap_bytes, GFP_KERNEL);
+		}
+	}
+	if (!vb->free_pages) {
+		vb->bmap_len = 0;
+		return;
+	}
+
+	vb->bmap_len = bitmap_bytes;
+	get_free_pages(vb->free_pages, max_pfn, vb->cache_drop.ctrl);
+}
+
+static void free_pages_handle_rq(struct virtio_balloon *vb)
+{
+	struct virtqueue *vq;
+	struct scatterlist sg[2];
+	unsigned int len;
+	struct cache_drop_ctl *ptr_cache_drop;
+	struct scatterlist sg_in;
+
+	vq = vb->free_pages_vq;
+	ptr_cache_drop = virtqueue_get_buf(vq, &len);
+
+	if (!ptr_cache_drop || len != sizeof(vb->cache_drop))
+		return;
+	update_free_pages_stats(vb);
+	sg_init_table(sg, 2);
+	sg_set_buf(&sg[0], &(vb->bmap_len), sizeof(vb->bmap_len));
+	sg_set_buf(&sg[1], vb->free_pages, vb->bmap_len);
+
+	sg_init_one(&sg_in, &vb->cache_drop, sizeof(vb->cache_drop));
+
+	virtqueue_add_outbuf(vq, &sg[0], 2, vb, GFP_KERNEL);
+	virtqueue_add_inbuf(vq, &sg_in, 1, &vb->cache_drop, GFP_KERNEL);
+	virtqueue_kick(vq);
+}
+
+static void free_pages_rq(struct virtqueue *vq)
+{
+	struct virtio_balloon *vb = vq->vdev->priv;
+
+	free_pages_handle_rq(vb);
+}
+
 /*
  * While most virtqueues communicate guest-initiated requests to the hypervisor,
  * the stats queue operates in reverse.  The driver initializes the virtqueue
@@ -392,16 +462,22 @@ static void update_balloon_size_func(struct work_struct *work)
 
 static int init_vqs(struct virtio_balloon *vb)
 {
-	struct virtqueue *vqs[3];
-	vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_request };
-	static const char * const names[] = { "inflate", "deflate", "stats" };
+	struct virtqueue *vqs[4];
+	vq_callback_t *callbacks[] = { balloon_ack, balloon_ack,
+					 stats_request, free_pages_rq };
+	const char *names[] = { "inflate", "deflate", "stats", "free_pages" };
 	int err, nvqs;
 
 	/*
 	 * We expect two virtqueues: inflate and deflate, and
 	 * optionally stat.
 	 */
-	nvqs = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_GET_FREE_PAGES))
+		nvqs = 4;
+	else
+		nvqs = virtio_has_feature(vb->vdev,
+					  VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
+
 	err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names);
 	if (err)
 		return err;
@@ -422,6 +498,16 @@ static int init_vqs(struct virtio_balloon *vb)
 			BUG();
 		virtqueue_kick(vb->stats_vq);
 	}
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_GET_FREE_PAGES)) {
+		struct scatterlist sg_in;
+
+		vb->free_pages_vq = vqs[3];
+		sg_init_one(&sg_in, &vb->cache_drop, sizeof(vb->cache_drop));
+		if (virtqueue_add_inbuf(vb->free_pages_vq, &sg_in, 1,
+		    &vb->cache_drop, GFP_KERNEL) < 0)
+			BUG();
+		virtqueue_kick(vb->free_pages_vq);
+	}
 	return 0;
 }
 
@@ -505,6 +591,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
 		goto out;
 	}
 
+	vb->bmap_len = 0;
+	vb->free_pages = NULL;
 	INIT_WORK(&vb->update_balloon_stats_work, update_balloon_stats_func);
 	INIT_WORK(&vb->update_balloon_size_work, update_balloon_size_func);
 	spin_lock_init(&vb->stop_update_lock);
@@ -567,6 +655,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
 	cancel_work_sync(&vb->update_balloon_stats_work);
 
 	remove_common(vb);
+	kfree(vb->free_pages);
 	kfree(vb);
 }
 
@@ -605,6 +694,7 @@ static unsigned int features[] = {
 	VIRTIO_BALLOON_F_MUST_TELL_HOST,
 	VIRTIO_BALLOON_F_STATS_VQ,
 	VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
+	VIRTIO_BALLOON_F_GET_FREE_PAGES,
 };
 
 static struct virtio_driver virtio_balloon_driver = {
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index 343d7dd..2b41e4f 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -34,6 +34,7 @@
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST	0 /* Tell before reclaiming pages */
 #define VIRTIO_BALLOON_F_STATS_VQ	1 /* Memory Stats virtqueue */
 #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM	2 /* Deflate balloon on OOM */
+#define VIRTIO_BALLOON_F_GET_FREE_PAGES	3 /* Get free page bitmap */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH kernel 2/2] virtio-balloon: extend balloon driver to support the new feature

[Michael S. Tsirkin]

On Tue, Apr 19, 2016 at 10:34:34PM +0800, Liang Li wrote:
> Extend the virtio balloon to support the new feature
> VIRTIO_BALLOON_F_GET_FREE_PAGES, so that we can use it to send
> the free page bitmap from guest to QEMU, the free page bitmap will
> be used for live migration optimization.
> 
> Signed-off-by: Liang Li <liang.z.li@intel.com>

Two points:
- please post description of your interface proposals
  to virtio tc comment list
- please split this up
	- feature to use a bitmap for inflate/deflate
	- a 3rd vq which does inflate/deflate in one go

  there seems no reason to use bitmap for free pages
  but not for inflate/deflate

> ---
>  drivers/virtio/virtio_balloon.c     | 100 ++++++++++++++++++++++++++++++++++--
>  include/uapi/linux/virtio_balloon.h |   1 +
>  2 files changed, 96 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 7b6d74f..cf17694 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -45,9 +45,17 @@ static int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
>  module_param(oom_pages, int, S_IRUSR | S_IWUSR);
>  MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
>  
> +extern void get_free_pages(unsigned long *free_page_bitmap,
> +				unsigned long len, int drop);
> +extern unsigned long get_max_pfn(void);
> +
> +struct cache_drop_ctrl {
> +	u64 ctrl;
> +};
> +
>  struct virtio_balloon {
>  	struct virtio_device *vdev;
> -	struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
> +	struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_pages_vq;
>  
>  	/* The balloon servicing is delegated to a freezable workqueue. */
>  	struct work_struct update_balloon_stats_work;
> @@ -77,6 +85,10 @@ struct virtio_balloon {
>  	unsigned int num_pfns;
>  	u32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
>  
> +	unsigned long *free_pages;
> +	unsigned long bmap_len;
> +	struct cache_drop_ctrl cache_drop;
> +
>  	/* Memory statistics */
>  	struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
>  
> @@ -256,6 +268,64 @@ static void update_balloon_stats(struct virtio_balloon *vb)
>  				pages_to_bytes(available));
>  }
>  
> +static void update_free_pages_stats(struct virtio_balloon *vb)
> +{
> +	unsigned long bitmap_bytes, max_pfn;
> +
> +	max_pfn = get_max_pfn();
> +	bitmap_bytes = ALIGN(max_pfn, BITS_PER_LONG) / 8;
> +
> +	if (!vb->free_pages)
> +		vb->free_pages = kzalloc(bitmap_bytes, GFP_KERNEL);
> +	else {
> +		if (bitmap_bytes < vb->bmap_len)
> +			memset(vb->free_pages, 0, bitmap_bytes);
> +		else {
> +			kfree(vb->free_pages);
> +			vb->free_pages = kzalloc(bitmap_bytes, GFP_KERNEL);
> +		}
> +	}
> +	if (!vb->free_pages) {
> +		vb->bmap_len = 0;
> +		return;
> +	}
> +
> +	vb->bmap_len = bitmap_bytes;
> +	get_free_pages(vb->free_pages, max_pfn, vb->cache_drop.ctrl);
> +}
> +
> +static void free_pages_handle_rq(struct virtio_balloon *vb)
> +{
> +	struct virtqueue *vq;
> +	struct scatterlist sg[2];
> +	unsigned int len;
> +	struct cache_drop_ctl *ptr_cache_drop;
> +	struct scatterlist sg_in;
> +
> +	vq = vb->free_pages_vq;
> +	ptr_cache_drop = virtqueue_get_buf(vq, &len);
> +
> +	if (!ptr_cache_drop || len != sizeof(vb->cache_drop))
> +		return;
> +	update_free_pages_stats(vb);
> +	sg_init_table(sg, 2);
> +	sg_set_buf(&sg[0], &(vb->bmap_len), sizeof(vb->bmap_len));
> +	sg_set_buf(&sg[1], vb->free_pages, vb->bmap_len);
> +
> +	sg_init_one(&sg_in, &vb->cache_drop, sizeof(vb->cache_drop));
> +
> +	virtqueue_add_outbuf(vq, &sg[0], 2, vb, GFP_KERNEL);
> +	virtqueue_add_inbuf(vq, &sg_in, 1, &vb->cache_drop, GFP_KERNEL);
> +	virtqueue_kick(vq);
> +}
> +
> +static void free_pages_rq(struct virtqueue *vq)
> +{
> +	struct virtio_balloon *vb = vq->vdev->priv;
> +
> +	free_pages_handle_rq(vb);
> +}
> +
>  /*
>   * While most virtqueues communicate guest-initiated requests to the hypervisor,
>   * the stats queue operates in reverse.  The driver initializes the virtqueue
> @@ -392,16 +462,22 @@ static void update_balloon_size_func(struct work_struct *work)
>  
>  static int init_vqs(struct virtio_balloon *vb)
>  {
> -	struct virtqueue *vqs[3];
> -	vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_request };
> -	static const char * const names[] = { "inflate", "deflate", "stats" };
> +	struct virtqueue *vqs[4];
> +	vq_callback_t *callbacks[] = { balloon_ack, balloon_ack,
> +					 stats_request, free_pages_rq };
> +	const char *names[] = { "inflate", "deflate", "stats", "free_pages" };
>  	int err, nvqs;
>  
>  	/*
>  	 * We expect two virtqueues: inflate and deflate, and
>  	 * optionally stat.
>  	 */
> -	nvqs = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
> +	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_GET_FREE_PAGES))
> +		nvqs = 4;
> +	else
> +		nvqs = virtio_has_feature(vb->vdev,
> +					  VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
> +
>  	err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names);
>  	if (err)
>  		return err;
> @@ -422,6 +498,16 @@ static int init_vqs(struct virtio_balloon *vb)
>  			BUG();
>  		virtqueue_kick(vb->stats_vq);
>  	}
> +	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_GET_FREE_PAGES)) {
> +		struct scatterlist sg_in;
> +
> +		vb->free_pages_vq = vqs[3];
> +		sg_init_one(&sg_in, &vb->cache_drop, sizeof(vb->cache_drop));
> +		if (virtqueue_add_inbuf(vb->free_pages_vq, &sg_in, 1,
> +		    &vb->cache_drop, GFP_KERNEL) < 0)
> +			BUG();
> +		virtqueue_kick(vb->free_pages_vq);
> +	}
>  	return 0;
>  }
>  
> @@ -505,6 +591,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
>  		goto out;
>  	}
>  
> +	vb->bmap_len = 0;
> +	vb->free_pages = NULL;
>  	INIT_WORK(&vb->update_balloon_stats_work, update_balloon_stats_func);
>  	INIT_WORK(&vb->update_balloon_size_work, update_balloon_size_func);
>  	spin_lock_init(&vb->stop_update_lock);
> @@ -567,6 +655,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
>  	cancel_work_sync(&vb->update_balloon_stats_work);
>  
>  	remove_common(vb);
> +	kfree(vb->free_pages);
>  	kfree(vb);
>  }
>  
> @@ -605,6 +694,7 @@ static unsigned int features[] = {
>  	VIRTIO_BALLOON_F_MUST_TELL_HOST,
>  	VIRTIO_BALLOON_F_STATS_VQ,
>  	VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
> +	VIRTIO_BALLOON_F_GET_FREE_PAGES,
>  };
>  
>  static struct virtio_driver virtio_balloon_driver = {
> diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
> index 343d7dd..2b41e4f 100644
> --- a/include/uapi/linux/virtio_balloon.h
> +++ b/include/uapi/linux/virtio_balloon.h
> @@ -34,6 +34,7 @@
>  #define VIRTIO_BALLOON_F_MUST_TELL_HOST	0 /* Tell before reclaiming pages */
>  #define VIRTIO_BALLOON_F_STATS_VQ	1 /* Memory Stats virtqueue */
>  #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM	2 /* Deflate balloon on OOM */
> +#define VIRTIO_BALLOON_F_GET_FREE_PAGES	3 /* Get free page bitmap */
>  
>  /* Size of a PFN in the balloon interface. */
>  #define VIRTIO_BALLOON_PFN_SHIFT 12
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH kernel 0/2] speed up live migration by skipping free pages

[Amit Shah]

On (Tue) 19 Apr 2016 [22:34:32], Liang Li wrote:
> Current QEMU live migration implementation mark all guest's RAM pages
> as dirtied in the ram bulk stage, all these pages will be processed
> and it consumes quite a lot of CPU cycles and network bandwidth.
> 
> From guest's point of view, it doesn't care about the content in free
> page. We can make use of this fact and skip processing the free
> pages, this can save a lot CPU cycles and reduce the network traffic
> significantly while speed up the live migration process obviously.
> 
> This patch set is the kernel side implementation.
> 
> The virtio-balloon driver is extended to send the free page bitmap
> from guest to QEMU.
> 
> After getting the free page bitmap, QEMU can use it to filter out
> guest's free pages. This make the live migration process much more
> efficient.
> 
> In order to skip more free pages, we add an interface to let the user
> decide whether dropping the cache in guest during live migration.

So if virtio-balloon is the way to go (i.e. speed is acceptable), I
just have one point then.  My main concern with using (or not using)
virtio-balloon was that a guest admin is going to disable the
virtio-balloon driver entirely because the admin won't want the guest
to give away pages to the host, esp. when the guest is to be a
high-performant one.

In this case, if a new command can be added to the balloon spec where
a guest driver indicates it's not going to participate in ballooning
activity (ie a guest will ignore any ballooning requests from the
host), but use the driver just for stats-sharing purposes, that can be
a workable solution here as well.  In that case, we can keep the
MM-related stuff inside the balloon driver, and also get the benefit
of the guest having control over how it uses its memory,
disincentivising guest admins from disabling the balloon entirely (it
will also benefit the guest to keep this driver loaded in such a
state, if migration is faster!).

		Amit

Re: [Qemu-devel] [PATCH kernel 0/2] speed up live migration by skipping free pages

[Michael S. Tsirkin]

On Mon, Apr 25, 2016 at 11:36:41AM +0530, Amit Shah wrote:
> On (Tue) 19 Apr 2016 [22:34:32], Liang Li wrote:
> > Current QEMU live migration implementation mark all guest's RAM pages
> > as dirtied in the ram bulk stage, all these pages will be processed
> > and it consumes quite a lot of CPU cycles and network bandwidth.
> > 
> > From guest's point of view, it doesn't care about the content in free
> > page. We can make use of this fact and skip processing the free
> > pages, this can save a lot CPU cycles and reduce the network traffic
> > significantly while speed up the live migration process obviously.
> > 
> > This patch set is the kernel side implementation.
> > 
> > The virtio-balloon driver is extended to send the free page bitmap
> > from guest to QEMU.
> > 
> > After getting the free page bitmap, QEMU can use it to filter out
> > guest's free pages. This make the live migration process much more
> > efficient.
> > 
> > In order to skip more free pages, we add an interface to let the user
> > decide whether dropping the cache in guest during live migration.
> 
> So if virtio-balloon is the way to go (i.e. speed is acceptable), I
> just have one point then.  My main concern with using (or not using)
> virtio-balloon was that a guest admin is going to disable the
> virtio-balloon driver entirely because the admin won't want the guest
> to give away pages to the host, esp. when the guest is to be a
> high-performant one.

The result will be the reverse of high-performance.

If you don't want to inflate a balloon, don't.

If you do but guest doesn't respond to inflate requests,
it's quite reasonable for host to kill it -
there is no way to distinguish between that and
guest being malicious.

I don't know of management tools doing that but
it's rather reasonable. What does happen is
some random guest memory is pushed it out to swap,
which is likely much worse than dropping unused memory
by moving it into the balloon.

> In this case, if a new command can be added to the balloon spec where
> a guest driver indicates it's not going to participate in ballooning
> activity (ie a guest will ignore any ballooning requests from the
> host), but use the driver just for stats-sharing purposes, that can be
> a workable solution here as well.  In that case, we can keep the
> MM-related stuff inside the balloon driver, and also get the benefit
> of the guest having control over how it uses its memory,
> disincentivising guest admins from disabling the balloon entirely (it
> will also benefit the guest to keep this driver loaded in such a
> state, if migration is faster!).
> 
> 		Amit

If there actually are people doing that, we should
figure out the reasons.

-- 
MST

Re: [Qemu-devel] [PATCH kernel 0/2] speed up live migration by skipping free pages

[Amit Shah]

On (Mon) 25 Apr 2016 [14:04:06], Michael S. Tsirkin wrote:
> On Mon, Apr 25, 2016 at 11:36:41AM +0530, Amit Shah wrote:
> > On (Tue) 19 Apr 2016 [22:34:32], Liang Li wrote:
> > > Current QEMU live migration implementation mark all guest's RAM pages
> > > as dirtied in the ram bulk stage, all these pages will be processed
> > > and it consumes quite a lot of CPU cycles and network bandwidth.
> > > 
> > > From guest's point of view, it doesn't care about the content in free
> > > page. We can make use of this fact and skip processing the free
> > > pages, this can save a lot CPU cycles and reduce the network traffic
> > > significantly while speed up the live migration process obviously.
> > > 
> > > This patch set is the kernel side implementation.
> > > 
> > > The virtio-balloon driver is extended to send the free page bitmap
> > > from guest to QEMU.
> > > 
> > > After getting the free page bitmap, QEMU can use it to filter out
> > > guest's free pages. This make the live migration process much more
> > > efficient.
> > > 
> > > In order to skip more free pages, we add an interface to let the user
> > > decide whether dropping the cache in guest during live migration.
> > 
> > So if virtio-balloon is the way to go (i.e. speed is acceptable), I
> > just have one point then.  My main concern with using (or not using)
> > virtio-balloon was that a guest admin is going to disable the
> > virtio-balloon driver entirely because the admin won't want the guest
> > to give away pages to the host, esp. when the guest is to be a
> > high-performant one.
> 
> The result will be the reverse of high-performance.
> 
> If you don't want to inflate a balloon, don't.
> 
> If you do but guest doesn't respond to inflate requests,
> it's quite reasonable for host to kill it -
> there is no way to distinguish between that and
> guest being malicious.

With the new command I'm suggesting, the guest will let the host know
that it has enabled this option, and it won't free up any RAM for the
host.

Also, just because a guest doesn't release some memory (which the
guest owns anyway) doesn't make it malicious, and killing such guests
is never going to end well for that hosting provider.

> I don't know of management tools doing that but
> it's rather reasonable. What does happen is
> some random guest memory is pushed it out to swap,
> which is likely much worse than dropping unused memory
> by moving it into the balloon.

Even if the host (admin) gave a guarantee that there won't be any
ballooning activity involved that will slow down the guest, a guest
admin can be paranoid enough to disable ballooning.  If, however, this
is made known to the host, it's likely a win-win situation because the
host knows the guest needs its RAM, and the guest can still use the
driver to send stats which the host can use during migration for
speedups.


		Amit

Re: [Qemu-devel] [PATCH kernel 0/2] speed up live migration by skipping free pages

[Michael S. Tsirkin]

On Mon, Apr 25, 2016 at 05:38:30PM +0530, Amit Shah wrote:
> On (Mon) 25 Apr 2016 [14:04:06], Michael S. Tsirkin wrote:
> > On Mon, Apr 25, 2016 at 11:36:41AM +0530, Amit Shah wrote:
> > > On (Tue) 19 Apr 2016 [22:34:32], Liang Li wrote:
> > > > Current QEMU live migration implementation mark all guest's RAM pages
> > > > as dirtied in the ram bulk stage, all these pages will be processed
> > > > and it consumes quite a lot of CPU cycles and network bandwidth.
> > > > 
> > > > From guest's point of view, it doesn't care about the content in free
> > > > page. We can make use of this fact and skip processing the free
> > > > pages, this can save a lot CPU cycles and reduce the network traffic
> > > > significantly while speed up the live migration process obviously.
> > > > 
> > > > This patch set is the kernel side implementation.
> > > > 
> > > > The virtio-balloon driver is extended to send the free page bitmap
> > > > from guest to QEMU.
> > > > 
> > > > After getting the free page bitmap, QEMU can use it to filter out
> > > > guest's free pages. This make the live migration process much more
> > > > efficient.
> > > > 
> > > > In order to skip more free pages, we add an interface to let the user
> > > > decide whether dropping the cache in guest during live migration.
> > > 
> > > So if virtio-balloon is the way to go (i.e. speed is acceptable), I
> > > just have one point then.  My main concern with using (or not using)
> > > virtio-balloon was that a guest admin is going to disable the
> > > virtio-balloon driver entirely because the admin won't want the guest
> > > to give away pages to the host, esp. when the guest is to be a
> > > high-performant one.
> > 
> > The result will be the reverse of high-performance.
> > 
> > If you don't want to inflate a balloon, don't.
> > 
> > If you do but guest doesn't respond to inflate requests,
> > it's quite reasonable for host to kill it -
> > there is no way to distinguish between that and
> > guest being malicious.
> 
> With the new command I'm suggesting, the guest will let the host know
> that it has enabled this option, and it won't free up any RAM for the
> host.
> 
> Also, just because a guest doesn't release some memory (which the
> guest owns anyway) doesn't make it malicious, and killing such guests
> is never going to end well for that hosting provider.
> 
> > I don't know of management tools doing that but
> > it's rather reasonable. What does happen is
> > some random guest memory is pushed it out to swap,
> > which is likely much worse than dropping unused memory
> > by moving it into the balloon.
> 
> Even if the host (admin) gave a guarantee that there won't be any
> ballooning activity involved that will slow down the guest, a guest
> admin can be paranoid enough to disable ballooning.  If, however, this
> is made known to the host, it's likely a win-win situation because the
> host knows the guest needs its RAM, and the guest can still use the
> driver to send stats which the host can use during migration for
> speedups.
> 
> 
> 		Amit

We'd need to understand the usecase better to design a good interface
for this. AFAIK the normal usecase for ballooning is for
memory overcommit: asking guest to free up memory might work
better than swap which makes host initiate a bunch of IO.
How is not inflating in this case a good idea?
I'm afraid I don't understand why was inflating balloon
requested if we do not want the guest to inflate the balloon.  What does
"paranoid" mean in this context?  This seems to imply some kind of
security concern. Is guest likely to need all of its memory or a
specific portion of it? Is it likely to be a static configuration or a
dynamic one? If dynamic, does guest also want to avoid deflating the
balloon or only inflating it?


-- 
MST

Re: [Qemu-devel] [PATCH kernel 0/2] speed up live migration by skipping free pages

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Mon, Apr 25, 2016 at 05:38:30PM +0530, Amit Shah wrote:
> > On (Mon) 25 Apr 2016 [14:04:06], Michael S. Tsirkin wrote:
> > > On Mon, Apr 25, 2016 at 11:36:41AM +0530, Amit Shah wrote:
> > > > On (Tue) 19 Apr 2016 [22:34:32], Liang Li wrote:
> > > > > Current QEMU live migration implementation mark all guest's RAM pages
> > > > > as dirtied in the ram bulk stage, all these pages will be processed
> > > > > and it consumes quite a lot of CPU cycles and network bandwidth.
> > > > > 
> > > > > From guest's point of view, it doesn't care about the content in free
> > > > > page. We can make use of this fact and skip processing the free
> > > > > pages, this can save a lot CPU cycles and reduce the network traffic
> > > > > significantly while speed up the live migration process obviously.
> > > > > 
> > > > > This patch set is the kernel side implementation.
> > > > > 
> > > > > The virtio-balloon driver is extended to send the free page bitmap
> > > > > from guest to QEMU.
> > > > > 
> > > > > After getting the free page bitmap, QEMU can use it to filter out
> > > > > guest's free pages. This make the live migration process much more
> > > > > efficient.
> > > > > 
> > > > > In order to skip more free pages, we add an interface to let the user
> > > > > decide whether dropping the cache in guest during live migration.
> > > > 
> > > > So if virtio-balloon is the way to go (i.e. speed is acceptable), I
> > > > just have one point then.  My main concern with using (or not using)
> > > > virtio-balloon was that a guest admin is going to disable the
> > > > virtio-balloon driver entirely because the admin won't want the guest
> > > > to give away pages to the host, esp. when the guest is to be a
> > > > high-performant one.
> > > 
> > > The result will be the reverse of high-performance.
> > > 
> > > If you don't want to inflate a balloon, don't.
> > > 
> > > If you do but guest doesn't respond to inflate requests,
> > > it's quite reasonable for host to kill it -
> > > there is no way to distinguish between that and
> > > guest being malicious.
> > 
> > With the new command I'm suggesting, the guest will let the host know
> > that it has enabled this option, and it won't free up any RAM for the
> > host.
> > 
> > Also, just because a guest doesn't release some memory (which the
> > guest owns anyway) doesn't make it malicious, and killing such guests
> > is never going to end well for that hosting provider.
> > 
> > > I don't know of management tools doing that but
> > > it's rather reasonable. What does happen is
> > > some random guest memory is pushed it out to swap,
> > > which is likely much worse than dropping unused memory
> > > by moving it into the balloon.
> > 
> > Even if the host (admin) gave a guarantee that there won't be any
> > ballooning activity involved that will slow down the guest, a guest
> > admin can be paranoid enough to disable ballooning.  If, however, this
> > is made known to the host, it's likely a win-win situation because the
> > host knows the guest needs its RAM, and the guest can still use the
> > driver to send stats which the host can use during migration for
> > speedups.
> > 
> > 
> > 		Amit
> 
> We'd need to understand the usecase better to design a good interface
> for this. AFAIK the normal usecase for ballooning is for
> memory overcommit: asking guest to free up memory might work
> better than swap which makes host initiate a bunch of IO.
> How is not inflating in this case a good idea?
> I'm afraid I don't understand why was inflating balloon
> requested if we do not want the guest to inflate the balloon.  What does
> "paranoid" mean in this context?  This seems to imply some kind of
> security concern. Is guest likely to need all of its memory or a
> specific portion of it? Is it likely to be a static configuration or a
> dynamic one? If dynamic, does guest also want to avoid deflating the
> balloon or only inflating it?

The semantics we want here are subtly different from ballooning;

  a) In ballooning the host is asking for the guest to give up some ram;
     that's not quite the case here - although if the guest dropped some
     caches that *might* be helpful.
  b) In ballooning we're interested in just amounts of free memory; here
     we care about _which_ pages are free.
  c) We don't want to make it hard for the guest to start using some of the
     memory again; if it was hard then we suddenly get back into a balancing
     act of needing to quantatively talk about how much free RAM we have,
     and worry about whether we shouldn't tell the host about a small pot
     we keep back etc.

Dave

> -- 
> MST
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK