[PATCH v2] Use ballooned pages for gntdev

[PATCH v2] Use ballooned pages for gntdev

[Daniel De Graaf]

Split up the balloon module, similar to how grants and events are
handled. This allow gntdev to use ballooned pages without adding a
dependency on the balloon module.

The interface is slightly different from that exposed in 2.6.32; the
page vector is allocated by the caller, not by the balloon driver. This
allows more freedom in how the returned pages are used, since they do
not all have to be returned to the balloon driver at the same time. It
also tries to reuse already ballooned pages rather than always
ballooning new pages to ensure they are in low memory as the 2.6.32
version did.

Changes since v1:
	* Rebased on konrad/devel/{balloon-hotplug,next-2.6.38} merge
	* Better function names
	* Adjust balloon stats for requested pages

[PATCH 1/3] xen-balloon: Move core balloon functionality out of module
[PATCH 2/3] xen-balloon: Add interface to retrieve ballooned pages
[PATCH 3/3] xen-gntdev: Use ballooned pages for grant mappings

[PATCH 1/3] xen-balloon: Move core balloon functionality out of module

[Daniel De Graaf]

The basic functionality of ballooning pages is useful for Xen drivers in
general. Rather than require a dependency on the balloon module, split
the functionality that is reused into the core. The balloon module is
still required to follow ballooning requests from xenstore or to view
balloon statistics in sysfs.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/Makefile      |    4 +-
 drivers/xen/balloon.c     |  229 +---------------------------------------
 drivers/xen/xen-balloon.c |  256 +++++++++++++++++++++++++++++++++++++++++++++
 include/xen/balloon.h     |   22 ++++
 4 files changed, 286 insertions(+), 225 deletions(-)
 create mode 100644 drivers/xen/xen-balloon.c
 create mode 100644 include/xen/balloon.h

diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index c1cb873..e56b02d 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -1,4 +1,4 @@
-obj-y	+= grant-table.o features.o events.o manage.o
+obj-y	+= grant-table.o features.o events.o manage.o balloon.o
 obj-y	+= xenbus/
 
 nostackp := $(call cc-option, -fno-stack-protector)
@@ -7,7 +7,7 @@ CFLAGS_features.o			:= $(nostackp)
 obj-$(CONFIG_BLOCK)		+= biomerge.o
 obj-$(CONFIG_HOTPLUG_CPU)	+= cpu_hotplug.o
 obj-$(CONFIG_XEN_XENCOMM)	+= xencomm.o
-obj-$(CONFIG_XEN_BALLOON)	+= balloon.o
+obj-$(CONFIG_XEN_BALLOON)	+= xen-balloon.o
 obj-$(CONFIG_XEN_DEV_EVTCHN)	+= xen-evtchn.o
 obj-$(CONFIG_XEN_PCIDEV_BACKEND)	+= pciback/
 obj-$(CONFIG_XEN_BLKDEV_BACKEND)	+= blkback/
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 12770ec..f2fa9ec 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -1,6 +1,4 @@
 /******************************************************************************
- * balloon.c
- *
  * Xen balloon driver - enables returning/claiming memory to/from Xen.
  *
  * Copyright (c) 2003, B Dragovic
@@ -33,7 +31,6 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/module.h>
 #include <linux/sched.h>
 #include <linux/errno.h>
 #include <linux/mm.h>
@@ -42,13 +39,11 @@
 #include <linux/highmem.h>
 #include <linux/mutex.h>
 #include <linux/list.h>
-#include <linux/sysdev.h>
 #include <linux/gfp.h>
 
 #include <asm/page.h>
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
-#include <asm/uaccess.h>
 #include <asm/tlb.h>
 #include <asm/e820.h>
 
@@ -58,14 +53,10 @@
 #include <xen/xen.h>
 #include <xen/interface/xen.h>
 #include <xen/interface/memory.h>
-#include <xen/xenbus.h>
+#include <xen/balloon.h>
 #include <xen/features.h>
 #include <xen/page.h>
 
-#define PAGES2KB(_p) ((_p)<<(PAGE_SHIFT-10))
-
-#define BALLOON_CLASS_NAME "xen_memory"
-
 /*
  * balloon_process() state:
  *
@@ -80,28 +71,11 @@ enum bp_state {
 	BP_ECANCELED
 };
 
-#define RETRY_UNLIMITED	0
-
-struct balloon_stats {
-	/* We aim for 'current allocation' == 'target allocation'. */
-	unsigned long current_pages;
-	unsigned long target_pages;
-	/* Number of pages in high- and low-memory balloons. */
-	unsigned long balloon_low;
-	unsigned long balloon_high;
-	unsigned long schedule_delay;
-	unsigned long max_schedule_delay;
-	unsigned long retry_count;
-	unsigned long max_retry_count;
-};
 
 static DEFINE_MUTEX(balloon_mutex);
 
-static struct sys_device balloon_sysdev;
-
-static int register_balloon(struct sys_device *sysdev);
-
-static struct balloon_stats balloon_stats;
+struct balloon_stats balloon_stats;
+EXPORT_SYMBOL_GPL(balloon_stats);
 
 /* We increase/decrease in batches which fit in a page */
 static unsigned long frame_list[PAGE_SIZE / sizeof(unsigned long)];
@@ -392,51 +366,13 @@ static void balloon_process(struct work_struct *work)
 }
 
 /* Resets the Xen limit, sets new target, and kicks off processing. */
-static void balloon_set_new_target(unsigned long target)
+void balloon_set_new_target(unsigned long target)
 {
 	/* No need for lock. Not read-modify-write updates. */
 	balloon_stats.target_pages = target;
 	schedule_delayed_work(&balloon_worker, 0);
 }
