Re: [PATCH] KVM virtio balloon driver

Re: [PATCH] KVM virtio balloon driver

[Dor Laor]

On Tue, 2008-01-15 at 17:01 -0200, Marcelo Tosatti wrote:

> OK, thats simpler. How about this:
> 

It's sure is simpler :)

> [PATCH] Virtio balloon driver
> 
> Add a balloon driver for KVM, host<->guest communication is performed
> via virtio.
> 
> Signed-off-by: Marcelo Tosatti <mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

[snip]

> +static void free_page_array(struct balloon_buf *buf, unsigned int npages)
> +{
> +	struct page *page;
> +	u32 *pfn = (u32 *)&buf->data;
> +	int i;
> +
> +	for (i=0; i<npages; i++) { 
> +		page = pfn_to_page(*pfn);
> +		list_del_init(&page->lru);
> +		 __free_page(page);
> +		pfn++;

In add_page_array below you update baloon_size & totalram_pages,
it is need here too.

> +	}
> +}
> +
> +static void add_page_array(struct virtballoon *v, struct balloon_buf *buf,
> +			   unsigned int npages)
> +{
> +	struct page *page;
> +	u32 *pfn = (u32 *)&buf->data;
> +	int i;
> +
> +	for (i=0; i<npages; i++) {
> +		page = pfn_to_page(*pfn);
> +		v->balloon_size++;
> +		totalram_pages--;
> +		list_add(&page->lru, &v->balloon_plist);
> +		pfn++;
> +	}
> +}
> +
> +static void inflate_done(struct virtballoon *v, struct balloon_buf *buf,
> +			 unsigned int npages)
> +{
> +	u8 status = buf->hdr.status;
> +
> +	/* inflate OK */
> +	if (!status)
> +		add_page_array(v, buf, npages);
> +	else 
> +		free_page_array(buf, npages);
> +}
> +
> +static void deflate_done(struct virtballoon *v, struct balloon_buf *buf,
> +			 unsigned int npages)
> +{
> +	u8 status = buf->hdr.status;
> +
> +	/* deflate OK, return pages to the system */
> +	if (!status) {
> +		free_page_array(buf, npages);

If there are update above then no need below.

> +		totalram_pages += npages;
> +		v->balloon_size -= npages;
> +	}
> +	return;
> +}
> +

[snip]

> +static void balloon_config_changed(struct virtio_device *vdev)
> +{
> +	struct virtballoon *v = vdev->priv;
> +	u32 target_nrpages;
> +

A check should be added to see if rmmod_wait is active.
If it is then don't allow the monitor to inflate the balloon since
we like to remove the module.
Best regards,
Dor

> +	__virtio_config_val(v->vdev, 0, &target_nrpages);
> +	atomic_set(&v->target_nrpages, target_nrpages);
> +	wake_up(&v->balloon_wait);
> +	dprintk(&vdev->dev, "%s\n", __func__);
> +}
> +
> +static struct virtio_driver virtio_balloon = {
> +	.driver.name =  KBUILD_MODNAME,
> +	.driver.owner = THIS_MODULE,
> +	.id_table =     id_table,
> +	.probe =        balloon_probe,
> +	.remove =	__devexit_p(balloon_remove),
> +	.config_changed	= balloon_config_changed,
> +};
> +
> +module_param(kvm_balloon_debug, int, 0);
> +
> +static int __init kvm_balloon_init(void)
> +{
> +	return register_virtio_driver(&virtio_balloon);
> +}
> +
> +static void __exit kvm_balloon_exit(void)
> +{
> +	struct virtballoon *v;
> +	
> +	list_for_each_entry(v, &balloon_devices, list) { 
> +		while (v->balloon_size) {
> +			DEFINE_WAIT(wait);
> +
> +			atomic_add(v->balloon_size, &v->target_nrpages);
> +			wake_up(&v->balloon_wait);
> +			prepare_to_wait(&v->rmmod_wait, &wait,
> +					TASK_INTERRUPTIBLE);
> +			schedule_timeout(HZ*10);
> +			finish_wait(&v->rmmod_wait, &wait);
> +		}
> +	}
> +
> +	unregister_virtio_driver(&virtio_balloon);
> +}
> +
> +module_init(kvm_balloon_init);
> +module_exit(kvm_balloon_exit);
> Index: linux-2.6-nv/drivers/virtio/virtio_pci.c
> ===================================================================
> --- linux-2.6-nv.orig/drivers/virtio/virtio_pci.c
> +++ linux-2.6-nv/drivers/virtio/virtio_pci.c
> @@ -67,6 +67,7 @@ static struct pci_device_id virtio_pci_i
>  	{ 0x1AF4, 0x1000, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }, /* Dummy entry */
>  	{ 0x1AF4, 0x1001, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }, /* Dummy entry */
>  	{ 0x1AF4, 0x1002, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }, /* Dummy entry */
> +	{ 0x1AF4, 0x1003, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 }, /* Balloon */
>  	{ 0 },
>  };
>  
> Index: linux-2.6-nv/include/linux/virtio_balloon.h
> ===================================================================
> --- /dev/null
> +++ linux-2.6-nv/include/linux/virtio_balloon.h
> @@ -0,0 +1,20 @@
> +#ifndef _LINUX_VIRTIO_BALLOON_H
> +#define _LINUX_VIRTIO_BALLOON_H
> +#include <linux/virtio_config.h>
> +
> +#define VIRTIO_ID_BALLOON 3
> +
> +#define CMD_BALLOON_INFLATE 0x1
> +#define CMD_BALLOON_DEFLATE 0x2
> +
> +struct virtio_balloon_hdr {
> +	__u8 cmd;
> +	__u8 status;
> +};
> +
> +struct virtio_balloon_config
> +{
> +	__u32 target_nrpages;
> +};
> +
> +#endif /* _LINUX_VIRTIO_BALLOON_H */
> _______________________________________________
> Virtualization mailing list
> Virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/virtualization


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[PATCH] KVM simplified virtio balloon driver

[Rusty Russell]

After discussions with Anthony Liguori, it seems that the virtio
balloon can be made even simpler.  Here's my attempt.

Since the balloon requires Guest cooperation anyway, there seems
little reason to force it to tell the Host when it wants to reuse a
page.  It can simply fault it in.

Moreover, the target is best expressed in balloon size, since there is
no portable way of getting the total RAM in the system.  The host can
do the math.

Tested with a (fairly hacky) lguest patch.

Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
---
 drivers/virtio/Kconfig          |   10 +
 drivers/virtio/Makefile         |    1
 drivers/virtio/virtio_balloon.c |  230 ++++++++++++++++++++++++++++++++++++++++
 include/linux/virtio_balloon.h  |   13 ++
 4 files changed, 254 insertions(+)

