Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1382586 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap Liang Li <liang.z.li@intel.com> - 2016-04-19 16:50 +0200
    Re: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap Rik van Riel <riel@redhat.com> - 2016-04-19 17:00 +0200
      RE: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap "Li, Liang Z" <liang.z.li@intel.com> - 2016-04-19 17:10 +0200
        Re: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap Rik van Riel <riel@redhat.com> - 2016-04-19 17:30 +0200
          RE: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap "Li, Liang Z" <liang.z.li@intel.com> - 2016-04-20 03:00 +0200
        Re: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap "Michael S. Tsirkin" <mst@redhat.com> - 2016-04-19 18:20 +0200
          RE: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap "Li, Liang Z" <liang.z.li@intel.com> - 2016-04-20 03:50 +0200
            Re: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap "Michael S. Tsirkin" <mst@redhat.com> - 2016-04-21 15:50 +0200
              RE: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap "Li, Liang Z" <liang.z.li@intel.com> - 2016-04-22 03:40 +0200
          Re: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap "Dr. David Alan Gilbert" <dgilbert@redhat.com> - 2016-04-22 11:50 +0200
            Re: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap "Michael S. Tsirkin" <mst@redhat.com> - 2016-04-22 16:00 +0200
              RE: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap "Li, Liang Z" <liang.z.li@intel.com> - 2016-04-25 05:20 +0200
                Re: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap "Michael S. Tsirkin" <mst@redhat.com> - 2016-04-25 12:50 +0200
                  RE: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap "Li, Liang Z" <liang.z.li@intel.com> - 2016-04-26 05:30 +0200
    Re: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap kbuild test robot <lkp@intel.com> - 2016-04-19 17:00 +0200
    Re: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap kbuild test robot <lkp@intel.com> - 2016-04-19 18:10 +0200
    Re: [PATCH kernel 1/2] mm: add the related functions to build the  free page bitmap kbuild test robot <lkp@intel.com> - 2016-04-19 18:20 +0200

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

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

[toc] | [next] | [standalone]

#1382592 — Re: [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

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?

[toc] | [prev] | [next] | [standalone]

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

> 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

[toc] | [prev] | [next] | [standalone]

#1382610 — Re: [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

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.

[toc] | [prev] | [next] | [standalone]

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

> 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

[toc] | [prev] | [next] | [standalone]

#1382668 — 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.
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

[toc] | [prev] | [next] | [standalone]

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

> 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

[toc] | [prev] | [next] | [standalone]

#1384242 — Re: [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

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

[toc] | [prev] | [next] | [standalone]

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

> 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

[toc] | [prev] | [next] | [standalone]

#1384842 — Re: [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

* 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

[toc] | [prev] | [next] | [standalone]

#1385197 — Re: [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

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

[toc] | [prev] | [next] | [standalone]

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

> 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

[toc] | [prev] | [next] | [standalone]

#1386230 — Re: [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

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

[toc] | [prev] | [next] | [standalone]

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

> 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

[toc] | [prev] | [next] | [standalone]

#1382594 — Re: [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1382663 — Re: [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1382671 — Re: [PATCH kernel 1/2] mm: add the related functions to build the free page bitmap

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web