-
-static struct xenbus_watch target_watch =
-{
-	.node = "memory/target"
-};
-
-/* React to a change in the target key */
-static void watch_target(struct xenbus_watch *watch,
-			 const char **vec, unsigned int len)
-{
-	unsigned long long new_target;
-	int err;
-
-	err = xenbus_scanf(XBT_NIL, "memory", "target", "%llu", &new_target);
-	if (err != 1) {
-		/* This is ok (for domain0 at least) - so just return */
-		return;
-	}
-
-	/* The given memory/target value is in KiB, so it needs converting to
-	 * pages. PAGE_SHIFT converts bytes to pages, hence PAGE_SHIFT - 10.
-	 */
-	balloon_set_new_target(new_target >> (PAGE_SHIFT - 10));
-}
-
-static int balloon_init_watcher(struct notifier_block *notifier,
-				unsigned long event,
-				void *data)
-{
-	int err;
-
-	err = register_xenbus_watch(&target_watch);
-	if (err)
-		printk(KERN_ERR "Failed to set balloon watcher\n");
-
-	return NOTIFY_DONE;
-}
-
-static struct notifier_block xenstore_notifier;
+EXPORT_SYMBOL_GPL(balloon_set_new_target);
 
 static int __init balloon_init(void)
 {
@@ -446,7 +382,7 @@ static int __init balloon_init(void)
 	if (!xen_domain())
 		return -ENODEV;
 
-	pr_info("xen_balloon: Initialising balloon driver.\n");
+	pr_info("xen/balloon: Initialising balloon driver.\n");
 
  	if (xen_pv_domain())
  		nr_pages = xen_start_info->nr_pages;
@@ -462,8 +398,6 @@ static int __init balloon_init(void)
 	balloon_stats.retry_count = 1;
 	balloon_stats.max_retry_count = 16;
 
-	register_balloon(&balloon_sysdev);
-
 	/*
 	 * Initialise the balloon with excess memory space.  We need
 	 * to make sure we don't add memory which doesn't exist or
@@ -484,160 +418,9 @@ static int __init balloon_init(void)
 		__balloon_append(page);
 	}
 
-	target_watch.callback = watch_target;
-	xenstore_notifier.notifier_call = balloon_init_watcher;
-
-	register_xenstore_notifier(&xenstore_notifier);
-
 	return 0;
 }
 
 subsys_initcall(balloon_init);
 
-static void balloon_exit(void)
-{
-    /* XXX - release balloon here */
-    return;
-}
-
-module_exit(balloon_exit);
-
-#define BALLOON_SHOW(name, format, args...)				\
-	static ssize_t show_##name(struct sys_device *dev,		\
-				   struct sysdev_attribute *attr,	\
-				   char *buf)				\
-	{								\
-		return sprintf(buf, format, ##args);			\
-	}								\
-	static SYSDEV_ATTR(name, S_IRUGO, show_##name, NULL)
-
-BALLOON_SHOW(current_kb, "%lu\n", PAGES2KB(balloon_stats.current_pages));
-BALLOON_SHOW(low_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_low));
-BALLOON_SHOW(high_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_high));
-
-static SYSDEV_ULONG_ATTR(schedule_delay, 0444, balloon_stats.schedule_delay);
-static SYSDEV_ULONG_ATTR(max_schedule_delay, 0644, balloon_stats.max_schedule_delay);
-static SYSDEV_ULONG_ATTR(retry_count, 0444, balloon_stats.retry_count);
-static SYSDEV_ULONG_ATTR(max_retry_count, 0644, balloon_stats.max_retry_count);
-
-static ssize_t show_target_kb(struct sys_device *dev, struct sysdev_attribute *attr,
-			      char *buf)
-{
-	return sprintf(buf, "%lu\n", PAGES2KB(balloon_stats.target_pages));
-}
-
-static ssize_t store_target_kb(struct sys_device *dev,
-			       struct sysdev_attribute *attr,
-			       const char *buf,
-			       size_t count)
-{
-	char *endchar;
-	unsigned long long target_bytes;
-
-	if (!capable(CAP_SYS_ADMIN))
-		return -EPERM;
-
-	target_bytes = simple_strtoull(buf, &endchar, 0) * 1024;
-
-	balloon_set_new_target(target_bytes >> PAGE_SHIFT);
-
-	return count;
-}
-
-static SYSDEV_ATTR(target_kb, S_IRUGO | S_IWUSR,
-		   show_target_kb, store_target_kb);
-
-
-static ssize_t show_target(struct sys_device *dev, struct sysdev_attribute *attr,
-			      char *buf)
-{
-	return sprintf(buf, "%llu\n",
-		       (unsigned long long)balloon_stats.target_pages
-		       << PAGE_SHIFT);
-}
-
-static ssize_t store_target(struct sys_device *dev,
-			    struct sysdev_attribute *attr,
-			    const char *buf,
-			    size_t count)
-{
-	char *endchar;
-	unsigned long long target_bytes;
-
-	if (!capable(CAP_SYS_ADMIN))
-		return -EPERM;
-
-	target_bytes = memparse(buf, &endchar);
-
-	balloon_set_new_target(target_bytes >> PAGE_SHIFT);
-
-	return count;
-}
-
-static SYSDEV_ATTR(target, S_IRUGO | S_IWUSR,
-		   show_target, store_target);
-
-
-static struct sysdev_attribute *balloon_attrs[] = {
-	&attr_target_kb,
-	&attr_target,
-	&attr_schedule_delay.attr,
-	&attr_max_schedule_delay.attr,
-	&attr_retry_count.attr,
-	&attr_max_retry_count.attr
-};
-
-static struct attribute *balloon_info_attrs[] = {
-	&attr_current_kb.attr,
-	&attr_low_kb.attr,
-	&attr_high_kb.attr,
-	NULL
-};
-
-static struct attribute_group balloon_info_group = {
-	.name = "info",
-	.attrs = balloon_info_attrs
-};
-
-static struct sysdev_class balloon_sysdev_class = {
-	.name = BALLOON_CLASS_NAME
-};
-
-static int register_balloon(struct sys_device *sysdev)
-{
-	int i, error;
-
-	error = sysdev_class_register(&balloon_sysdev_class);
-	if (error)
-		return error;
-
-	sysdev->id = 0;
-	sysdev->cls = &balloon_sysdev_class;
-
-	error = sysdev_register(sysdev);
-	if (error) {
-		sysdev_class_unregister(&balloon_sysdev_class);
-		return error;
-	}
-
-	for (i = 0; i < ARRAY_SIZE(balloon_attrs); i++) {
-		error = sysdev_create_file(sysdev, balloon_attrs[i]);
-		if (error)
-			goto fail;
-	}
-
-	error = sysfs_create_group(&sysdev->kobj, &balloon_info_group);
-	if (error)
-		goto fail;
-
-	return 0;
-
- fail:
-	while (--i >= 0)
-		sysdev_remove_file(sysdev, balloon_attrs[i]);
-	sysdev_unregister(sysdev);
-	sysdev_class_unregister(&balloon_sysdev_class);
-	return error;
-}
-
 MODULE_LICENSE("GPL");