diff -r c4762959de25 drivers/virtio/Kconfig
--- a/drivers/virtio/Kconfig	Thu Jan 17 10:31:37 2008 +1100
+++ b/drivers/virtio/Kconfig	Thu Jan 17 12:28:23 2008 +1100
@@ -23,3 +23,13 @@ config VIRTIO_PCI
 
 	  If unsure, say M.
 
+config VIRTIO_BALLOON
+	tristate "Virtio balloon driver (EXPERIMENTAL)"
+	select VIRTIO
+	select VIRTIO_RING
+	---help---
+	 This driver supports increasing and decreasing the amount 
+	 of memory within a KVM guest.
+
+	 If unsure, say M.
+
diff -r c4762959de25 drivers/virtio/Makefile
--- a/drivers/virtio/Makefile	Thu Jan 17 10:31:37 2008 +1100
+++ b/drivers/virtio/Makefile	Thu Jan 17 12:28:23 2008 +1100
@@ -1,3 +1,4 @@ obj-$(CONFIG_VIRTIO) += virtio.o
 obj-$(CONFIG_VIRTIO) += virtio.o
 obj-$(CONFIG_VIRTIO_RING) += virtio_ring.o
 obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
+obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
diff -r c4762959de25 drivers/virtio/virtio_balloon.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/drivers/virtio/virtio_balloon.c	Thu Jan 17 12:28:23 2008 +1100
@@ -0,0 +1,235 @@
+/* Virtio balloon implementation, inspired by Dor Loar and Marcelo
+ * Tosatti's implementations.
+ *
+ *  Copyright 2008 Rusty Russell IBM Corporation
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+#define DEBUG
+#include <linux/virtio.h>
+#include <linux/virtio_balloon.h>
+#include <linux/swap.h>
+#include <linux/kthread.h>
+#include <linux/freezer.h>
+
+struct virtio_balloon
+{
+	struct virtio_device *vdev;
+	struct virtqueue *vq;
+
+	/* Where the ballooning thread waits for config to change. */
+	wait_queue_head_t config_change;
+
+	/* The thread servicing the balloon. */
+	struct task_struct *thread;
+
+	/* Waiting for host to ack the pages we released. */
+	struct completion acked;
+
+	/* The pages we've told the Host we're not using. */
+	unsigned int num_pages;
+	struct list_head pages;
+
+	/* The array of pfns we tell the Host about. */
+	unsigned int num_pfns;
+	u32 pfns[256];
+};
+
+static struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_BALLOON, VIRTIO_DEV_ANY_ID},
+	{ 0 },
+};
+
+static void leak_balloon(struct virtio_balloon *vb, unsigned int num)
+{
+	struct page *page;
+	unsigned int i;
+
+	/* Simply free pages, and usage will fault them back in. */
+	for (i = 0; i < num; i++) {
+		page = list_first_entry(&vb->pages, struct page, lru);
+		list_del(&page->lru);
+		__free_page(page);
+		vb->num_pages--;
+		totalram_pages++;
+	}
+}
+
+static void balloon_ack(struct virtqueue *vq)
+{
+	struct virtio_balloon *vb;
+	unsigned int len;
+
+	vb = vq->vq_ops->get_buf(vq, &len);
+	if (vb)
+		complete(&vb->acked);
+}
+
+static void fill_balloon(struct virtio_balloon *vb, unsigned int num)
+{
+	struct scatterlist sg;
+
+	/* We can only do one array worth at a time. */
+	num = min(num, ARRAY_SIZE(vb->pfns));
+
+	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
+		struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY);
+		if (!page) {
+			if (printk_ratelimit())
+				dev_printk(KERN_INFO, &vb->vdev->dev,
+					   "Out of puff! Can't get %u pages\n",
+					   num);
+			/* Sleep for at least 1/5 of a second before retry. */
+			msleep(200);
+			break;
+		}
+		vb->pfns[vb->num_pfns] = page_to_pfn(page);
+		totalram_pages--;
+		vb->num_pages++;
+		list_add(&page->lru, &vb->pages);
+	}
+
+	/* Didn't get any?  Oh well. */
+	if (vb->num_pfns == 0)
+		return;
+	
+	init_completion(&vb->acked);
+	sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
+
+	/* We should always be able to add one buffer to an empty queue. */
+	if (vb->vq->vq_ops->add_buf(vb->vq, &sg, 1, 0, vb) != 0)
+		BUG();
+	vb->vq->vq_ops->kick(vb->vq);
+
+	/* When host has read buffer, this completes via balloon_ack */
+	wait_for_completion(&vb->acked);
+}
+
+static void virtballoon_changed(struct virtio_device *vdev)
+{
+	struct virtio_balloon *vb = vdev->priv;
+
+	wake_up(&vb->config_change);
+}
+
+static inline int towards_target(struct virtio_balloon *vb)
+{
+	u32 v;
+	__virtio_config_val(vb->vdev,
+			    offsetof(struct virtio_balloon_config, num_pages),
+			    &v);
+	return v - vb->num_pages;
+}
+
+static int balloon(void *_vballoon)
+{
+	struct virtio_balloon *vb = _vballoon;
+
+	set_freezable();
+	while (!kthread_should_stop()) {
+		int diff;
+
+		try_to_freeze();
+		wait_event_interruptible(vb->config_change,
+					 (diff = towards_target(vb)) != 0
+					 || kthread_should_stop());
+		if (diff > 0)
+			fill_balloon(vb, diff);
+		else if (diff < 0)
+			leak_balloon(vb, -diff);
+	}
+	return 0;
+}
+
+static int virtballoon_probe(struct virtio_device *vdev)
+{
+	struct virtio_balloon *vb;
+	int err;
+
+	vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
+	if (!vb) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	INIT_LIST_HEAD(&vb->pages);
+	vb->num_pages = 0;
+	init_waitqueue_head(&vb->config_change);
+	vb->vdev = vdev;
+
+	/* We expect one virtqueue, for output. */
+	vb->vq = vdev->config->find_vq(vdev, 0, balloon_ack);
+	if (IS_ERR(vb->vq)) {
+		err = PTR_ERR(vb->vq);
+		goto out_free_vb;
+	}
+
+	vb->thread = kthread_run(balloon, vb, "vballoon");
+	if (IS_ERR(vb->thread)) {
+		err = PTR_ERR(vb->thread);
+		goto out_del_vq;
+	}
+	return 0;
+
+out_del_vq:
+	vdev->config->del_vq(vb->vq);
+out_free_vb:
+	kfree(vb);
+out:
+	return err;
+}
+
+static void virtballoon_remove(struct virtio_device *vdev)
+{
+	struct virtio_balloon *vb = vdev->priv;
+	struct page *i, *next;
+
+	kthread_stop(vb->thread);
+
+	/* There might be pages left in the balloon: free them. */
+	list_for_each_entry_safe(i, next, &vb->pages, lru) {
+		vb->num_pages--;
+		__free_page(i);
+	}
+	BUG_ON(vb->num_pages);
+
+	vdev->config->del_vq(vb->vq);
+	kfree(vb);
+}
+
+static struct virtio_driver virtio_balloon = {
+	.driver.name =	KBUILD_MODNAME,
+	.driver.owner =	THIS_MODULE,
+	.id_table =	id_table,
+	.probe =	virtballoon_probe,
+	.remove =	__devexit_p(virtballoon_remove),
+	.config_changed = virtballoon_changed,
+};
+
+static int __init init(void)
+{
+	return register_virtio_driver(&virtio_balloon);
+}
+
+static void __exit fini(void)
+{
+	unregister_virtio_driver(&virtio_balloon);
+}
+module_init(init);
+module_exit(fini);
+
+MODULE_DEVICE_TABLE(virtio, id_table);
+MODULE_DESCRIPTION("Virtio balloon driver");
+MODULE_LICENSE("GPL");
diff -r c4762959de25 include/linux/virtio_balloon.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/include/linux/virtio_balloon.h	Thu Jan 17 12:28:23 2008 +1100
@@ -0,0 +1,13 @@
+#ifndef _LINUX_VIRTIO_BALLOON_H
+#define _LINUX_VIRTIO_BALLOON_H
+#include <linux/virtio_config.h>
+
+/* The ID for virtio_balloon */
+#define VIRTIO_ID_BALLOON	5
+
+struct virtio_balloon_config
+{
+	/* Number of pages host wants Guest to give up. */
+	__le32 num_pages;
+};
+#endif /* _LINUX_VIRTIO_BALLOON_H */

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Anthony Liguori]

