Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1404299 > unrolled thread
| Started by | Liang Li <liang.z.li@intel.com> |
|---|---|
| First post | 2016-05-20 12:10 +0200 |
| Last post | 2016-05-25 16:40 +0200 |
| Articles | 3 on this page of 23 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH RFC kernel] balloon: speed up inflating/deflating process Liang Li <liang.z.li@intel.com> - 2016-05-20 12:10 +0200
Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process Paolo Bonzini <pbonzini@redhat.com> - 2016-05-20 13:30 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-24 10:00 +0200
Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process Cornelia Huck <cornelia.huck@de.ibm.com> - 2016-05-20 13:30 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-24 09:50 +0200
Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Michael S. Tsirkin" <mst@redhat.com> - 2016-05-20 14:10 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-24 12:00 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-24 12:00 +0200
Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Michael S. Tsirkin" <mst@redhat.com> - 2016-05-24 12:10 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-24 12:40 +0200
Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Michael S. Tsirkin" <mst@redhat.com> - 2016-05-24 13:20 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-24 16:40 +0200
Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Michael S. Tsirkin" <mst@redhat.com> - 2016-05-24 17:20 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-25 03:00 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-25 03:10 +0200
Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Michael S. Tsirkin" <mst@redhat.com> - 2016-05-25 10:40 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-25 10:50 +0200
Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Michael S. Tsirkin" <mst@redhat.com> - 2016-05-25 11:00 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-25 11:30 +0200
Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Michael S. Tsirkin" <mst@redhat.com> - 2016-05-25 11:50 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-25 12:20 +0200
Re: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Michael S. Tsirkin" <mst@redhat.com> - 2016-05-25 12:40 +0200
RE: [PATCH RFC kernel] balloon: speed up inflating/deflating process "Li, Liang Z" <liang.z.li@intel.com> - 2016-05-25 16:40 +0200
Page 2 of 2 — ← Prev page 1 [2]
| From | "Li, Liang Z" <liang.z.li@intel.com> |
|---|---|
| Date | 2016-05-25 12:20 +0200 |
| Message-ID | <rCE4V-8dd-7@gated-at.bofh.it> |
| In reply to | #1406778 |
> > > >
> > > > Hi MST,
> > > >
> > > > I have measured the performance when using a 32K page bitmap,
> > >
> > > Just to make sure. Do you mean a 32Kbyte bitmap?
> > > Covering 1Gbyte of memory?
> > Yes.
> >
> > >
> > > > and inflate the balloon to 3GB
> > > > of an idle guest with 4GB RAM.
> > >
> > > Should take 3 requests then, right?
> > >
> >
> > No, we can't assign the PFN when allocating page in balloon driver,
> > So the PFNs of pages allocated may be across a large range, we will
> > tell the host once the pfn_max -pfn_min >= 0x40000(1GB range), so the
> > requests count is most likely to be more than 3.
> >
> > > > Now:
> > > > total inflating time: 338ms
> > > > the count of virtio data transmission: 373
> > >
> > > Why was this so high? I would expect 3 transmissions.
> >
> > I follow your suggestion:
> > ----------------------------------------------------------------------
> > -------------- Suggestion to address all above comments:
> > 1. allocate a bunch of pages and link them up,
> > calculating the min and the max pfn.
> > if max-min exceeds the allocated bitmap size,
> > tell host.
> > 2. limit allocated bitmap size to something reasonable.
> > How about 32Kbytes? This is 256kilo bit in the map, which comes
> > out to 1Giga bytes of memory in the balloon.
> > ----------------------------------------------------------------------
> > --------------- Because the PFNs of the allocated pages are not linear
> > increased, so 3 transmissions are impossible.
> >
> >
> > Liang
>
> Interesting. How about instead of tell host, we do multiple scans, each time
> ignoring pages out of range?
>
> for (pfn = min pfn; pfn < max pfn; pfn += 1G) {
> foreach page
> if page pfn < pfn || page pfn >= pfn + 1G
> continue
> set bit
> tell host
> }
>
That means we have to allocate/free all the requested pages first, and then tell the host.
It works fine for inflating, but for deflating, because the page has been deleted from the vb-> vb_dev_info->pages,
so, we have to use a struct to save the dequeued pages before calling release_pages_balloon(),
I think a page bitmap is the best struct to save these pages, because it consumes less memory.
And that bitmap should be large enough to save pfn 0 to max_pfn.
If the above is true, then we are back to the square one. we really need a large page bitmap. Right?
Liang
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2016-05-25 12:40 +0200 |
| Message-ID | <rCEoi-8jG-11@gated-at.bofh.it> |
| In reply to | #1406807 |
On Wed, May 25, 2016 at 10:10:47AM +0000, Li, Liang Z wrote:
> > > > >
> > > > > Hi MST,
> > > > >
> > > > > I have measured the performance when using a 32K page bitmap,
> > > >
> > > > Just to make sure. Do you mean a 32Kbyte bitmap?
> > > > Covering 1Gbyte of memory?
> > > Yes.
> > >
> > > >
> > > > > and inflate the balloon to 3GB
> > > > > of an idle guest with 4GB RAM.
> > > >
> > > > Should take 3 requests then, right?
> > > >
> > >
> > > No, we can't assign the PFN when allocating page in balloon driver,
> > > So the PFNs of pages allocated may be across a large range, we will
> > > tell the host once the pfn_max -pfn_min >= 0x40000(1GB range), so the
> > > requests count is most likely to be more than 3.
> > >
> > > > > Now:
> > > > > total inflating time: 338ms
> > > > > the count of virtio data transmission: 373
> > > >
> > > > Why was this so high? I would expect 3 transmissions.
> > >
> > > I follow your suggestion:
> > > ----------------------------------------------------------------------
> > > -------------- Suggestion to address all above comments:
> > > 1. allocate a bunch of pages and link them up,
> > > calculating the min and the max pfn.
> > > if max-min exceeds the allocated bitmap size,
> > > tell host.
> > > 2. limit allocated bitmap size to something reasonable.
> > > How about 32Kbytes? This is 256kilo bit in the map, which comes
> > > out to 1Giga bytes of memory in the balloon.
> > > ----------------------------------------------------------------------
> > > --------------- Because the PFNs of the allocated pages are not linear
> > > increased, so 3 transmissions are impossible.
> > >
> > >
> > > Liang
> >
> > Interesting. How about instead of tell host, we do multiple scans, each time
> > ignoring pages out of range?
> >
> > for (pfn = min pfn; pfn < max pfn; pfn += 1G) {
> > foreach page
> > if page pfn < pfn || page pfn >= pfn + 1G
> > continue
> > set bit
> > tell host
> > }
> >
>
> That means we have to allocate/free all the requested pages first, and then tell the host.
> It works fine for inflating, but for deflating, because the page has been deleted from the vb-> vb_dev_info->pages,
> so, we have to use a struct to save the dequeued pages before calling release_pages_balloon(),
struct list_head? I think you can just replace set_page_pfns with
list_add(&page->lru, &page_list);
> I think a page bitmap is the best struct to save these pages, because it consumes less memory.
> And that bitmap should be large enough to save pfn 0 to max_pfn.
>
> If the above is true, then we are back to the square one. we really need a large page bitmap. Right?
>
> Liang
These look like implementation issues to me.
I think the below might be helpful (completely untested),
your work can go on top.
--->
virtio-balloon: rework deflate to add page to a tmp list
Will allow faster notifications using a bitmap down the road.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 476c0e3..44050a3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -195,8 +195,9 @@ static void release_pages_balloon(struct virtio_balloon *vb)
static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
{
unsigned num_freed_pages;
- struct page *page;
+ struct page *page, *next;
struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
+ LIST_HEAD(pages); /* Pages dequeued for handing to Host */
/* We can only do one array worth at a time. */
num = min(num, ARRAY_SIZE(vb->pfns));
@@ -207,10 +208,13 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
page = balloon_page_dequeue(vb_dev_info);
if (!page)
break;
- set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+ list_add(&page->lru, &pages);
vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
}
+ list_for_each_entry_safe(page, next, &pages, lru)
+ set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+
num_freed_pages = vb->num_pfns;
/*
* Note that if
--
MST
[toc] | [prev] | [next] | [standalone]
| From | "Li, Liang Z" <liang.z.li@intel.com> |
|---|---|
| Date | 2016-05-25 16:40 +0200 |
| Message-ID | <rCI8y-25o-19@gated-at.bofh.it> |
| In reply to | #1406822 |
> > > Interesting. How about instead of tell host, we do multiple scans,
> > > each time ignoring pages out of range?
> > >
> > > for (pfn = min pfn; pfn < max pfn; pfn += 1G) {
> > > foreach page
> > > if page pfn < pfn || page pfn >= pfn + 1G
> > > continue
> > > set bit
> > > tell host
> > > }
> > >
> >
> > That means we have to allocate/free all the requested pages first, and then
> tell the host.
> > It works fine for inflating, but for deflating, because the page has
> > been deleted from the vb-> vb_dev_info->pages, so, we have to use a
> > struct to save the dequeued pages before calling
> > release_pages_balloon(),
>
> struct list_head? I think you can just replace set_page_pfns with
> list_add(&page->lru, &page_list);
>
>
That's is fine, I will retry and get back to you.
Thanks!
Liang
>
> > I think a page bitmap is the best struct to save these pages, because it
> consumes less memory.
> > And that bitmap should be large enough to save pfn 0 to max_pfn.
> >
> > If the above is true, then we are back to the square one. we really need a
> large page bitmap. Right?
> >
> > Liang
>
> These look like implementation issues to me.
>
> I think the below might be helpful (completely untested), your work can go
> on top.
>
> --->
>
> virtio-balloon: rework deflate to add page to a tmp list
>
> Will allow faster notifications using a bitmap down the road.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ---
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 476c0e3..44050a3 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -195,8 +195,9 @@ static void release_pages_balloon(struct
> virtio_balloon *vb) static unsigned leak_balloon(struct virtio_balloon *vb,
> size_t num) {
> unsigned num_freed_pages;
> - struct page *page;
> + struct page *page, *next;
> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> + LIST_HEAD(pages); /* Pages dequeued for handing to
> Host */
>
> /* We can only do one array worth at a time. */
> num = min(num, ARRAY_SIZE(vb->pfns));
> @@ -207,10 +208,13 @@ static unsigned leak_balloon(struct virtio_balloon
> *vb, size_t num)
> page = balloon_page_dequeue(vb_dev_info);
> if (!page)
> break;
> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + list_add(&page->lru, &pages);
> vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
> }
>
> + list_for_each_entry_safe(page, next, &pages, lru)
> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> +
> num_freed_pages = vb->num_pfns;
> /*
> * Note that if
> --
> MST
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web