diff --git a/drivers/xen/xen-balloon.c b/drivers/xen/xen-balloon.c
new file mode 100644
index 0000000..a4ff225
--- /dev/null
+++ b/drivers/xen/xen-balloon.c
@@ -0,0 +1,256 @@
+/******************************************************************************
+ * Xen balloon driver - enables returning/claiming memory to/from Xen.
+ *
+ * Copyright (c) 2003, B Dragovic
+ * Copyright (c) 2003-2004, M Williamson, K Fraser
+ * Copyright (c) 2005 Dan M. Smith, IBM Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sysdev.h>
+#include <linux/capability.h>
+
+#include <xen/xen.h>
+#include <xen/interface/xen.h>
+#include <xen/balloon.h>
+#include <xen/xenbus.h>
+#include <xen/features.h>
+#include <xen/page.h>
+
+#define PAGES2KB(_p) ((_p)<<(PAGE_SHIFT-10))
+
+#define BALLOON_CLASS_NAME "xen_memory"
+
+static struct sys_device balloon_sysdev;
+
+static int register_balloon(struct sys_device *sysdev);
+
+static struct xenbus_watch target_watch =
+{
+	.node = "memory/target"
+};
+
+/* React to a change in the target key */
+static void watch_target(struct xenbus_watch *watch,
+			 const char **vec, unsigned int len)
+{
+	unsigned long long new_target;
+	int err;
+
+	err = xenbus_scanf(XBT_NIL, "memory", "target", "%llu", &new_target);
+	if (err != 1) {
+		/* This is ok (for domain0 at least) - so just return */
+		return;
+	}
+
+	/* The given memory/target value is in KiB, so it needs converting to
+	 * pages. PAGE_SHIFT converts bytes to pages, hence PAGE_SHIFT - 10.
+	 */
+	balloon_set_new_target(new_target >> (PAGE_SHIFT - 10));
+}
+
+static int balloon_init_watcher(struct notifier_block *notifier,
+				unsigned long event,
+				void *data)
+{
+	int err;
+
+	err = register_xenbus_watch(&target_watch);
+	if (err)
+		printk(KERN_ERR "Failed to set balloon watcher\n");
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block xenstore_notifier;
+
+static int __init balloon_init(void)
+{
+	if (!xen_domain())
+		return -ENODEV;
+
+	pr_info("xen-balloon: Initialising balloon driver.\n");
+
+	register_balloon(&balloon_sysdev);
+
+	target_watch.callback = watch_target;
+	xenstore_notifier.notifier_call = balloon_init_watcher;
+
+	register_xenstore_notifier(&xenstore_notifier);
+
+	return 0;
+}
+subsys_initcall(balloon_init);
+
+static void balloon_exit(void)
+{
+    /* XXX - release balloon here */
+    return;
+}
+
+module_exit(balloon_exit);
+
+#define BALLOON_SHOW(name, format, args...)				\
+	static ssize_t show_##name(struct sys_device *dev,		\
+				   struct sysdev_attribute *attr,	\
+				   char *buf)				\
+	{								\
+		return sprintf(buf, format, ##args);			\
+	}								\
+	static SYSDEV_ATTR(name, S_IRUGO, show_##name, NULL)
+
+BALLOON_SHOW(current_kb, "%lu\n", PAGES2KB(balloon_stats.current_pages));
+BALLOON_SHOW(low_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_low));
+BALLOON_SHOW(high_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_high));
+
+static SYSDEV_ULONG_ATTR(schedule_delay, 0444, balloon_stats.schedule_delay);
+static SYSDEV_ULONG_ATTR(max_schedule_delay, 0644, balloon_stats.max_schedule_delay);
+static SYSDEV_ULONG_ATTR(retry_count, 0444, balloon_stats.retry_count);
+static SYSDEV_ULONG_ATTR(max_retry_count, 0644, balloon_stats.max_retry_count);
+
+static ssize_t show_target_kb(struct sys_device *dev, struct sysdev_attribute *attr,
+			      char *buf)
+{
+	return sprintf(buf, "%lu\n", PAGES2KB(balloon_stats.target_pages));
+}
+
+static ssize_t store_target_kb(struct sys_device *dev,
+			       struct sysdev_attribute *attr,
+			       const char *buf,
+			       size_t count)
+{
+	char *endchar;
+	unsigned long long target_bytes;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	target_bytes = simple_strtoull(buf, &endchar, 0) * 1024;
+
+	balloon_set_new_target(target_bytes >> PAGE_SHIFT);
+
+	return count;
+}
+
+static SYSDEV_ATTR(target_kb, S_IRUGO | S_IWUSR,
+		   show_target_kb, store_target_kb);
+
+
+static ssize_t show_target(struct sys_device *dev, struct sysdev_attribute *attr,
+			      char *buf)
+{
+	return sprintf(buf, "%llu\n",
+		       (unsigned long long)balloon_stats.target_pages
+		       << PAGE_SHIFT);
+}
+
+static ssize_t store_target(struct sys_device *dev,
+			    struct sysdev_attribute *attr,
+			    const char *buf,
+			    size_t count)
+{
+	char *endchar;
+	unsigned long long target_bytes;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	target_bytes = memparse(buf, &endchar);
+
+	balloon_set_new_target(target_bytes >> PAGE_SHIFT);
+
+	return count;
+}
+
+static SYSDEV_ATTR(target, S_IRUGO | S_IWUSR,
+		   show_target, store_target);
+
+
+static struct sysdev_attribute *balloon_attrs[] = {
+	&attr_target_kb,
+	&attr_target,
+	&attr_schedule_delay.attr,
+	&attr_max_schedule_delay.attr,
+	&attr_retry_count.attr,
+	&attr_max_retry_count.attr
+};
+
+static struct attribute *balloon_info_attrs[] = {
+	&attr_current_kb.attr,
+	&attr_low_kb.attr,
+	&attr_high_kb.attr,
+	NULL
+};
+
+static struct attribute_group balloon_info_group = {
+	.name = "info",
+	.attrs = balloon_info_attrs
+};
+
+static struct sysdev_class balloon_sysdev_class = {
+	.name = BALLOON_CLASS_NAME
+};
+
+static int register_balloon(struct sys_device *sysdev)
+{
+	int i, error;
+
+	error = sysdev_class_register(&balloon_sysdev_class);
+	if (error)
+		return error;
+
+	sysdev->id = 0;
+	sysdev->cls = &balloon_sysdev_class;
+
+	error = sysdev_register(sysdev);
+	if (error) {
+		sysdev_class_unregister(&balloon_sysdev_class);
+		return error;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(balloon_attrs); i++) {
+		error = sysdev_create_file(sysdev, balloon_attrs[i]);
+		if (error)
+			goto fail;
+	}
+
+	error = sysfs_create_group(&sysdev->kobj, &balloon_info_group);
+	if (error)
+		goto fail;
+
+	return 0;
+
+ fail:
+	while (--i >= 0)
+		sysdev_remove_file(sysdev, balloon_attrs[i]);
+	sysdev_unregister(sysdev);
+	sysdev_class_unregister(&balloon_sysdev_class);
+	return error;
+}
+
+MODULE_LICENSE("GPL");
diff --git a/include/xen/balloon.h b/include/xen/balloon.h
new file mode 100644
index 0000000..f72e479
--- /dev/null
+++ b/include/xen/balloon.h
@@ -0,0 +1,22 @@
+/******************************************************************************
+ * Xen balloon functionality
+ */
+
+#define RETRY_UNLIMITED	0
+
+struct balloon_stats {
+	/* We aim for 'current allocation' == 'target allocation'. */
+	unsigned long current_pages;
+	unsigned long target_pages;
+	/* Number of pages in high- and low-memory balloons. */
+	unsigned long balloon_low;
+	unsigned long balloon_high;
+	unsigned long schedule_delay;
+	unsigned long max_schedule_delay;
+	unsigned long retry_count;
+	unsigned long max_retry_count;
+};
+
+extern struct balloon_stats balloon_stats;
+
+void balloon_set_new_target(unsigned long target);
-- 
1.7.3.4