Rusty Russell wrote:
> After discussions with Anthony Liguori, it seems that the virtio
> balloon can be made even simpler.  Here's my attempt.
>
> Since the balloon requires Guest cooperation anyway, there seems
> little reason to force it to tell the Host when it wants to reuse a
> page.  It can simply fault it in.
>
> Moreover, the target is best expressed in balloon size, since there is
> no portable way of getting the total RAM in the system.  The host can
> do the math.
>
> Tested with a (fairly hacky) lguest patch.
>
> Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
> ---
>  drivers/virtio/Kconfig          |   10 +
>  drivers/virtio/Makefile         |    1
>  drivers/virtio/virtio_balloon.c |  230 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/virtio_balloon.h  |   13 ++
>  4 files changed, 254 insertions(+)
>
> diff -r c4762959de25 drivers/virtio/Kconfig
> --- a/drivers/virtio/Kconfig	Thu Jan 17 10:31:37 2008 +1100
> +++ b/drivers/virtio/Kconfig	Thu Jan 17 12:28:23 2008 +1100
> @@ -23,3 +23,13 @@ config VIRTIO_PCI
>  
>  	  If unsure, say M.
>  
> +config VIRTIO_BALLOON
> +	tristate "Virtio balloon driver (EXPERIMENTAL)"
> +	select VIRTIO
> +	select VIRTIO_RING
> +	---help---
> +	 This driver supports increasing and decreasing the amount 
> +	 of memory within a KVM guest.
> +
> +	 If unsure, say M.
> +
> diff -r c4762959de25 drivers/virtio/Makefile
> --- a/drivers/virtio/Makefile	Thu Jan 17 10:31:37 2008 +1100
> +++ b/drivers/virtio/Makefile	Thu Jan 17 12:28:23 2008 +1100
> @@ -1,3 +1,4 @@ obj-$(CONFIG_VIRTIO) += virtio.o
>  obj-$(CONFIG_VIRTIO) += virtio.o
>  obj-$(CONFIG_VIRTIO_RING) += virtio_ring.o
>  obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
> +obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
> diff -r c4762959de25 drivers/virtio/virtio_balloon.c
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/drivers/virtio/virtio_balloon.c	Thu Jan 17 12:28:23 2008 +1100
> @@ -0,0 +1,235 @@
> +/* Virtio balloon implementation, inspired by Dor Loar and Marcelo
> + * Tosatti's implementations.
> + *
> + *  Copyright 2008 Rusty Russell IBM Corporation
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + */
> +#define DEBUG
> +#include <linux/virtio.h>
> +#include <linux/virtio_balloon.h>
> +#include <linux/swap.h>
> +#include <linux/kthread.h>
> +#include <linux/freezer.h>
> +
> +struct virtio_balloon
> +{
> +	struct virtio_device *vdev;
> +	struct virtqueue *vq;
> +
> +	/* Where the ballooning thread waits for config to change. */
> +	wait_queue_head_t config_change;
> +
> +	/* The thread servicing the balloon. */
> +	struct task_struct *thread;
> +
> +	/* Waiting for host to ack the pages we released. */
> +	struct completion acked;
> +
> +	/* The pages we've told the Host we're not using. */
> +	unsigned int num_pages;
> +	struct list_head pages;
> +
> +	/* The array of pfns we tell the Host about. */
> +	unsigned int num_pfns;
> +	u32 pfns[256];
> +};
> +
> +static struct virtio_device_id id_table[] = {
> +	{ VIRTIO_ID_BALLOON, VIRTIO_DEV_ANY_ID},
>   

Could use a space after VIRTIO_DEV_ANY_ID

> +	{ 0 },
> +};
> +
> +static void leak_balloon(struct virtio_balloon *vb, unsigned int num)
> +{
> +	struct page *page;
> +	unsigned int i;
> +
> +	/* Simply free pages, and usage will fault them back in. */
> +	for (i = 0; i < num; i++) {
> +		page = list_first_entry(&vb->pages, struct page, lru);
> +		list_del(&page->lru);
> +		__free_page(page);
> +		vb->num_pages--;
> +		totalram_pages++;
>   

Do we really want to modify totalram_pages in this driver?  The only 
other place that I see that modifies it is in mm/memory_hotplug and it 
also modifies other things (like num_physpages).  The cmm driver doesn't 
touch totalram_pages.

It would be very useful too to write vb->num_pages into the config space 
whenever it was updated.  This way, the host can easily keep track of 
where the guest is at in terms of ballooning.

Regards,

Anthony Liguori

> +	}
> +}
> +
> +static void balloon_ack(struct virtqueue *vq)
> +{
> +	struct virtio_balloon *vb;
> +	unsigned int len;
> +
> +	vb = vq->vq_ops->get_buf(vq, &len);
> +	if (vb)
> +		complete(&vb->acked);
> +}
> +
> +static void fill_balloon(struct virtio_balloon *vb, unsigned int num)
> +{
> +	struct scatterlist sg;
> +
> +	/* We can only do one array worth at a time. */
> +	num = min(num, ARRAY_SIZE(vb->pfns));
> +
> +	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
> +		struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY);
> +		if (!page) {
> +			if (printk_ratelimit())
> +				dev_printk(KERN_INFO, &vb->vdev->dev,
> +					   "Out of puff! Can't get %u pages\n",
> +					   num);
> +			/* Sleep for at least 1/5 of a second before retry. */
> +			msleep(200);
> +			break;
> +		}
> +		vb->pfns[vb->num_pfns] = page_to_pfn(page);
> +		totalram_pages--;
> +		vb->num_pages++;
> +		list_add(&page->lru, &vb->pages);
> +	}
> +
> +	/* Didn't get any?  Oh well. */
> +	if (vb->num_pfns == 0)
> +		return;
> +	
> +	init_completion(&vb->acked);
> +	sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
> +
> +	/* We should always be able to add one buffer to an empty queue. */
> +	if (vb->vq->vq_ops->add_buf(vb->vq, &sg, 1, 0, vb) != 0)
> +		BUG();
> +	vb->vq->vq_ops->kick(vb->vq);
> +
> +	/* When host has read buffer, this completes via balloon_ack */
> +	wait_for_completion(&vb->acked);
> +}
> +
> +static void virtballoon_changed(struct virtio_device *vdev)
> +{
> +	struct virtio_balloon *vb = vdev->priv;
> +
> +	wake_up(&vb->config_change);
> +}
> +
> +static inline int towards_target(struct virtio_balloon *vb)
> +{
> +	u32 v;
> +	__virtio_config_val(vb->vdev,
> +			    offsetof(struct virtio_balloon_config, num_pages),
> +			    &v);
> +	return v - vb->num_pages;
> +}
> +
> +static int balloon(void *_vballoon)
> +{
> +	struct virtio_balloon *vb = _vballoon;
> +
> +	set_freezable();
> +	while (!kthread_should_stop()) {
> +		int diff;
> +
> +		try_to_freeze();
> +		wait_event_interruptible(vb->config_change,
> +					 (diff = towards_target(vb)) != 0
> +					 || kthread_should_stop());
> +		if (diff > 0)
> +			fill_balloon(vb, diff);
> +		else if (diff < 0)
> +			leak_balloon(vb, -diff);
> +	}
> +	return 0;
> +}
> +
> +static int virtballoon_probe(struct virtio_device *vdev)
> +{
> +	struct virtio_balloon *vb;
> +	int err;
> +
> +	vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
> +	if (!vb) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +
> +	INIT_LIST_HEAD(&vb->pages);
> +	vb->num_pages = 0;
> +	init_waitqueue_head(&vb->config_change);
> +	vb->vdev = vdev;
> +
> +	/* We expect one virtqueue, for output. */
> +	vb->vq = vdev->config->find_vq(vdev, 0, balloon_ack);
> +	if (IS_ERR(vb->vq)) {
> +		err = PTR_ERR(vb->vq);
> +		goto out_free_vb;
> +	}
> +
> +	vb->thread = kthread_run(balloon, vb, "vballoon");
> +	if (IS_ERR(vb->thread)) {
> +		err = PTR_ERR(vb->thread);
> +		goto out_del_vq;
> +	}
> +	return 0;
> +
> +out_del_vq:
> +	vdev->config->del_vq(vb->vq);
> +out_free_vb:
> +	kfree(vb);
> +out:
> +	return err;
> +}
> +
> +static void virtballoon_remove(struct virtio_device *vdev)
> +{
> +	struct virtio_balloon *vb = vdev->priv;
> +	struct page *i, *next;
> +
> +	kthread_stop(vb->thread);
> +
> +	/* There might be pages left in the balloon: free them. */
> +	list_for_each_entry_safe(i, next, &vb->pages, lru) {
> +		vb->num_pages--;
> +		__free_page(i);
> +	}
> +	BUG_ON(vb->num_pages);
> +
> +	vdev->config->del_vq(vb->vq);
> +	kfree(vb);
> +}
> +
> +static struct virtio_driver virtio_balloon = {
> +	.driver.name =	KBUILD_MODNAME,
> +	.driver.owner =	THIS_MODULE,
> +	.id_table =	id_table,
> +	.probe =	virtballoon_probe,
> +	.remove =	__devexit_p(virtballoon_remove),
> +	.config_changed = virtballoon_changed,
> +};
> +
> +static int __init init(void)
> +{
> +	return register_virtio_driver(&virtio_balloon);
> +}
> +
> +static void __exit fini(void)
> +{
> +	unregister_virtio_driver(&virtio_balloon);
> +}
> +module_init(init);
> +module_exit(fini);
> +
> +MODULE_DEVICE_TABLE(virtio, id_table);
> +MODULE_DESCRIPTION("Virtio balloon driver");
> +MODULE_LICENSE("GPL");
> diff -r c4762959de25 include/linux/virtio_balloon.h
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/include/linux/virtio_balloon.h	Thu Jan 17 12:28:23 2008 +1100
> @@ -0,0 +1,13 @@
> +#ifndef _LINUX_VIRTIO_BALLOON_H
> +#define _LINUX_VIRTIO_BALLOON_H
> +#include <linux/virtio_config.h>
> +
> +/* The ID for virtio_balloon */
> +#define VIRTIO_ID_BALLOON	5
> +
> +struct virtio_balloon_config
> +{
> +	/* Number of pages host wants Guest to give up. */
> +	__le32 num_pages;
> +};
> +#endif /* _LINUX_VIRTIO_BALLOON_H */
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> kvm-devel mailing list
> kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Rusty Russell]

On Thursday 17 January 2008 13:14:58 Anthony Liguori wrote:
> Rusty Russell wrote:
> > +static struct virtio_device_id id_table[] = {
> > +	{ VIRTIO_ID_BALLOON, VIRTIO_DEV_ANY_ID},
>
> Could use a space after VIRTIO_DEV_ANY_ID

Thanks, fixed.

> > +		__free_page(page);
> > +		vb->num_pages--;
> > +		totalram_pages++;
>
> Do we really want to modify totalram_pages in this driver?  The only
> other place that I see that modifies it is in mm/memory_hotplug and it
> also modifies other things (like num_physpages).  The cmm driver doesn't
> touch totalram_pages.

I don't think there's a standard here, they're all ad-hoc (eg. no locking)  
Modifying totalram_pages has the nice effect of showing up in "free" in the 
guest.

We should probably not modify num_physpages, because some places seem to use 
it as an address space limit.  But we should probably fix all those 
networking size heuristics to use totalram_pages instead of num_physpages.

> It would be very useful too to write vb->num_pages into the config space
> whenever it was updated.  This way, the host can easily keep track of
> where the guest is at in terms of ballooning.

OTOH it's currently pretty obvious (and usually fatal) if the guest has 
trouble meeting the balloon requirements.  A serious host needs a way of 
detecting stress in the guest anyway, which this doesn't offer until it's too 
late...

Rusty.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Anthony Liguori]