[PATCH 2/3] xen-balloon: Add interface to retrieve ballooned pages

[Daniel De Graaf]

Pages that have been ballooned are useful for other Xen drivers doing
grant table actions, because these pages have valid struct page/PFNs but
have no valid MFN so are available for remapping.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/balloon.c |   73 +++++++++++++++++++++++++++++++++++++++++++++----
 include/xen/balloon.h |    3 ++
 2 files changed, 70 insertions(+), 6 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index f2fa9ec..9c35cec 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -128,14 +128,17 @@ static void balloon_append(struct page *page)
 }
 
 /* balloon_retrieve: rescue a page from the balloon, if it is not empty. */
-static struct page *balloon_retrieve(void)
+static struct page *balloon_retrieve(bool prefer_highmem)
 {
 	struct page *page;
 
 	if (list_empty(&ballooned_pages))
 		return NULL;
 
-	page = list_entry(ballooned_pages.next, struct page, lru);
+	if (prefer_highmem)
+		page = list_entry(ballooned_pages.prev, struct page, lru);
+	else
+		page = list_entry(ballooned_pages.next, struct page, lru);
 	list_del(&page->lru);
 
 	if (PageHighMem(page)) {
@@ -240,7 +243,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
 	}
 
 	for (i = 0; i < rc; i++) {
-		page = balloon_retrieve();
+		page = balloon_retrieve(false);
 		BUG_ON(page == NULL);
 
 		pfn = page_to_pfn(page);
@@ -270,7 +273,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
 	return BP_DONE;
 }
 
-static enum bp_state decrease_reservation(unsigned long nr_pages)
+static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 {
 	enum bp_state state = BP_DONE;
 	unsigned long  pfn, i;
@@ -286,7 +289,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages)
 		nr_pages = ARRAY_SIZE(frame_list);
 
 	for (i = 0; i < nr_pages; i++) {
-		if ((page = alloc_page(GFP_BALLOON)) == NULL) {
+		if ((page = alloc_page(gfp)) == NULL) {
 			pr_info("xen_balloon: %s: Cannot allocate memory\n", __func__);
 			nr_pages = i;
 			state = BP_EAGAIN;
@@ -348,7 +351,7 @@ static void balloon_process(struct work_struct *work)
 			state = increase_reservation(credit);
 
 		if (credit < 0)
-			state = decrease_reservation(-credit);
+			state = decrease_reservation(-credit, GFP_BALLOON);
 
 		state = update_schedule(state);
 
@@ -374,6 +377,64 @@ void balloon_set_new_target(unsigned long target)
 }
 EXPORT_SYMBOL_GPL(balloon_set_new_target);
 
+/**
+ * alloc_xenballooned_pages - get pages that have been ballooned out
+ * @nr_pages: Number of pages to get
+ * @pages: pages returned
+ * @return 0 on success, error otherwise
+ */
+int alloc_xenballooned_pages(int nr_pages, struct page** pages)
+{
+	int pgno = 0;
+	struct page* page;
+	mutex_lock(&balloon_mutex);
+	while (pgno < nr_pages) {
+		page = balloon_retrieve(true);
+		if (page) {
+			pages[pgno++] = page;
+		} else {
+			enum bp_state st;
+			st = decrease_reservation(nr_pages - pgno, GFP_HIGHUSER);
+			if (st != BP_DONE)
+				goto out_undo;
+		}
+	}
+	mutex_unlock(&balloon_mutex);
+	return 0;
+ out_undo:
+	while (pgno)
+		balloon_append(pages[--pgno]);
+	/* Free the memory back to the kernel soon */
+	schedule_delayed_work(&balloon_worker, 0);
+	mutex_unlock(&balloon_mutex);
+	return -ENOMEM;
+}
+EXPORT_SYMBOL(alloc_xenballooned_pages);
+
+/**
+ * free_xenballooned_pages - return pages retrieved with get_ballooned_pages
+ * @nr_pages: Number of pages
+ * @pages: pages to return
+ */
+void free_xenballooned_pages(int nr_pages, struct page** pages)
+{
+	int i;
+
+	mutex_lock(&balloon_mutex);
+
+	for (i = 0; i < nr_pages; i++) {
+		if (pages[i])
+			balloon_append(pages[i]);
+	}
+
+	/* The balloon may be too large now. Shrink it if needed. */
+	if (current_target() != balloon_stats.current_pages)
+		schedule_delayed_work(&balloon_worker, 0);
+
+	mutex_unlock(&balloon_mutex);
+}
+EXPORT_SYMBOL(free_xenballooned_pages);
+
 static int __init balloon_init(void)
 {
  	unsigned long pfn, nr_pages, extra_pfn_end;
diff --git a/include/xen/balloon.h b/include/xen/balloon.h
index f72e479..a2b22f0 100644
--- a/include/xen/balloon.h
+++ b/include/xen/balloon.h
@@ -20,3 +20,6 @@ struct balloon_stats {
 extern struct balloon_stats balloon_stats;
 
 void balloon_set_new_target(unsigned long target);
+
+int alloc_xenballooned_pages(int nr_pages, struct page** pages);
+void free_xenballooned_pages(int nr_pages, struct page** pages);
-- 
1.7.3.4

[PATCH 3/3] xen-gntdev: Use ballooned pages for grant mappings

[Daniel De Graaf]

Grant mappings cause the PFN<->MFN mapping to be lost on the pages used
for the mapping. Instead of leaking memory, use pages that have already
been ballooned out and so have no valid mapping. This removes the need
for the bad-page leak workaround as pages are repopulated by the balloon
driver.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/gntdev.c |   38 +++++---------------------------------
 1 files changed, 5 insertions(+), 33 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index d43ff30..a6de673 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -36,6 +36,7 @@
 
 #include <xen/xen.h>
 #include <xen/grant_table.h>
+#include <xen/balloon.h>
 #include <xen/gntdev.h>
 #include <xen/events.h>
 #include <asm/xen/hypervisor.h>
@@ -122,10 +123,10 @@ static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count)
 	    NULL == add->pages)
 		goto err;
 
+	if (alloc_xenballooned_pages(count, add->pages))
+		goto err;
+
 	for (i = 0; i < count; i++) {
-		add->pages[i] = alloc_page(GFP_KERNEL | __GFP_HIGHMEM);
-		if (add->pages[i] == NULL)
-			goto err;
 		add->map_ops[i].handle = -1;
 		add->unmap_ops[i].handle = -1;
 	}
@@ -137,11 +138,6 @@ static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count)
 	return add;
 
 err:
-	if (add->pages)
-		for (i = 0; i < count; i++) {
-			if (add->pages[i])
-				__free_page(add->pages[i]);
-		}
 	kfree(add->pages);
 	kfree(add->grants);
 	kfree(add->map_ops);
@@ -184,8 +180,6 @@ static struct grant_map *gntdev_find_map_index(struct gntdev_priv *priv,
 
 static void gntdev_put_map(struct grant_map *map)
 {
-	int i;
-
 	if (!map)
 		return;
 
@@ -202,29 +196,7 @@ static void gntdev_put_map(struct grant_map *map)
 		if (!use_ptemod)
 			unmap_grant_pages(map, 0, map->count);
 
-		for (i = 0; i < map->count; i++) {
-			uint32_t check, *tmp;
-			if (!map->pages[i])
-				continue;
-			/* XXX When unmapping in an HVM domain, Xen will
-			 * sometimes end up mapping the GFN to an invalid MFN.
-			 * In this case, writes will be discarded and reads will
-			 * return all 0xFF bytes.  Leak these unusable GFNs
-			 * until Xen supports fixing their p2m mapping.
-			 *
-			 * Confirmed present in Xen 4.1-RC3 with HVM source
-			 */
-			tmp = kmap(map->pages[i]);
-			*tmp = 0xdeaddead;
-			mb();
-			check = *tmp;
-			kunmap(map->pages[i]);
-			if (check == 0xdeaddead)
-				__free_page(map->pages[i]);
-			else
-				pr_debug("Discard page %d=%ld\n", i,
-					page_to_pfn(map->pages[i]));
-		}
+		free_xenballooned_pages(map->count, map->pages);
 	}
 	kfree(map->pages);
 	kfree(map->grants);
-- 
1.7.3.4

Re: [PATCH v2] Use ballooned pages for gntdev

[Ian Campbell]

On Wed, 2011-03-09 at 23:07 +0000, Daniel De Graaf wrote:
> Split up the balloon module, similar to how grants and events are
> handled. This allow gntdev to use ballooned pages without adding a
> dependency on the balloon module.
> 
> The interface is slightly different from that exposed in 2.6.32; the
> page vector is allocated by the caller, not by the balloon driver. This
> allows more freedom in how the returned pages are used, since they do
> not all have to be returned to the balloon driver at the same time. It
> also tries to reuse already ballooned pages rather than always
> ballooning new pages to ensure they are in low memory as the 2.6.32
> version did.
> 
> Changes since v1:
> 	* Rebased on konrad/devel/{balloon-hotplug,next-2.6.38} merge
> 	* Better function names
> 	* Adjust balloon stats for requested pages
> 
> [PATCH 1/3] xen-balloon: Move core balloon functionality out of module
> [PATCH 2/3] xen-balloon: Add interface to retrieve ballooned pages
> [PATCH 3/3] xen-gntdev: Use ballooned pages for grant mappings

Does this introduces a new potential failure case? When a normal balloon
operation takes place is it is now possible for the ballooned_pages list
to be empty (because gntdev has stolen stuff from it) where before we
knew the list was non-empty at that point?

e.g. increase_reservation includes:
		page = balloon_retrieve();
		BUG_ON(page == NULL);
as well as:
	page = balloon_first_page();
	for (i = 0; i < nr_pages; i++) {
		BUG_ON(page == NULL);
		frame_list[i] = page_to_pfn(page);
		page = balloon_next_page(page);
	}

Are we sure we aren't about to exercise those BUG_ONs?

As a secondary concern, assuming we don't crash, does the balloon
process correctly sleep until explicitly kicked when ballooned_pages
becomes empty? Or does it keep needlessly waking up on the jiffy timer?
IOW does credit in balloon_process become 0 when this situation occurs?

Or does adjusting the balloon stats ensure this all works out alright?

An interesting experiment would probably be to allocate a bunch of
address space via gntdev and then manually balloon the guest above it's
current allocation, back below the lower limit due to the gntdev
allocation, back up to starting point etc etc.

Ian.

Re: [PATCH v2] Use ballooned pages for gntdev

[Daniel De Graaf]

On 03/10/2011 04:18 AM, Ian Campbell wrote:
> On Wed, 2011-03-09 at 23:07 +0000, Daniel De Graaf wrote:
>> Split up the balloon module, similar to how grants and events are
>> handled. This allow gntdev to use ballooned pages without adding a
>> dependency on the balloon module.
>>
>> The interface is slightly different from that exposed in 2.6.32; the
>> page vector is allocated by the caller, not by the balloon driver. This
>> allows more freedom in how the returned pages are used, since they do
>> not all have to be returned to the balloon driver at the same time. It
>> also tries to reuse already ballooned pages rather than always
>> ballooning new pages to ensure they are in low memory as the 2.6.32
>> version did.
>>
>> Changes since v1:
>> 	* Rebased on konrad/devel/{balloon-hotplug,next-2.6.38} merge
>> 	* Better function names
>> 	* Adjust balloon stats for requested pages
>>
>> [PATCH 1/3] xen-balloon: Move core balloon functionality out of module
>> [PATCH 2/3] xen-balloon: Add interface to retrieve ballooned pages
>> [PATCH 3/3] xen-gntdev: Use ballooned pages for grant mappings
> 
> Does this introduces a new potential failure case? When a normal balloon
> operation takes place is it is now possible for the ballooned_pages list
> to be empty (because gntdev has stolen stuff from it) where before we
> knew the list was non-empty at that point?
> 
> e.g. increase_reservation includes:
> 		page = balloon_retrieve();
> 		BUG_ON(page == NULL);
> as well as:
> 	page = balloon_first_page();
> 	for (i = 0; i < nr_pages; i++) {
> 		BUG_ON(page == NULL);
> 		frame_list[i] = page_to_pfn(page);
> 		page = balloon_next_page(page);
> 	}
> 
> Are we sure we aren't about to exercise those BUG_ONs?

This is safe because increase_reservation is protected by the balloon
mutex. There is a loop just before the hypercall that verifies that
there are enough pages in the list.

> As a secondary concern, assuming we don't crash, does the balloon
> process correctly sleep until explicitly kicked when ballooned_pages
> becomes empty? Or does it keep needlessly waking up on the jiffy timer?
> IOW does credit in balloon_process become 0 when this situation occurs?
> 
> Or does adjusting the balloon stats ensure this all works out alright?

Adjusting the statistics should make this work out properly. Since only
pages actually in the list will be counted in balloon_low+balloon_high,
which is the lower bound for credit, we will always be able to satisfy
the reservation changes requested by balloon_process. The jiffy timer is
used only when Linux or Xen cannot satisfy the memory allocations required
to adjust the balloon size.

> An interesting experiment would probably be to allocate a bunch of
> address space via gntdev and then manually balloon the guest above it's
> current allocation, back below the lower limit due to the gntdev
> allocation, back up to starting point etc etc.
> 
> Ian.
> 

That does sound like a useful test case.

-- 
Daniel De Graaf
National Security Agency

Re: [PATCH v2] Use ballooned pages for gntdev

[Ian Campbell]

On Thu, 2011-03-10 at 14:56 +0000, Daniel De Graaf wrote:
> On 03/10/2011 04:18 AM, Ian Campbell wrote:
> > On Wed, 2011-03-09 at 23:07 +0000, Daniel De Graaf wrote:
> >> Split up the balloon module, similar to how grants and events are
> >> handled. This allow gntdev to use ballooned pages without adding a
> >> dependency on the balloon module.
> >>
> >> The interface is slightly different from that exposed in 2.6.32; the
> >> page vector is allocated by the caller, not by the balloon driver. This
> >> allows more freedom in how the returned pages are used, since they do
> >> not all have to be returned to the balloon driver at the same time. It
> >> also tries to reuse already ballooned pages rather than always
> >> ballooning new pages to ensure they are in low memory as the 2.6.32
> >> version did.
> >>
> >> Changes since v1:
> >> 	* Rebased on konrad/devel/{balloon-hotplug,next-2.6.38} merge
> >> 	* Better function names
> >> 	* Adjust balloon stats for requested pages
> >>
> >> [PATCH 1/3] xen-balloon: Move core balloon functionality out of module
> >> [PATCH 2/3] xen-balloon: Add interface to retrieve ballooned pages
> >> [PATCH 3/3] xen-gntdev: Use ballooned pages for grant mappings
> > 
> > Does this introduces a new potential failure case? When a normal balloon
> > operation takes place is it is now possible for the ballooned_pages list
> > to be empty (because gntdev has stolen stuff from it) where before we
> > knew the list was non-empty at that point?
> > 
> > e.g. increase_reservation includes:
> > 		page = balloon_retrieve();
> > 		BUG_ON(page == NULL);
> > as well as:
> > 	page = balloon_first_page();
> > 	for (i = 0; i < nr_pages; i++) {
> > 		BUG_ON(page == NULL);
> > 		frame_list[i] = page_to_pfn(page);
> > 		page = balloon_next_page(page);
> > 	}
> > 
> > Are we sure we aren't about to exercise those BUG_ONs?
> 
> This is safe because increase_reservation is protected by the balloon
> mutex. There is a loop just before the hypercall that verifies that
> there are enough pages in the list.

Which loop? I don't see anything like that in either
increase_reservation or the caller (balloon process).

> > As a secondary concern, assuming we don't crash, does the balloon
> > process correctly sleep until explicitly kicked when ballooned_pages
> > becomes empty? Or does it keep needlessly waking up on the jiffy timer?
> > IOW does credit in balloon_process become 0 when this situation occurs?
> > 
> > Or does adjusting the balloon stats ensure this all works out alright?
> 
> Adjusting the statistics should make this work out properly. Since only
> pages actually in the list will be counted in balloon_low+balloon_high,
> which is the lower bound for credit, we will always be able to satisfy
> the reservation changes requested by balloon_process. The jiffy timer is
> used only when Linux or Xen cannot satisfy the memory allocations required
> to adjust the balloon size.

OK.

> 
> > An interesting experiment would probably be to allocate a bunch of
> > address space via gntdev and then manually balloon the guest above it's
> > current allocation, back below the lower limit due to the gntdev
> > allocation, back up to starting point etc etc.
> > 
> > Ian.
> > 
> 
> That does sound like a useful test case.
> 

Re: [PATCH v2] Use ballooned pages for gntdev

[Daniel De Graaf]

On 03/10/2011 10:57 AM, Ian Campbell wrote:
> On Thu, 2011-03-10 at 14:56 +0000, Daniel De Graaf wrote:
>> On 03/10/2011 04:18 AM, Ian Campbell wrote:
>>> On Wed, 2011-03-09 at 23:07 +0000, Daniel De Graaf wrote:
>>>> Split up the balloon module, similar to how grants and events are
>>>> handled. This allow gntdev to use ballooned pages without adding a
>>>> dependency on the balloon module.
>>>>
>>>> The interface is slightly different from that exposed in 2.6.32; the
>>>> page vector is allocated by the caller, not by the balloon driver. This
>>>> allows more freedom in how the returned pages are used, since they do
>>>> not all have to be returned to the balloon driver at the same time. It
>>>> also tries to reuse already ballooned pages rather than always
>>>> ballooning new pages to ensure they are in low memory as the 2.6.32
>>>> version did.
>>>>
>>>> Changes since v1:
>>>> 	* Rebased on konrad/devel/{balloon-hotplug,next-2.6.38} merge
>>>> 	* Better function names
>>>> 	* Adjust balloon stats for requested pages
>>>>
>>>> [PATCH 1/3] xen-balloon: Move core balloon functionality out of module
>>>> [PATCH 2/3] xen-balloon: Add interface to retrieve ballooned pages
>>>> [PATCH 3/3] xen-gntdev: Use ballooned pages for grant mappings
>>>
>>> Does this introduces a new potential failure case? When a normal balloon
>>> operation takes place is it is now possible for the ballooned_pages list
>>> to be empty (because gntdev has stolen stuff from it) where before we
>>> knew the list was non-empty at that point?
>>>
>>> e.g. increase_reservation includes:
>>> 		page = balloon_retrieve();
>>> 		BUG_ON(page == NULL);
>>> as well as:
>>> 	page = balloon_first_page();
>>> 	for (i = 0; i < nr_pages; i++) {
>>> 		BUG_ON(page == NULL);
>>> 		frame_list[i] = page_to_pfn(page);
>>> 		page = balloon_next_page(page);
>>> 	}
>>>
>>> Are we sure we aren't about to exercise those BUG_ONs?
>>
>> This is safe because increase_reservation is protected by the balloon
>> mutex. There is a loop just before the hypercall that verifies that
>> there are enough pages in the list.
> 
> Which loop? I don't see anything like that in either
> increase_reservation or the caller (balloon process).
> 

In increase_reservation:
        page = balloon_first_page();
        for (i = 0; i < nr_pages; i++) {
                if (!page) {
                        nr_pages = i;
                        break;
                }
                frame_list[i] = page_to_pfn(page);
                page = balloon_next_page(page);
        }

This ensures that the balloon size is at least nr_pages.

>>> As a secondary concern, assuming we don't crash, does the balloon
>>> process correctly sleep until explicitly kicked when ballooned_pages
>>> becomes empty? Or does it keep needlessly waking up on the jiffy timer?
>>> IOW does credit in balloon_process become 0 when this situation occurs?
>>>
>>> Or does adjusting the balloon stats ensure this all works out alright?
>>
>> Adjusting the statistics should make this work out properly. Since only
>> pages actually in the list will be counted in balloon_low+balloon_high,
>> which is the lower bound for credit, we will always be able to satisfy
>> the reservation changes requested by balloon_process. The jiffy timer is
>> used only when Linux or Xen cannot satisfy the memory allocations required
>> to adjust the balloon size.
> 
> OK.
> 
>>
>>> An interesting experiment would probably be to allocate a bunch of
>>> address space via gntdev and then manually balloon the guest above it's
>>> current allocation, back below the lower limit due to the gntdev
>>> allocation, back up to starting point etc etc.
>>>
>>> Ian.
>>>
>>
>> That does sound like a useful test case.
>>
> 
> 

Re: [PATCH v2] Use ballooned pages for gntdev

[Ian Campbell]

On Thu, 2011-03-10 at 16:10 +0000, Daniel De Graaf wrote:
> On 03/10/2011 10:57 AM, Ian Campbell wrote:
> > On Thu, 2011-03-10 at 14:56 +0000, Daniel De Graaf wrote:
> >> On 03/10/2011 04:18 AM, Ian Campbell wrote:
> >>> On Wed, 2011-03-09 at 23:07 +0000, Daniel De Graaf wrote:
> >>>> Split up the balloon module, similar to how grants and events are
> >>>> handled. This allow gntdev to use ballooned pages without adding a
> >>>> dependency on the balloon module.
> >>>>
> >>>> The interface is slightly different from that exposed in 2.6.32; the
> >>>> page vector is allocated by the caller, not by the balloon driver. This
> >>>> allows more freedom in how the returned pages are used, since they do
> >>>> not all have to be returned to the balloon driver at the same time. It
> >>>> also tries to reuse already ballooned pages rather than always
> >>>> ballooning new pages to ensure they are in low memory as the 2.6.32
> >>>> version did.
> >>>>
> >>>> Changes since v1:
> >>>> 	* Rebased on konrad/devel/{balloon-hotplug,next-2.6.38} merge
> >>>> 	* Better function names
> >>>> 	* Adjust balloon stats for requested pages
> >>>>
> >>>> [PATCH 1/3] xen-balloon: Move core balloon functionality out of module
> >>>> [PATCH 2/3] xen-balloon: Add interface to retrieve ballooned pages
> >>>> [PATCH 3/3] xen-gntdev: Use ballooned pages for grant mappings
> >>>
> >>> Does this introduces a new potential failure case? When a normal balloon
> >>> operation takes place is it is now possible for the ballooned_pages list
> >>> to be empty (because gntdev has stolen stuff from it) where before we
> >>> knew the list was non-empty at that point?
> >>>
> >>> e.g. increase_reservation includes:
> >>> 		page = balloon_retrieve();
> >>> 		BUG_ON(page == NULL);
> >>> as well as:
> >>> 	page = balloon_first_page();
> >>> 	for (i = 0; i < nr_pages; i++) {
> >>> 		BUG_ON(page == NULL);
> >>> 		frame_list[i] = page_to_pfn(page);
> >>> 		page = balloon_next_page(page);
> >>> 	}
> >>>
> >>> Are we sure we aren't about to exercise those BUG_ONs?
> >>
> >> This is safe because increase_reservation is protected by the balloon
> >> mutex. There is a loop just before the hypercall that verifies that
> >> there are enough pages in the list.
> > 
> > Which loop? I don't see anything like that in either
> > increase_reservation or the caller (balloon process).
> > 
> 
> In increase_reservation:
>         page = balloon_first_page();
>         for (i = 0; i < nr_pages; i++) {
>                 if (!page) {
>                         nr_pages = i;
>                         break;
>                 }
>                 frame_list[i] = page_to_pfn(page);
>                 page = balloon_next_page(page);
>         }
> 
> This ensures that the balloon size is at least nr_pages.

I must be missing a patch, on my version this is exactly the loop with
the BUG_ON in it that I was referring to:
	page = balloon_first_page();
	for (i = 0; i < nr_pages; i++) {
		BUG_ON(page == NULL);
		frame_list[i] = page_to_pfn(page);
		page = balloon_next_page(page);
	}

Ian

Re: [PATCH v2] Use ballooned pages for gntdev

[Daniel De Graaf]

On 03/10/2011 11:22 AM, Ian Campbell wrote:
> On Thu, 2011-03-10 at 16:10 +0000, Daniel De Graaf wrote:
>> On 03/10/2011 10:57 AM, Ian Campbell wrote:
>>> On Thu, 2011-03-10 at 14:56 +0000, Daniel De Graaf wrote:
>>>> On 03/10/2011 04:18 AM, Ian Campbell wrote:
>>>>> On Wed, 2011-03-09 at 23:07 +0000, Daniel De Graaf wrote:
>>>>>> Split up the balloon module, similar to how grants and events are
>>>>>> handled. This allow gntdev to use ballooned pages without adding a
>>>>>> dependency on the balloon module.
>>>>>>
>>>>>> The interface is slightly different from that exposed in 2.6.32; the
>>>>>> page vector is allocated by the caller, not by the balloon driver. This
>>>>>> allows more freedom in how the returned pages are used, since they do
>>>>>> not all have to be returned to the balloon driver at the same time. It
>>>>>> also tries to reuse already ballooned pages rather than always
>>>>>> ballooning new pages to ensure they are in low memory as the 2.6.32
>>>>>> version did.
>>>>>>
>>>>>> Changes since v1:
>>>>>> 	* Rebased on konrad/devel/{balloon-hotplug,next-2.6.38} merge
>>>>>> 	* Better function names
>>>>>> 	* Adjust balloon stats for requested pages
>>>>>>
>>>>>> [PATCH 1/3] xen-balloon: Move core balloon functionality out of module
>>>>>> [PATCH 2/3] xen-balloon: Add interface to retrieve ballooned pages
>>>>>> [PATCH 3/3] xen-gntdev: Use ballooned pages for grant mappings
>>>>>
>>>>> Does this introduces a new potential failure case? When a normal balloon
>>>>> operation takes place is it is now possible for the ballooned_pages list
>>>>> to be empty (because gntdev has stolen stuff from it) where before we
>>>>> knew the list was non-empty at that point?
>>>>>
>>>>> e.g. increase_reservation includes:
>>>>> 		page = balloon_retrieve();
>>>>> 		BUG_ON(page == NULL);
>>>>> as well as:
>>>>> 	page = balloon_first_page();
>>>>> 	for (i = 0; i < nr_pages; i++) {
>>>>> 		BUG_ON(page == NULL);
>>>>> 		frame_list[i] = page_to_pfn(page);
>>>>> 		page = balloon_next_page(page);
>>>>> 	}
>>>>>
>>>>> Are we sure we aren't about to exercise those BUG_ONs?
>>>>
>>>> This is safe because increase_reservation is protected by the balloon
>>>> mutex. There is a loop just before the hypercall that verifies that
>>>> there are enough pages in the list.
>>>
>>> Which loop? I don't see anything like that in either
>>> increase_reservation or the caller (balloon process).
>>>
>>
>> In increase_reservation:
>>         page = balloon_first_page();
>>         for (i = 0; i < nr_pages; i++) {
>>                 if (!page) {
>>                         nr_pages = i;
>>                         break;
>>                 }
>>                 frame_list[i] = page_to_pfn(page);
>>                 page = balloon_next_page(page);
>>         }
>>
>> This ensures that the balloon size is at least nr_pages.
> 
> I must be missing a patch, on my version this is exactly the loop with
> the BUG_ON in it that I was referring to:
> 	page = balloon_first_page();
> 	for (i = 0; i < nr_pages; i++) {
> 		BUG_ON(page == NULL);
> 		frame_list[i] = page_to_pfn(page);
> 		page = balloon_next_page(page);
> 	}
> 
> Ian
> 

Either way, increase_reservation is only called from balloon_process, with
nr_pages = current_target() - balloon_stats.current_pages, which is at
most balloon_stats.balloon_low + balloon_stats.balloon_high due to how
current_target() is calculated. Since all of these calculations are done
under the balloon mutex, it's impossible to trigger the BUG_ON in your
version or the break in konrad's #devel/balloon-hotplug branch.

-- 
Daniel De Graaf
National Security Agency

Re: [PATCH v2] Use ballooned pages for gntdev

[Konrad Rzeszutek Wilk]

> version or the break in konrad's #devel/balloon-hotplug branch.

#devel/balloon now.