Rusty Russell wrote:
> On Thursday 17 January 2008 13:14:58 Anthony Liguori wrote:
>   
>> Rusty Russell wrote:
>>     
>>> +static struct virtio_device_id id_table[] = {
>>> +	{ VIRTIO_ID_BALLOON, VIRTIO_DEV_ANY_ID},
>>>       
>> Could use a space after VIRTIO_DEV_ANY_ID
>>     
>
> Thanks, fixed.
>
>   
>>> +		__free_page(page);
>>> +		vb->num_pages--;
>>> +		totalram_pages++;
>>>       
>> Do we really want to modify totalram_pages in this driver?  The only
>> other place that I see that modifies it is in mm/memory_hotplug and it
>> also modifies other things (like num_physpages).  The cmm driver doesn't
>> touch totalram_pages.
>>     
>
> I don't think there's a standard here, they're all ad-hoc (eg. no locking)  
> Modifying totalram_pages has the nice effect of showing up in "free" in the 
> guest.
>
> We should probably not modify num_physpages, because some places seem to use 
> it as an address space limit.  But we should probably fix all those 
> networking size heuristics to use totalram_pages instead of num_physpages.
>
>   
>> It would be very useful too to write vb->num_pages into the config space
>> whenever it was updated.  This way, the host can easily keep track of
>> where the guest is at in terms of ballooning.
>>     
>
> OTOH it's currently pretty obvious (and usually fatal) if the guest has 
> trouble meeting the balloon requirements.  A serious host needs a way of 
> detecting stress in the guest anyway, which this doesn't offer until it's too 
> late...
>   

The question I'm interested in answering though is not if but when.  I 
would like to know when the guest has reached it's target.

And while we do get the madvise call outs, it's possible that pages have 
been faulted in since then.

Regards,

Anthony Liguori

> Rusty.
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Rusty Russell]

On Thursday 17 January 2008 15:01:46 Anthony Liguori wrote:
> Rusty Russell wrote:
> > OTOH it's currently pretty obvious (and usually fatal) if the guest has
> > trouble meeting the balloon requirements.  A serious host needs a way of
> > detecting stress in the guest anyway, which this doesn't offer until it's
> > too late...
>
> The question I'm interested in answering though is not if but when.  I
> would like to know when the guest has reached it's target.

I'm saying that it will be v. quickly in all but "too much squeeze" case.

> And while we do get the madvise call outs, it's possible that pages have
> been faulted in since then.

But that's exactly what the balloon number *doesn't* tell you.  It can tell 
you that it's released pages back to be used by the OS, but not whether the 
OS has used them.

I think this number is good for debugging the balloon driver, but for anything 
else it's a false friend.

Rusty.
PS.  Please cut down mails when you reply.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Avi Kivity]

Anthony Liguori wrote:
>>   
>>     
>>> It would be very useful too to write vb->num_pages into the config space
>>> whenever it was updated.  This way, the host can easily keep track of
>>> where the guest is at in terms of ballooning.
>>>     
>>>       
>> OTOH it's currently pretty obvious (and usually fatal) if the guest has 
>> trouble meeting the balloon requirements.  A serious host needs a way of 
>> detecting stress in the guest anyway, which this doesn't offer until it's too 
>> late...
>>   
>>     
>
> The question I'm interested in answering though is not if but when.  I 
> would like to know when the guest has reached it's target.
>
>   

Yes.  We don't want to activate the RSS controller immediately; give the 
guest some time to adjust voluntarily.


-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Christian Borntraeger]

Am Donnerstag, 17. Januar 2008 schrieb Rusty Russell:
> Since the balloon requires Guest cooperation anyway, there seems
> little reason to force it to tell the Host when it wants to reuse a
> page.  It can simply fault it in.
Yes, thats what we do in the s390 cmm driver.

All in all the driver has similarities with cmm. I dont know, if we can 
consolidate some code.
Besides the hypervisor, we have a additional user interface: /proc/sys/vm/. 
The root user can specify the amount of pages in the balloon 
via /proc/sys/vm/cmm_pages. 

Another idea: Martin added an oom notifier to the cmm driver. Before the 
oom-killer kicks in cmm will try to free 256 pages. I think your virtio 
balloon driver should do the same - it seems to be the correct tradeoff. 

Christian


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Martin Schwidefsky]

On Thu, 2008-01-17 at 10:32 +0100, Christian Borntraeger wrote:
> Am Donnerstag, 17. Januar 2008 schrieb Rusty Russell:
> > Since the balloon requires Guest cooperation anyway, there seems
> > little reason to force it to tell the Host when it wants to reuse a
> > page.  It can simply fault it in.
> Yes, thats what we do in the s390 cmm driver.
> 
> All in all the driver has similarities with cmm. I dont know, if we can 
> consolidate some code.
> Besides the hypervisor, we have a additional user interface: /proc/sys/vm/. 
> The root user can specify the amount of pages in the balloon 
> via /proc/sys/vm/cmm_pages. 
> 
> Another idea: Martin added an oom notifier to the cmm driver. Before the 
> oom-killer kicks in cmm will try to free 256 pages. I think your virtio 
> balloon driver should do the same - it seems to be the correct tradeoff. 

Nod, you definitly want to add an oom notifier. If 256 pages is the
correct number of pages to free is debatable. We have seen long delays
for a process that quickly eats up memory if there are lots of pages in
the balloon. The problem is that the memory management tries hard to
find memory until it decides to oom kill a process, only to be stopped
in the last second by the oom notifier. The 1MB is quickly eaten up
again so the whole things starts again. The profile of such a scenario
shows that almost all cpu is burned in the page reclaim code.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Dor Laor]

On Thu, 2008-01-17 at 11:25 +0100, Martin Schwidefsky wrote:
> > 
> > Another idea: Martin added an oom notifier to the cmm driver. Before the 
> > oom-killer kicks in cmm will try to free 256 pages. I think your virtio 
> > balloon driver should do the same - it seems to be the correct tradeoff. 
> 
> Nod, you definitly want to add an oom notifier. If 256 pages is the
> correct number of pages to free is debatable. We have seen long delays
> for a process that quickly eats up memory if there are lots of pages in
> the balloon. The problem is that the memory management tries hard to
> find memory until it decides to oom kill a process, only to be stopped
> in the last second by the oom notifier. The 1MB is quickly eaten up
> again so the whole things starts again. The profile of such a scenario
> shows that almost all cpu is burned in the page reclaim code.
> 

Seconded, in that case we can add a config space notification from the
guest to the host that will be triggered by the oom.
The host will get this notification and will decide whether to allow the
guest to deflate the balloon or to keep the current balloon size because
the whole host is over committed.
Regards,
Dor.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Anthony Liguori]

Dor Laor wrote:
> On Thu, 2008-01-17 at 11:25 +0100, Martin Schwidefsky wrote:
>   
>>> Another idea: Martin added an oom notifier to the cmm driver. Before the 
>>> oom-killer kicks in cmm will try to free 256 pages. I think your virtio 
>>> balloon driver should do the same - it seems to be the correct tradeoff. 
>>>       
>> Nod, you definitly want to add an oom notifier. If 256 pages is the
>> correct number of pages to free is debatable. We have seen long delays
>> for a process that quickly eats up memory if there are lots of pages in
>> the balloon. The problem is that the memory management tries hard to
>> find memory until it decides to oom kill a process, only to be stopped
>> in the last second by the oom notifier. The 1MB is quickly eaten up
>> again so the whole things starts again. The profile of such a scenario
>> shows that almost all cpu is burned in the page reclaim code.
>>
>>     
>
> Seconded, in that case we can add a config space notification from the
> guest to the host that will be triggered by the oom.
> The host will get this notification and will decide whether to allow the
> guest to deflate the balloon or to keep the current balloon size because
> the whole host is over committed.
>   

The host doesn't decide whether to allow the guest to deflate.  Virtual 
memory size and resident memory side are independent in KVM.  The host 
decides what the RSS is but the guest is free to determine it's VSS.  
The host provides hints to the guest about it's RSS size (via 
ballooning) so the guest can optimize it's VSS size.  If guest chooses 
to make it's VSS size to large, it will only hurt itself (by being 
forced to swap by the host).

Regards,

Anthony Liguori

> Regards,
> Dor.
>
>   

Re: [PATCH] KVM simplified virtio balloon driver

[Dor Laor]

On Thu, 2008-01-17 at 07:56 -0600, Anthony Liguori wrote:
> Dor Laor wrote:
> > On Thu, 2008-01-17 at 11:25 +0100, Martin Schwidefsky wrote:
> >   
> >>> Another idea: Martin added an oom notifier to the cmm driver. Before the 
> >>> oom-killer kicks in cmm will try to free 256 pages. I think your virtio 
> >>> balloon driver should do the same - it seems to be the correct tradeoff. 
> >>>       
> >> Nod, you definitly want to add an oom notifier. If 256 pages is the
> >> correct number of pages to free is debatable. We have seen long delays
> >> for a process that quickly eats up memory if there are lots of pages in
> >> the balloon. The problem is that the memory management tries hard to
> >> find memory until it decides to oom kill a process, only to be stopped
> >> in the last second by the oom notifier. The 1MB is quickly eaten up
> >> again so the whole things starts again. The profile of such a scenario
> >> shows that almost all cpu is burned in the page reclaim code.
> >>
> >>     
> >
> > Seconded, in that case we can add a config space notification from the
> > guest to the host that will be triggered by the oom.
> > The host will get this notification and will decide whether to allow the
> > guest to deflate the balloon or to keep the current balloon size because
> > the whole host is over committed.
> >   
> 
> The host doesn't decide whether to allow the guest to deflate.  Virtual 
> memory size and resident memory side are independent in KVM.  The host 
> decides what the RSS is but the guest is free to determine it's VSS.  
> The host provides hints to the guest about it's RSS size (via 
> ballooning) so the guest can optimize it's VSS size.  If guest chooses 
> to make it's VSS size to large, it will only hurt itself (by being 
> forced to swap by the host).

ok, this is true when the new memory controller targeted for the next
kernel.In that case I more then agree.
Nevertheless, the guest should pass the host data about it's current
memory usage (working set size, oom triggering, etc) so the host would 
change the rss limit dynamically.
Thanks,
Dor


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Anthony Liguori]

Dor Laor wrote:
> On Thu, 2008-01-17 at 07:56 -0600, Anthony Liguori wrote:
>   
>> Dor Laor wrote:
>>     
>>> On Thu, 2008-01-17 at 11:25 +0100, Martin Schwidefsky wrote:
>>>   
>>>       
>>>>> Another idea: Martin added an oom notifier to the cmm driver. Before the 
>>>>> oom-killer kicks in cmm will try to free 256 pages. I think your virtio 
>>>>> balloon driver should do the same - it seems to be the correct tradeoff. 
>>>>>       
>>>>>           
>>>> Nod, you definitly want to add an oom notifier. If 256 pages is the
>>>> correct number of pages to free is debatable. We have seen long delays
>>>> for a process that quickly eats up memory if there are lots of pages in
>>>> the balloon. The problem is that the memory management tries hard to
>>>> find memory until it decides to oom kill a process, only to be stopped
>>>> in the last second by the oom notifier. The 1MB is quickly eaten up
>>>> again so the whole things starts again. The profile of such a scenario
>>>> shows that almost all cpu is burned in the page reclaim code.
>>>>
>>>>     
>>>>         
>>> Seconded, in that case we can add a config space notification from the
>>> guest to the host that will be triggered by the oom.
>>> The host will get this notification and will decide whether to allow the
>>> guest to deflate the balloon or to keep the current balloon size because
>>> the whole host is over committed.
>>>   
>>>       
>> The host doesn't decide whether to allow the guest to deflate.  Virtual 
>> memory size and resident memory side are independent in KVM.  The host 
>> decides what the RSS is but the guest is free to determine it's VSS.  
>> The host provides hints to the guest about it's RSS size (via 
>> ballooning) so the guest can optimize it's VSS size.  If guest chooses 
>> to make it's VSS size to large, it will only hurt itself (by being 
>> forced to swap by the host).
>>     
>
> ok, this is true when the new memory controller targeted for the next
> kernel.In that case I more then agree.
> Nevertheless, the guest should pass the host data about it's current
> memory usage (working set size, oom triggering, etc) so the host would 
> change the rss limit dynamically.
>   

What is the host going to do if the guest is OOMing?  There is no way 
that the host can *verify* that the guest is OOMing so nothing prevents 
the guest from lying about the fact that it's OOMing in an attempt to 
get more memory.

More importantly though, the RSS limit has nothing to do with the guest 
OOMing.  Reducing the guest's RSS limit will result in the guest 
swapping, not OOMing.  Only the guest can prevent itself from OOMing by 
free'ing pages that were previously ballooned and that is what is being 
recommended here.

Regards,

Anthony Liguori

> Thanks,
> Dor
>
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Avi Kivity]

Rusty Russell wrote:
> After discussions with Anthony Liguori, it seems that the virtio
> balloon can be made even simpler.  Here's my attempt.
>
> Since the balloon requires Guest cooperation anyway, there seems
> little reason to force it to tell the Host when it wants to reuse a
> page.  It can simply fault it in.
>
>   

Faulting is synchronous, while deflating is (or can be made) 
asynchronous. If the host needs to do some work to get the memory, the 
guest will be slowed down considerably.

If we have explicit deflate, the host can call madvise(MADV_WILLNEED) or 
actually touch the pages before the guest accesses them.


-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Anthony Liguori]

Avi Kivity wrote:
> Rusty Russell wrote:
>> After discussions with Anthony Liguori, it seems that the virtio
>> balloon can be made even simpler.  Here's my attempt.
>>
>> Since the balloon requires Guest cooperation anyway, there seems
>> little reason to force it to tell the Host when it wants to reuse a
>> page.  It can simply fault it in.
>>
>>   
> 
> Faulting is synchronous, while deflating is (or can be made) 
> asynchronous. If the host needs to do some work to get the memory, the 
> guest will be slowed down considerably.

Good point.  Basically, we have two page hinting operations which 
roughly correspond to madvise(MADV_DONTNEED) and madvise(MADV_WILLNEED).

Regards,

Anthony Liguori

> If we have explicit deflate, the host can call madvise(MADV_WILLNEED) or 
> actually touch the pages before the guest accesses them.
> 
> 


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Marcelo Tosatti]

On Sat, Jan 19, 2008 at 04:37:43PM -0600, Anthony Liguori wrote:
> Avi Kivity wrote:
> >Rusty Russell wrote:
> >>After discussions with Anthony Liguori, it seems that the virtio
> >>balloon can be made even simpler.  Here's my attempt.
> >>
> >>Since the balloon requires Guest cooperation anyway, there seems
> >>little reason to force it to tell the Host when it wants to reuse a
> >>page.  It can simply fault it in.
> >>
> >>  
> >
> >Faulting is synchronous, while deflating is (or can be made) 
> >asynchronous. If the host needs to do some work to get the memory, the 
> >guest will be slowed down considerably.
> 
> Good point.  Basically, we have two page hinting operations which 
> roughly correspond to madvise(MADV_DONTNEED) and madvise(MADV_WILLNEED).

Also, the simplified driver does not handle errors at all.

I don't think that assuming madvise() can't fail is a good thing.



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Anthony Liguori]

Marcelo Tosatti wrote:
> On Sat, Jan 19, 2008 at 04:37:43PM -0600, Anthony Liguori wrote:
>   
>> Avi Kivity wrote:
>>     
>>> Rusty Russell wrote:
>>>       
>>>> After discussions with Anthony Liguori, it seems that the virtio
>>>> balloon can be made even simpler.  Here's my attempt.
>>>>
>>>> Since the balloon requires Guest cooperation anyway, there seems
>>>> little reason to force it to tell the Host when it wants to reuse a
>>>> page.  It can simply fault it in.
>>>>
>>>>  
>>>>         
>>> Faulting is synchronous, while deflating is (or can be made) 
>>> asynchronous. If the host needs to do some work to get the memory, the 
>>> guest will be slowed down considerably.
>>>       
>> Good point.  Basically, we have two page hinting operations which 
>> roughly correspond to madvise(MADV_DONTNEED) and madvise(MADV_WILLNEED).
>>     
>
> Also, the simplified driver does not handle errors at all.
>
> I don't think that assuming madvise() can't fail is a good thing.
>   

I don't think the host should be communicating to the guest if madvise() 
fails.  The notification is a hint to the host.  The guest doesn't know 
what the host is doing in response to that hint.

Regards,

Anthony Liguori



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM simplified virtio balloon driver

[Rusty Russell]

Here's the latest.  Hope this works for everyone (putting in a oom handler or
shrinker requires a lock, but can be done quite easily).

Untested.
===
After discussions with Anthony Liguori, it seems that the virtio
balloon can be made even simpler.  Here's my attempt.

Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
---
 drivers/virtio/Kconfig          |   10 +
 drivers/virtio/Makefile         |    1 
 drivers/virtio/virtio_balloon.c |  284 ++++++++++++++++++++++++++++++++++++++++
 include/linux/virtio_balloon.h  |   18 ++
 4 files changed, 313 insertions(+)

diff -r e977a88ffefd drivers/virtio/Kconfig
--- a/drivers/virtio/Kconfig	Thu Jan 24 12:34:05 2008 +1100
+++ b/drivers/virtio/Kconfig	Thu Jan 24 12:40:07 2008 +1100
@@ -23,3 +23,13 @@ config VIRTIO_PCI
 
 	  If unsure, say M.
 
+config VIRTIO_BALLOON
+	tristate "Virtio balloon driver (EXPERIMENTAL)"
+	select VIRTIO
+	select VIRTIO_RING
+	---help---
+	 This driver supports increasing and decreasing the amount 
+	 of memory within a KVM guest.
+
+	 If unsure, say M.
+
diff -r e977a88ffefd drivers/virtio/Makefile
--- a/drivers/virtio/Makefile	Thu Jan 24 12:34:05 2008 +1100
+++ b/drivers/virtio/Makefile	Thu Jan 24 12:40:07 2008 +1100
@@ -1,3 +1,4 @@ obj-$(CONFIG_VIRTIO) += virtio.o
 obj-$(CONFIG_VIRTIO) += virtio.o
 obj-$(CONFIG_VIRTIO_RING) += virtio_ring.o
 obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
+obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
diff -r e977a88ffefd drivers/virtio/virtio_balloon.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/drivers/virtio/virtio_balloon.c	Thu Jan 24 12:40:07 2008 +1100
@@ -0,0 +1,284 @@
+/* Virtio balloon implementation, inspired by Dor Loar and Marcelo
+ * Tosatti's implementations.
+ *
+ *  Copyright 2008 Rusty Russell IBM Corporation
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+#define DEBUG
+#include <linux/virtio.h>
+#include <linux/virtio_balloon.h>
+#include <linux/swap.h>
+#include <linux/kthread.h>
+#include <linux/freezer.h>
+
+struct virtio_balloon
+{
+	struct virtio_device *vdev;
+	struct virtqueue *inflate_vq, *deflate_vq;
+
+	/* Where the ballooning thread waits for config to change. */
+	wait_queue_head_t config_change;
+
+	/* The thread servicing the balloon. */
+	struct task_struct *thread;
+
+	/* Waiting for host to ack the pages we released. */
+	struct completion acked;
+
+	/* Do we have to tell Host *before* we reuse pages? */
+	bool tell_host_first;
+
+	/* The pages we've told the Host we're not using. */
+	unsigned int num_pages;
+	struct list_head pages;
+
+	/* The array of pfns we tell the Host about. */
+	unsigned int num_pfns;
+	u32 pfns[256];
+};
+
+static struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_BALLOON, VIRTIO_DEV_ANY_ID },
+	{ 0 },
+};
+
+static void balloon_ack(struct virtqueue *vq)
+{
+	struct virtio_balloon *vb;
+	unsigned int len;
+
+	vb = vq->vq_ops->get_buf(vq, &len);
+	if (vb)
+		complete(&vb->acked);
+}
+
+static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
+{
+	struct scatterlist sg;
+
+	sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
+
+	init_completion(&vb->acked);
+
+	/* We should always be able to add one buffer to an empty queue. */
+	if (vq->vq_ops->add_buf(vq, &sg, 1, 0, vb) != 0)
+		BUG();
+	vq->vq_ops->kick(vq);
+
+	/* When host has read buffer, this completes via balloon_ack */
+	wait_for_completion(&vb->acked);
+}
+
+static void fill_balloon(struct virtio_balloon *vb, unsigned int num)
+{
+	/* We can only do one array worth at a time. */
+	num = min(num, ARRAY_SIZE(vb->pfns));
+
+	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
+		struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY);
+		if (!page) {
+			if (printk_ratelimit())
+				dev_printk(KERN_INFO, &vb->vdev->dev,
+					   "Out of puff! Can't get %u pages\n",
+					   num);
+			/* Sleep for at least 1/5 of a second before retry. */
+			msleep(200);
+			break;
+		}
+		vb->pfns[vb->num_pfns] = page_to_pfn(page);
+		totalram_pages--;
+		vb->num_pages++;
+		list_add(&page->lru, &vb->pages);
+	}
+
+	/* Didn't get any?  Oh well. */
+	if (vb->num_pfns == 0)
+		return;
+
+	tell_host(vb, vb->inflate_vq);
+}
+
+static void release_pages_by_pfn(const u32 pfns[], unsigned int num)
+{
+	unsigned int i;
+
+	for (i = 0; i < num; i++) {
+		__free_page(pfn_to_page(pfns[i]));
+		totalram_pages++;
+	}
+}
+
+static void leak_balloon(struct virtio_balloon *vb, unsigned int num)
+{
+	struct page *page;
+
+	/* We can only do one array worth at a time. */
+	num = min(num, ARRAY_SIZE(vb->pfns));
+
+	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
+		page = list_first_entry(&vb->pages, struct page, lru);
+		list_del(&page->lru);
+		vb->pfns[vb->num_pfns] = page_to_pfn(page);
+		vb->num_pages--;
+	}
+
+	if (vb->tell_host_first) {
+		tell_host(vb, vb->deflate_vq);
+		release_pages_by_pfn(vb->pfns, vb->num_pfns);
+	} else {
+		release_pages_by_pfn(vb->pfns, vb->num_pfns);
+		tell_host(vb, vb->deflate_vq);
+	}
+}
+
+static void virtballoon_changed(struct virtio_device *vdev)
+{
+	struct virtio_balloon *vb = vdev->priv;
+
+	wake_up(&vb->config_change);
+}
+
+static inline int towards_target(struct virtio_balloon *vb)
+{
+	u32 v;
+	__virtio_config_val(vb->vdev,
+			    offsetof(struct virtio_balloon_config, num_pages),
+			    &v);
+	return v - vb->num_pages;
+}
+
+static void update_balloon_size(struct virtio_balloon *vb)
+{
+	__le32 actual = cpu_to_le32(vb->num_pages);
+
+	vb->vdev->config->set(vb->vdev,
+			      offsetof(struct virtio_balloon_config, actual),
+			      &actual, sizeof(actual));
+}
+
+static int balloon(void *_vballoon)
+{
+	struct virtio_balloon *vb = _vballoon;
+
+	set_freezable();
+	while (!kthread_should_stop()) {
+		int diff;
+
+		try_to_freeze();
+		wait_event_interruptible(vb->config_change,
+					 (diff = towards_target(vb)) != 0
+					 || kthread_should_stop());
+		if (diff > 0)
+			fill_balloon(vb, diff);
+		else if (diff < 0)
+			leak_balloon(vb, -diff);
+		update_balloon_size(vb);
+	}
+	return 0;
+}
+
+static int virtballoon_probe(struct virtio_device *vdev)
+{
+	struct virtio_balloon *vb;
+	int err;
+
+	vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
+	if (!vb) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	INIT_LIST_HEAD(&vb->pages);
+	vb->num_pages = 0;
+	init_waitqueue_head(&vb->config_change);
+	vb->vdev = vdev;
+
+	/* We expect two virtqueues. */
+	vb->inflate_vq = vdev->config->find_vq(vdev, 0, balloon_ack);
+	if (IS_ERR(vb->inflate_vq)) {
+		err = PTR_ERR(vb->inflate_vq);
+		goto out_free_vb;
+	}
+
+	vb->deflate_vq = vdev->config->find_vq(vdev, 1, balloon_ack);
+	if (IS_ERR(vb->deflate_vq)) {
+		err = PTR_ERR(vb->deflate_vq);
+		goto out_del_inflate_vq;
+	}
+
+	vb->thread = kthread_run(balloon, vb, "vballoon");
+	if (IS_ERR(vb->thread)) {
+		err = PTR_ERR(vb->thread);
+		goto out_del_deflate_vq;
+	}
+
+	vb->tell_host_first
+		= vdev->config->feature(vdev, VIRTIO_BALLOON_F_MUST_TELL_HOST);
+
+	return 0;
+
+out_del_deflate_vq:
+	vdev->config->del_vq(vb->deflate_vq);
+out_del_inflate_vq:
+	vdev->config->del_vq(vb->inflate_vq);
+out_free_vb:
+	kfree(vb);
+out:
+	return err;
+}
+
+static void virtballoon_remove(struct virtio_device *vdev)
+{
+	struct virtio_balloon *vb = vdev->priv;
+
+	kthread_stop(vb->thread);
+
+	/* There might be pages left in the balloon: free them. */
+	while (vb->num_pages)
+		leak_balloon(vb, vb->num_pages);
+
+	/* Now we reset the device so we can clean up the queues. */
+	vdev->config->reset(vdev);
+
+	vdev->config->del_vq(vb->deflate_vq);
+	vdev->config->del_vq(vb->inflate_vq);
+	kfree(vb);
+}
+
+static struct virtio_driver virtio_balloon = {
+	.driver.name =	KBUILD_MODNAME,
+	.driver.owner =	THIS_MODULE,
+	.id_table =	id_table,
+	.probe =	virtballoon_probe,
+	.remove =	__devexit_p(virtballoon_remove),
+	.config_changed = virtballoon_changed,
+};
+
+static int __init init(void)
+{
+	return register_virtio_driver(&virtio_balloon);
+}
+
+static void __exit fini(void)
+{
+	unregister_virtio_driver(&virtio_balloon);
+}
+module_init(init);
+module_exit(fini);
+
+MODULE_DEVICE_TABLE(virtio, id_table);
+MODULE_DESCRIPTION("Virtio balloon driver");
+MODULE_LICENSE("GPL");
diff -r e977a88ffefd include/linux/virtio_balloon.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/include/linux/virtio_balloon.h	Thu Jan 24 12:40:07 2008 +1100
@@ -0,0 +1,18 @@
+#ifndef _LINUX_VIRTIO_BALLOON_H
+#define _LINUX_VIRTIO_BALLOON_H
+#include <linux/virtio_config.h>
+
+/* The ID for virtio_balloon */
+#define VIRTIO_ID_BALLOON	5
+
+/* The feature bitmap for virtio balloon */
+#define VIRTIO_BALLOON_F_MUST_TELL_HOST	0 /* Tell before reclaiming pages */
+
+struct virtio_balloon_config
+{
+	/* Number of pages host wants Guest to give up. */
+	__le32 num_pages;
+	/* Number of pages we've actually got in balloon. */
+	__le32 actual;
+};
+#endif /* _LINUX_VIRTIO_BALLOON_H */

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/