2015-06-25 17:11:19

by David Vrabel

[permalink] [raw]
Subject: [PATCHv1 0/8] mm,xen/balloon: memory hotplug improvements

The series improves the use of hotplug memory in the Xen balloon
driver.

- Reliably find a non-conflicting location for the hotplugged memory
(this fixes memory hotplug in a number of cases, particularly in
dom0).

- Use hotplugged memory for alloc_xenballooned_pages() (keeping more
memory available for the domain and reducing fragmentation of the
p2m).

David


2015-06-25 17:13:18

by David Vrabel

[permalink] [raw]
Subject: [PATCHv1 1/8] mm: memory hotplug with an existing resource

Add add_memory_resource() to add memory using an existing "System RAM"
resource. This is useful if the memory region is being located by
finding a free resource slot with allocate_resource().

Xen guests will make use of this in their balloon driver to hotplug
arbitrary amounts of memory in response to toolstack requests.

Signed-off-by: David Vrabel <[email protected]>
Cc: Andrew Morton <[email protected]>
---
include/linux/memory_hotplug.h | 2 ++
mm/memory_hotplug.c | 28 +++++++++++++++++++++-------
2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 6ffa0ac..c76d371 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -11,6 +11,7 @@ struct zone;
struct pglist_data;
struct mem_section;
struct memory_block;
+struct resource;

#ifdef CONFIG_MEMORY_HOTPLUG

@@ -266,6 +267,7 @@ static inline void remove_memory(int nid, u64 start, u64 size) {}
extern int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
void *arg, int (*func)(struct memory_block *, void *));
extern int add_memory(int nid, u64 start, u64 size);
+extern int add_memory_resource(int nid, struct resource *resource);
extern int zone_for_memory(int nid, u64 start, u64 size, int zone_default);
extern int arch_add_memory(int nid, u64 start, u64 size);
extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 9e88f74..69eb1d0 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1215,23 +1215,21 @@ int zone_for_memory(int nid, u64 start, u64 size, int zone_default)
}

/* we are OK calling __meminit stuff here - we have CONFIG_MEMORY_HOTPLUG */
-int __ref add_memory(int nid, u64 start, u64 size)
+int __ref add_memory_resource(int nid, struct resource *res)
{
+ u64 start, size;
pg_data_t *pgdat = NULL;
bool new_pgdat;
bool new_node;
- struct resource *res;
int ret;

+ start = res->start;
+ size = resource_size(res);
+
ret = check_hotplug_memory_range(start, size);
if (ret)
return ret;

- res = register_memory_resource(start, size);
- ret = -EEXIST;
- if (!res)
- return ret;
-
{ /* Stupid hack to suppress address-never-null warning */
void *p = NODE_DATA(nid);
new_pgdat = !p;
@@ -1281,6 +1279,22 @@ out:
mem_hotplug_done();
return ret;
}
+EXPORT_SYMBOL_GPL(add_memory_resource);
+
+int __ref add_memory(int nid, u64 start, u64 size)
+{
+ struct resource *res;
+ int ret;
+
+ res = register_memory_resource(start, size);
+ if (!res)
+ return -EEXIST;
+
+ ret = add_memory_resource(nid, res);
+ if (ret < 0)
+ release_memory_resource(res);
+ return ret;
+}
EXPORT_SYMBOL_GPL(add_memory);

#ifdef CONFIG_MEMORY_HOTREMOVE
--
1.7.10.4

2015-06-25 17:11:41

by David Vrabel

[permalink] [raw]
Subject: [PATCHv1 2/8] xen/balloon: remove scratch page left overs

Commit 0bb599fd30108883b00c7d4a226eeb49111e6932 (xen: remove scratch
frames for ballooned pages and m2p override) removed the use of the
scratch page for ballooned out pages.

Remove some left over function definitions.

Signed-off-by: David Vrabel <[email protected]>
---
include/xen/balloon.h | 3 ---
1 file changed, 3 deletions(-)

diff --git a/include/xen/balloon.h b/include/xen/balloon.h
index a4c1c6a..cc2e1a7 100644
--- a/include/xen/balloon.h
+++ b/include/xen/balloon.h
@@ -29,9 +29,6 @@ int alloc_xenballooned_pages(int nr_pages, struct page **pages,
bool highmem);
void free_xenballooned_pages(int nr_pages, struct page **pages);

-struct page *get_balloon_scratch_page(void);
-void put_balloon_scratch_page(void);
-
struct device;
#ifdef CONFIG_XEN_SELFBALLOONING
extern int register_xen_selfballooning(struct device *dev);
--
1.7.10.4

2015-06-25 17:11:31

by David Vrabel

[permalink] [raw]
Subject: [PATCHv1 3/8] x86/xen: discard RAM regions above the maximum reservation

During setup, discard RAM regions that are above the maximum
reservation (instead of marking them as E820_UNUSABLE). This allows
hotplug memory to be placed at these addresses.

Signed-off-by: David Vrabel <[email protected]>
---
arch/x86/xen/setup.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 55f388e..32910c5 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -646,6 +646,7 @@ char * __init xen_memory_setup(void)
phys_addr_t addr = map[i].addr;
phys_addr_t size = map[i].size;
u32 type = map[i].type;
+ bool discard = false;

if (type == E820_RAM) {
if (addr < mem_end) {
@@ -656,10 +657,11 @@ char * __init xen_memory_setup(void)
xen_add_extra_mem(addr, size);
xen_max_p2m_pfn = PFN_DOWN(addr + size);
} else
- type = E820_UNUSABLE;
+ discard = true;
}

- xen_align_and_add_e820_region(addr, size, type);
+ if (!discard)
+ xen_align_and_add_e820_region(addr, size, type);

map[i].addr += size;
map[i].size -= size;
--
1.7.10.4

2015-06-25 17:12:31

by David Vrabel

[permalink] [raw]
Subject: [PATCHv1 4/8] xen/balloon: find non-conflicting regions to place hotplugged memory

Instead of placing hotplugged memory at the end of RAM (which may
conflict with PCI devices or reserved regions) use allocate_resource()
to get a new, suitably aligned resource that does not conflict.

Signed-off-by: David Vrabel <[email protected]>
---
drivers/xen/balloon.c | 63 +++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 53 insertions(+), 10 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index fd93369..d0121ee 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -54,6 +54,7 @@
#include <linux/memory.h>
#include <linux/memory_hotplug.h>
#include <linux/percpu-defs.h>
+#include <linux/slab.h>

#include <asm/page.h>
#include <asm/pgalloc.h>
@@ -208,6 +209,42 @@ static bool balloon_is_inflated(void)
return false;
}

+static struct resource *additional_memory_resource(phys_addr_t size)
+{
+ struct resource *res;
+ int ret;
+
+ res = kzalloc(sizeof(*res), GFP_KERNEL);
+ if (!res)
+ return NULL;
+
+ res->name = "System RAM";
+ res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+
+ ret = allocate_resource(&iomem_resource, res,
+ size, 0, -1,
+ PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
+ if (ret < 0) {
+ pr_err("Cannot allocate new System RAM resource\n");
+ kfree(res);
+ return NULL;
+ }
+
+ return res;
+}
+
+static void release_memory_resource(struct resource *resource)
+{
+ if (!resource)
+ return;
+ /*
+ * No need to reset region to identity mapped since we now
+ * know that no I/O can be in this region
+ */
+ release_resource(resource);
+ kfree(resource);
+}
+
/*
* reserve_additional_memory() adds memory region of size >= credit above
* max_pfn. New region is section aligned and size is modified to be multiple
@@ -221,13 +258,17 @@ static bool balloon_is_inflated(void)

static enum bp_state reserve_additional_memory(long credit)
{
+ struct resource *resource;
int nid, rc;
- u64 hotplug_start_paddr;
- unsigned long balloon_hotplug = credit;
+ unsigned long balloon_hotplug;
+
+ balloon_hotplug = round_up(credit, PAGES_PER_SECTION);
+
+ resource = additional_memory_resource(balloon_hotplug * PAGE_SIZE);
+ if (!resource)
+ goto err;

- hotplug_start_paddr = PFN_PHYS(SECTION_ALIGN_UP(max_pfn));
- balloon_hotplug = round_up(balloon_hotplug, PAGES_PER_SECTION);
- nid = memory_add_physaddr_to_nid(hotplug_start_paddr);
+ nid = memory_add_physaddr_to_nid(resource->start);

#ifdef CONFIG_XEN_HAVE_PVMMU
/*
@@ -242,21 +283,20 @@ static enum bp_state reserve_additional_memory(long credit)
if (!xen_feature(XENFEAT_auto_translated_physmap)) {
unsigned long pfn, i;

- pfn = PFN_DOWN(hotplug_start_paddr);
+ pfn = PFN_DOWN(resource->start);
for (i = 0; i < balloon_hotplug; i++) {
if (!set_phys_to_machine(pfn + i, INVALID_P2M_ENTRY)) {
pr_warn("set_phys_to_machine() failed, no memory added\n");
- return BP_ECANCELED;
+ goto err;
}
}
}
#endif

- rc = add_memory(nid, hotplug_start_paddr, balloon_hotplug << PAGE_SHIFT);
-
+ rc = add_memory_resource(nid, resource);
if (rc) {
pr_warn("Cannot add additional memory (%i)\n", rc);
- return BP_ECANCELED;
+ goto err;
}

balloon_hotplug -= credit;
@@ -265,6 +305,9 @@ static enum bp_state reserve_additional_memory(long credit)
balloon_stats.balloon_hotplug = balloon_hotplug;

return BP_DONE;
+ err:
+ release_memory_resource(resource);
+ return BP_ECANCELED;
}

static void xen_online_page(struct page *page)
--
1.7.10.4

2015-06-25 17:12:16

by David Vrabel

[permalink] [raw]
Subject: [PATCHv1 5/8] xen/balloon: rationalize memory hotplug stats

The stats used for memory hotplug make no sense and are fiddled with
in odd ways. Remove them and introduce total_pages to track the total
number of pages (both populated and unpopulated) including those within
hotplugged regions (note that this includes not yet onlined pages).

This will be useful when deciding whether additional memory needs to be
hotplugged.

Signed-off-by: David Vrabel <[email protected]>
---
drivers/xen/balloon.c | 75 ++++++++-----------------------------------------
include/xen/balloon.h | 5 +---
2 files changed, 13 insertions(+), 67 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index d0121ee..960ac79 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -194,21 +194,6 @@ static enum bp_state update_schedule(enum bp_state state)
}

#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
-static long current_credit(void)
-{
- return balloon_stats.target_pages - balloon_stats.current_pages -
- balloon_stats.hotplug_pages;
-}
-
-static bool balloon_is_inflated(void)
-{
- if (balloon_stats.balloon_low || balloon_stats.balloon_high ||
- balloon_stats.balloon_hotplug)
- return true;
- else
- return false;
-}
-
static struct resource *additional_memory_resource(phys_addr_t size)
{
struct resource *res;
@@ -299,10 +284,7 @@ static enum bp_state reserve_additional_memory(long credit)
goto err;
}

- balloon_hotplug -= credit;
-
- balloon_stats.hotplug_pages += credit;
- balloon_stats.balloon_hotplug = balloon_hotplug;
+ balloon_stats.total_pages += balloon_hotplug;

return BP_DONE;
err:
@@ -318,11 +300,6 @@ static void xen_online_page(struct page *page)

__balloon_append(page);

- if (balloon_stats.hotplug_pages)
- --balloon_stats.hotplug_pages;
- else
- --balloon_stats.balloon_hotplug;
-
mutex_unlock(&balloon_mutex);
}

@@ -339,32 +316,22 @@ static struct notifier_block xen_memory_nb = {
.priority = 0
};
#else
-static long current_credit(void)
+static enum bp_state reserve_additional_memory(long credit)
{
- unsigned long target = balloon_stats.target_pages;
-
- target = min(target,
- balloon_stats.current_pages +
- balloon_stats.balloon_low +
- balloon_stats.balloon_high);
-
- return target - balloon_stats.current_pages;
+ balloon_stats.target_pages = balloon_stats.current_pages;
+ return BP_DONE;
}
+#endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */

-static bool balloon_is_inflated(void)
+static long current_credit(void)
{
- if (balloon_stats.balloon_low || balloon_stats.balloon_high)
- return true;
- else
- return false;
+ return balloon_stats.target_pages - balloon_stats.current_pages;
}

-static enum bp_state reserve_additional_memory(long credit)
+static bool balloon_is_inflated(void)
{
- balloon_stats.target_pages = balloon_stats.current_pages;
- return BP_DONE;
+ return balloon_stats.balloon_low || balloon_stats.balloon_high;
}
-#endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */

static enum bp_state increase_reservation(unsigned long nr_pages)
{
@@ -377,15 +344,6 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
.domid = DOMID_SELF
};

-#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
- if (!balloon_stats.balloon_low && !balloon_stats.balloon_high) {
- nr_pages = min(nr_pages, balloon_stats.balloon_hotplug);
- balloon_stats.hotplug_pages += nr_pages;
- balloon_stats.balloon_hotplug -= nr_pages;
- return BP_DONE;
- }
-#endif
-
if (nr_pages > ARRAY_SIZE(frame_list))
nr_pages = ARRAY_SIZE(frame_list);

@@ -448,15 +406,6 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
.domid = DOMID_SELF
};

-#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
- if (balloon_stats.hotplug_pages) {
- nr_pages = min(nr_pages, balloon_stats.hotplug_pages);
- balloon_stats.hotplug_pages -= nr_pages;
- balloon_stats.balloon_hotplug += nr_pages;
- return BP_DONE;
- }
-#endif
-
if (nr_pages > ARRAY_SIZE(frame_list))
nr_pages = ARRAY_SIZE(frame_list);

@@ -646,6 +595,8 @@ static void __init balloon_add_region(unsigned long start_pfn,
don't subtract from it. */
__balloon_append(page);
}
+
+ balloon_stats.total_pages += extra_pfn_end - start_pfn;
}

static int __init balloon_init(void)
@@ -663,6 +614,7 @@ static int __init balloon_init(void)
balloon_stats.target_pages = balloon_stats.current_pages;
balloon_stats.balloon_low = 0;
balloon_stats.balloon_high = 0;
+ balloon_stats.total_pages = balloon_stats.current_pages;

balloon_stats.schedule_delay = 1;
balloon_stats.max_schedule_delay = 32;
@@ -670,9 +622,6 @@ static int __init balloon_init(void)
balloon_stats.max_retry_count = RETRY_UNLIMITED;

#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
- balloon_stats.hotplug_pages = 0;
- balloon_stats.balloon_hotplug = 0;
-
set_online_page_callback(&xen_online_page);
register_memory_notifier(&xen_memory_nb);
#endif
diff --git a/include/xen/balloon.h b/include/xen/balloon.h
index cc2e1a7..c8aee7a 100644
--- a/include/xen/balloon.h
+++ b/include/xen/balloon.h
@@ -11,14 +11,11 @@ struct balloon_stats {
/* Number of pages in high- and low-memory balloons. */
unsigned long balloon_low;
unsigned long balloon_high;
+ unsigned long total_pages;
unsigned long schedule_delay;
unsigned long max_schedule_delay;
unsigned long retry_count;
unsigned long max_retry_count;
-#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
- unsigned long hotplug_pages;
- unsigned long balloon_hotplug;
-#endif
};

extern struct balloon_stats balloon_stats;
--
1.7.10.4

2015-06-25 17:12:02

by David Vrabel

[permalink] [raw]
Subject: [PATCHv1 6/8] xen/balloon: only hotplug additional memory if required

Now that we track the total number of pages (included hotplugged
regions), it is easy to determine if more memory needs to be
hotplugged.

Signed-off-by: David Vrabel <[email protected]>
---
drivers/xen/balloon.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 960ac79..dd41da8 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -241,12 +241,22 @@ static void release_memory_resource(struct resource *resource)
* bit set). Real size of added memory is established at page onlining stage.
*/

-static enum bp_state reserve_additional_memory(long credit)
+static enum bp_state reserve_additional_memory(void)
{
+ long credit;
struct resource *resource;
int nid, rc;
unsigned long balloon_hotplug;

+ credit = balloon_stats.target_pages - balloon_stats.total_pages;
+
+ /*
+ * Already hotplugged enough pages? Wait for them to be
+ * onlined.
+ */
+ if (credit <= 0)
+ return BP_EAGAIN;
+
balloon_hotplug = round_up(credit, PAGES_PER_SECTION);

resource = additional_memory_resource(balloon_hotplug * PAGE_SIZE);
@@ -316,7 +326,7 @@ static struct notifier_block xen_memory_nb = {
.priority = 0
};
#else
-static enum bp_state reserve_additional_memory(long credit)
+static enum bp_state reserve_additional_memory(void)
{
balloon_stats.target_pages = balloon_stats.current_pages;
return BP_DONE;
@@ -483,7 +493,7 @@ static void balloon_process(struct work_struct *work)
if (balloon_is_inflated())
state = increase_reservation(credit);
else
- state = reserve_additional_memory(credit);
+ state = reserve_additional_memory();
}

if (credit < 0)
--
1.7.10.4

2015-06-25 17:13:42

by David Vrabel

[permalink] [raw]
Subject: [PATCHv1 7/8] xen/balloon: make alloc_xenballoon_pages() always allocate low pages

All users of alloc_xenballoon_pages() wanted low memory pages, so
remove the option for high memory.

Signed-off-by: David Vrabel <[email protected]>
---
arch/x86/xen/grant-table.c | 2 +-
drivers/xen/balloon.c | 21 ++++++++-------------
drivers/xen/grant-table.c | 2 +-
drivers/xen/privcmd.c | 2 +-
drivers/xen/xenbus/xenbus_client.c | 3 +--
include/xen/balloon.h | 3 +--
6 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/arch/x86/xen/grant-table.c b/arch/x86/xen/grant-table.c
index 1580e7a..e079500 100644
--- a/arch/x86/xen/grant-table.c
+++ b/arch/x86/xen/grant-table.c
@@ -133,7 +133,7 @@ static int __init xlated_setup_gnttab_pages(void)
kfree(pages);
return -ENOMEM;
}
- rc = alloc_xenballooned_pages(nr_grant_frames, pages, 0 /* lowmem */);
+ rc = alloc_xenballooned_pages(nr_grant_frames, pages);
if (rc) {
pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
nr_grant_frames, rc);
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index dd41da8..95c261c 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -134,17 +134,16 @@ 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(bool prefer_highmem)
+static struct page *balloon_retrieve(bool require_lowmem)
{
struct page *page;

if (list_empty(&ballooned_pages))
return NULL;

- if (prefer_highmem)
- page = list_entry(ballooned_pages.prev, struct page, lru);
- else
- page = list_entry(ballooned_pages.next, struct page, lru);
+ page = list_entry(ballooned_pages.next, struct page, lru);
+ if (require_lowmem && PageHighMem(page))
+ return NULL;
list_del(&page->lru);

if (PageHighMem(page))
@@ -527,24 +526,20 @@ 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
- * @highmem: allow highmem pages
* @return 0 on success, error otherwise
*/
-int alloc_xenballooned_pages(int nr_pages, struct page **pages, bool highmem)
+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(highmem);
- if (page && (highmem || !PageHighMem(page))) {
+ page = balloon_retrieve(true);
+ if (page) {
pages[pgno++] = page;
} else {
enum bp_state st;
- if (page)
- balloon_append(page);
- st = decrease_reservation(nr_pages - pgno,
- highmem ? GFP_HIGHUSER : GFP_USER);
+ st = decrease_reservation(nr_pages - pgno, GFP_USER);
if (st != BP_DONE)
goto out_undo;
}
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index b1c7170..13ecd18 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -688,7 +688,7 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
int i;
int ret;

- ret = alloc_xenballooned_pages(nr_pages, pages, false);
+ ret = alloc_xenballooned_pages(nr_pages, pages);
if (ret < 0)
return ret;

diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 5a29616..59cfec9 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -401,7 +401,7 @@ static int alloc_empty_pages(struct vm_area_struct *vma, int numpgs)
if (pages == NULL)
return -ENOMEM;

- rc = alloc_xenballooned_pages(numpgs, pages, 0);
+ rc = alloc_xenballooned_pages(numpgs, pages);
if (rc != 0) {
pr_warn("%s Could not alloc %d pfns rc:%d\n", __func__,
numpgs, rc);
diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
index 96b2011..7f23158 100644
--- a/drivers/xen/xenbus/xenbus_client.c
+++ b/drivers/xen/xenbus/xenbus_client.c
@@ -614,8 +614,7 @@ static int xenbus_map_ring_valloc_hvm(struct xenbus_device *dev,
if (!node)
return -ENOMEM;

- err = alloc_xenballooned_pages(nr_grefs, node->hvm.pages,
- false /* lowmem */);
+ err = alloc_xenballooned_pages(nr_grefs, node->hvm.pages);
if (err)
goto out_err;

diff --git a/include/xen/balloon.h b/include/xen/balloon.h
index c8aee7a..83efdeb 100644
--- a/include/xen/balloon.h
+++ b/include/xen/balloon.h
@@ -22,8 +22,7 @@ extern struct balloon_stats balloon_stats;

void balloon_set_new_target(unsigned long target);

-int alloc_xenballooned_pages(int nr_pages, struct page **pages,
- bool highmem);
+int alloc_xenballooned_pages(int nr_pages, struct page **pages);
void free_xenballooned_pages(int nr_pages, struct page **pages);

struct device;
--
1.7.10.4

2015-06-25 17:12:25

by David Vrabel

[permalink] [raw]
Subject: [PATCHv1 8/8] xen/balloon: use hotplugged pages for foreign mappings etc.

alloc_xenballooned_pages() is used to get ballooned pages to back
foreign mappings etc. Instead of having to balloon out real pages,
use (if supported) hotplugged memory.

This makes more memory available to the guest and reduces
fragmentation in the p2m.

If userspace is lacking a udev rule (or similar) to online hotplugged
regions automatically, alloc_xenballooned_pages() will timeout and
fall back to the old behaviour of ballooning out pages.

Signed-off-by: David Vrabel <[email protected]>
---
drivers/xen/balloon.c | 32 ++++++++++++++++++++++++++------
include/xen/balloon.h | 1 +
2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 95c261c..a26c5f3 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -97,6 +97,7 @@ static xen_pfn_t frame_list[PAGE_SIZE / sizeof(unsigned long)];

/* List of ballooned pages, threaded through the mem_map array. */
static LIST_HEAD(ballooned_pages);
+static DECLARE_WAIT_QUEUE_HEAD(balloon_wq);

/* Main work function, always executed in process context. */
static void balloon_process(struct work_struct *work);
@@ -125,6 +126,7 @@ static void __balloon_append(struct page *page)
list_add(&page->lru, &ballooned_pages);
balloon_stats.balloon_low++;
}
+ wake_up(&balloon_wq);
}

static void balloon_append(struct page *page)
@@ -247,7 +249,8 @@ static enum bp_state reserve_additional_memory(void)
int nid, rc;
unsigned long balloon_hotplug;

- credit = balloon_stats.target_pages - balloon_stats.total_pages;
+ credit = balloon_stats.target_pages + balloon_stats.target_unpopulated
+ - balloon_stats.total_pages;

/*
* Already hotplugged enough pages? Wait for them to be
@@ -328,7 +331,7 @@ static struct notifier_block xen_memory_nb = {
static enum bp_state reserve_additional_memory(void)
{
balloon_stats.target_pages = balloon_stats.current_pages;
- return BP_DONE;
+ return BP_ECANCELED;
}
#endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */

@@ -532,13 +535,31 @@ int alloc_xenballooned_pages(int nr_pages, struct page **pages)
{
int pgno = 0;
struct page *page;
+
mutex_lock(&balloon_mutex);
+
+ balloon_stats.target_unpopulated += nr_pages;
+
while (pgno < nr_pages) {
page = balloon_retrieve(true);
if (page) {
pages[pgno++] = page;
} else {
enum bp_state st;
+
+ st = reserve_additional_memory();
+ if (st != BP_ECANCELED) {
+ int ret;
+
+ mutex_unlock(&balloon_mutex);
+ ret = wait_event_timeout(balloon_wq,
+ !list_empty(&ballooned_pages),
+ msecs_to_jiffies(100));
+ mutex_lock(&balloon_mutex);
+ if (ret > 0)
+ continue;
+ }
+
st = decrease_reservation(nr_pages - pgno, GFP_USER);
if (st != BP_DONE)
goto out_undo;
@@ -547,11 +568,8 @@ int alloc_xenballooned_pages(int nr_pages, struct page **pages)
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);
+ free_xenballooned_pages(pgno, pages);
return -ENOMEM;
}
EXPORT_SYMBOL(alloc_xenballooned_pages);
@@ -572,6 +590,8 @@ void free_xenballooned_pages(int nr_pages, struct page **pages)
balloon_append(pages[i]);
}

+ balloon_stats.target_unpopulated -= nr_pages;
+
/* The balloon may be too large now. Shrink it if needed. */
if (current_credit())
schedule_delayed_work(&balloon_worker, 0);
diff --git a/include/xen/balloon.h b/include/xen/balloon.h
index 83efdeb..d1767df 100644
--- a/include/xen/balloon.h
+++ b/include/xen/balloon.h
@@ -8,6 +8,7 @@ struct balloon_stats {
/* We aim for 'current allocation' == 'target allocation'. */
unsigned long current_pages;
unsigned long target_pages;
+ unsigned long target_unpopulated;
/* Number of pages in high- and low-memory balloons. */
unsigned long balloon_low;
unsigned long balloon_high;
--
1.7.10.4

2015-06-25 18:01:31

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCHv1 1/8] mm: memory hotplug with an existing resource

On Thu, Jun 25, 2015 at 06:10:56PM +0100, David Vrabel wrote:
> Add add_memory_resource() to add memory using an existing "System RAM"
> resource. This is useful if the memory region is being located by
> finding a free resource slot with allocate_resource().
>
> Xen guests will make use of this in their balloon driver to hotplug
> arbitrary amounts of memory in response to toolstack requests.
>
> Signed-off-by: David Vrabel <[email protected]>
> Cc: Andrew Morton <[email protected]>

Reviewed-by: Daniel Kiper <[email protected]>

Daniel

2015-06-25 18:02:12

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCHv1 2/8] xen/balloon: remove scratch page left overs

On Thu, Jun 25, 2015 at 06:10:57PM +0100, David Vrabel wrote:
> Commit 0bb599fd30108883b00c7d4a226eeb49111e6932 (xen: remove scratch
> frames for ballooned pages and m2p override) removed the use of the
> scratch page for ballooned out pages.
>
> Remove some left over function definitions.
>
> Signed-off-by: David Vrabel <[email protected]>

Reviewed-by: Daniel Kiper <[email protected]>

Daniel

2015-06-25 18:03:59

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCHv1 3/8] x86/xen: discard RAM regions above the maximum reservation

On Thu, Jun 25, 2015 at 06:10:58PM +0100, David Vrabel wrote:
> During setup, discard RAM regions that are above the maximum
> reservation (instead of marking them as E820_UNUSABLE). This allows
> hotplug memory to be placed at these addresses.
>
> Signed-off-by: David Vrabel <[email protected]>

Reviewed-by: Daniel Kiper <[email protected]>

Daniel

2015-06-25 18:17:10

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCHv1 4/8] xen/balloon: find non-conflicting regions to place hotplugged memory

On Thu, Jun 25, 2015 at 06:10:59PM +0100, David Vrabel wrote:
> Instead of placing hotplugged memory at the end of RAM (which may
> conflict with PCI devices or reserved regions) use allocate_resource()
> to get a new, suitably aligned resource that does not conflict.
>
> Signed-off-by: David Vrabel <[email protected]>

In general Reviewed-by: Daniel Kiper <[email protected]>

but two nitpicks below...

> ---
> drivers/xen/balloon.c | 63 +++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 53 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index fd93369..d0121ee 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -54,6 +54,7 @@
> #include <linux/memory.h>
> #include <linux/memory_hotplug.h>
> #include <linux/percpu-defs.h>
> +#include <linux/slab.h>
>
> #include <asm/page.h>
> #include <asm/pgalloc.h>
> @@ -208,6 +209,42 @@ static bool balloon_is_inflated(void)
> return false;
> }
>
> +static struct resource *additional_memory_resource(phys_addr_t size)
> +{
> + struct resource *res;
> + int ret;
> +
> + res = kzalloc(sizeof(*res), GFP_KERNEL);
> + if (!res)
> + return NULL;
> +
> + res->name = "System RAM";
> + res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> +
> + ret = allocate_resource(&iomem_resource, res,
> + size, 0, -1,
> + PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
> + if (ret < 0) {
> + pr_err("Cannot allocate new System RAM resource\n");
> + kfree(res);
> + return NULL;
> + }
> +
> + return res;
> +}
> +
> +static void release_memory_resource(struct resource *resource)
> +{
> + if (!resource)
> + return;

Please add one empty line here.

> + /*
> + * No need to reset region to identity mapped since we now
> + * know that no I/O can be in this region
> + */
> + release_resource(resource);
> + kfree(resource);
> +}
> +
> /*
> * reserve_additional_memory() adds memory region of size >= credit above
> * max_pfn. New region is section aligned and size is modified to be multiple
> @@ -221,13 +258,17 @@ static bool balloon_is_inflated(void)
>
> static enum bp_state reserve_additional_memory(long credit)
> {
> + struct resource *resource;
> int nid, rc;
> - u64 hotplug_start_paddr;
> - unsigned long balloon_hotplug = credit;
> + unsigned long balloon_hotplug;
> +
> + balloon_hotplug = round_up(credit, PAGES_PER_SECTION);
> +
> + resource = additional_memory_resource(balloon_hotplug * PAGE_SIZE);
> + if (!resource)
> + goto err;
>
> - hotplug_start_paddr = PFN_PHYS(SECTION_ALIGN_UP(max_pfn));
> - balloon_hotplug = round_up(balloon_hotplug, PAGES_PER_SECTION);
> - nid = memory_add_physaddr_to_nid(hotplug_start_paddr);
> + nid = memory_add_physaddr_to_nid(resource->start);
>
> #ifdef CONFIG_XEN_HAVE_PVMMU
> /*
> @@ -242,21 +283,20 @@ static enum bp_state reserve_additional_memory(long credit)
> if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> unsigned long pfn, i;
>
> - pfn = PFN_DOWN(hotplug_start_paddr);
> + pfn = PFN_DOWN(resource->start);
> for (i = 0; i < balloon_hotplug; i++) {
> if (!set_phys_to_machine(pfn + i, INVALID_P2M_ENTRY)) {
> pr_warn("set_phys_to_machine() failed, no memory added\n");
> - return BP_ECANCELED;
> + goto err;
> }
> }
> }
> #endif
>
> - rc = add_memory(nid, hotplug_start_paddr, balloon_hotplug << PAGE_SHIFT);
> -
> + rc = add_memory_resource(nid, resource);
> if (rc) {
> pr_warn("Cannot add additional memory (%i)\n", rc);
> - return BP_ECANCELED;
> + goto err;
> }
>
> balloon_hotplug -= credit;
> @@ -265,6 +305,9 @@ static enum bp_state reserve_additional_memory(long credit)
> balloon_stats.balloon_hotplug = balloon_hotplug;
>
> return BP_DONE;

Ditto.

> + err:
> + release_memory_resource(resource);
> + return BP_ECANCELED;
> }
>
> static void xen_online_page(struct page *page)
> --
> 1.7.10.4

Daniel

2015-06-25 18:38:58

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCHv1 5/8] xen/balloon: rationalize memory hotplug stats

On Thu, Jun 25, 2015 at 06:11:00PM +0100, David Vrabel wrote:
> The stats used for memory hotplug make no sense and are fiddled with
> in odd ways. Remove them and introduce total_pages to track the total
> number of pages (both populated and unpopulated) including those within
> hotplugged regions (note that this includes not yet onlined pages).
>
> This will be useful when deciding whether additional memory needs to be
> hotplugged.
>
> Signed-off-by: David Vrabel <[email protected]>

Nice optimization! I suppose that it is remnant from very early
version of memory hotplug. Probably after a few patch series
iterations hotplug_pages and balloon_hotplug lost their meaning
and I did not catch it. Additionally, as I can see there is not
any consumer for total_pages here. So, I think that we can go
further and remove this obfuscated code at all.

Daniel

2015-06-25 18:55:04

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCHv1 5/8] xen/balloon: rationalize memory hotplug stats

On Thu, Jun 25, 2015 at 08:38:36PM +0200, Daniel Kiper wrote:
> On Thu, Jun 25, 2015 at 06:11:00PM +0100, David Vrabel wrote:
> > The stats used for memory hotplug make no sense and are fiddled with
> > in odd ways. Remove them and introduce total_pages to track the total
> > number of pages (both populated and unpopulated) including those within
> > hotplugged regions (note that this includes not yet onlined pages).
> >
> > This will be useful when deciding whether additional memory needs to be
> > hotplugged.
> >
> > Signed-off-by: David Vrabel <[email protected]>
>
> Nice optimization! I suppose that it is remnant from very early
> version of memory hotplug. Probably after a few patch series
> iterations hotplug_pages and balloon_hotplug lost their meaning
> and I did not catch it. Additionally, as I can see there is not
> any consumer for total_pages here. So, I think that we can go
> further and remove this obfuscated code at all.

Err... Ignore that. I missed next patch... Should not both of them
merged in one or commit comment contain clear info that this will
be used by next patch.

Daniel

2015-06-25 21:18:59

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCHv1 6/8] xen/balloon: only hotplug additional memory if required

On Thu, Jun 25, 2015 at 06:11:01PM +0100, David Vrabel wrote:
> Now that we track the total number of pages (included hotplugged
> regions), it is easy to determine if more memory needs to be
> hotplugged.
>
> Signed-off-by: David Vrabel <[email protected]>
> ---
> drivers/xen/balloon.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 960ac79..dd41da8 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -241,12 +241,22 @@ static void release_memory_resource(struct resource *resource)
> * bit set). Real size of added memory is established at page onlining stage.
> */
>
> -static enum bp_state reserve_additional_memory(long credit)
> +static enum bp_state reserve_additional_memory(void)
> {
> + long credit;
> struct resource *resource;
> int nid, rc;
> unsigned long balloon_hotplug;
>
> + credit = balloon_stats.target_pages - balloon_stats.total_pages;
> +
> + /*
> + * Already hotplugged enough pages? Wait for them to be
> + * onlined.
> + */

Comment is wrong or at least misleading. Both values does not depend on onlining.

> + if (credit <= 0)
> + return BP_EAGAIN;

Not BP_EAGAIN for sure. It should be BP_DONE but then balloon_process() will go
into loop until memory is onlined at least up to balloon_stats.target_pages.
BP_ECANCELED does work but it is misleading because it is not an error. So, maybe
we should introduce BP_STOP (or something like that) which works like BP_ECANCELED
and is not BP_ECANCELED.

Daniel

2015-06-25 21:32:04

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCHv1 5/8] xen/balloon: rationalize memory hotplug stats

On Thu, Jun 25, 2015 at 08:54:45PM +0200, Daniel Kiper wrote:
> On Thu, Jun 25, 2015 at 08:38:36PM +0200, Daniel Kiper wrote:
> > On Thu, Jun 25, 2015 at 06:11:00PM +0100, David Vrabel wrote:
> > > The stats used for memory hotplug make no sense and are fiddled with
> > > in odd ways. Remove them and introduce total_pages to track the total
> > > number of pages (both populated and unpopulated) including those within
> > > hotplugged regions (note that this includes not yet onlined pages).
> > >
> > > This will be useful when deciding whether additional memory needs to be
> > > hotplugged.
> > >
> > > Signed-off-by: David Vrabel <[email protected]>
> >
> > Nice optimization! I suppose that it is remnant from very early
> > version of memory hotplug. Probably after a few patch series
> > iterations hotplug_pages and balloon_hotplug lost their meaning
> > and I did not catch it. Additionally, as I can see there is not
> > any consumer for total_pages here. So, I think that we can go
> > further and remove this obfuscated code at all.
>
> Err... Ignore that. I missed next patch... Should not both of them
> merged in one or commit comment contain clear info that this will
> be used by next patch.

This patch, #6 and probably #3 change reserve_additional_memory() behavior.
Please check comment before that function and update it accordingly.

It looks that balloon_stats.total_pages is used only in memory hotplug case.
Please do references (and definition) to it inside #ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
like it is done in balloon_stats.hotplug_pages and balloon_stats.balloon_hotplug case.

Daniel

2015-06-25 21:37:19

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCHv1 7/8] xen/balloon: make alloc_xenballoon_pages() always allocate low pages

On Thu, Jun 25, 2015 at 06:11:02PM +0100, David Vrabel wrote:
> All users of alloc_xenballoon_pages() wanted low memory pages, so
> remove the option for high memory.
>
> Signed-off-by: David Vrabel <[email protected]>

Reviewed-by: Daniel Kiper <[email protected]>

Daniel

2015-06-25 21:50:12

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCHv1 8/8] xen/balloon: use hotplugged pages for foreign mappings etc.

On Thu, Jun 25, 2015 at 06:11:03PM +0100, David Vrabel wrote:
> alloc_xenballooned_pages() is used to get ballooned pages to back
> foreign mappings etc. Instead of having to balloon out real pages,
> use (if supported) hotplugged memory.
>
> This makes more memory available to the guest and reduces
> fragmentation in the p2m.
>
> If userspace is lacking a udev rule (or similar) to online hotplugged
> regions automatically, alloc_xenballooned_pages() will timeout and
> fall back to the old behaviour of ballooning out pages.
>
> Signed-off-by: David Vrabel <[email protected]>

In general Reviewed-by: Daniel Kiper <[email protected]> but...

> ---
> drivers/xen/balloon.c | 32 ++++++++++++++++++++++++++------
> include/xen/balloon.h | 1 +
> 2 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 95c261c..a26c5f3 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -97,6 +97,7 @@ static xen_pfn_t frame_list[PAGE_SIZE / sizeof(unsigned long)];
>
> /* List of ballooned pages, threaded through the mem_map array. */
> static LIST_HEAD(ballooned_pages);
> +static DECLARE_WAIT_QUEUE_HEAD(balloon_wq);
>
> /* Main work function, always executed in process context. */
> static void balloon_process(struct work_struct *work);
> @@ -125,6 +126,7 @@ static void __balloon_append(struct page *page)
> list_add(&page->lru, &ballooned_pages);
> balloon_stats.balloon_low++;
> }
> + wake_up(&balloon_wq);
> }
>
> static void balloon_append(struct page *page)
> @@ -247,7 +249,8 @@ static enum bp_state reserve_additional_memory(void)
> int nid, rc;
> unsigned long balloon_hotplug;
>
> - credit = balloon_stats.target_pages - balloon_stats.total_pages;
> + credit = balloon_stats.target_pages + balloon_stats.target_unpopulated
> + - balloon_stats.total_pages;
>
> /*
> * Already hotplugged enough pages? Wait for them to be
> @@ -328,7 +331,7 @@ static struct notifier_block xen_memory_nb = {
> static enum bp_state reserve_additional_memory(void)
> {
> balloon_stats.target_pages = balloon_stats.current_pages;
> - return BP_DONE;
> + return BP_ECANCELED;
> }
> #endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */
>
> @@ -532,13 +535,31 @@ int alloc_xenballooned_pages(int nr_pages, struct page **pages)
> {
> int pgno = 0;
> struct page *page;
> +
> mutex_lock(&balloon_mutex);
> +
> + balloon_stats.target_unpopulated += nr_pages;
> +
> while (pgno < nr_pages) {
> page = balloon_retrieve(true);
> if (page) {
> pages[pgno++] = page;
> } else {
> enum bp_state st;
> +
> + st = reserve_additional_memory();
> + if (st != BP_ECANCELED) {

...think if you use BP_ECANCELED in patch #6...

Daniel

2015-06-26 08:56:47

by David Vrabel

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCHv1 6/8] xen/balloon: only hotplug additional memory if required

On 25/06/15 22:18, Daniel Kiper wrote:
> On Thu, Jun 25, 2015 at 06:11:01PM +0100, David Vrabel wrote:
>> Now that we track the total number of pages (included hotplugged
>> regions), it is easy to determine if more memory needs to be
>> hotplugged.
>>
>> Signed-off-by: David Vrabel <[email protected]>
>> ---
>> drivers/xen/balloon.c | 16 +++++++++++++---
>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
>> index 960ac79..dd41da8 100644
>> --- a/drivers/xen/balloon.c
>> +++ b/drivers/xen/balloon.c
>> @@ -241,12 +241,22 @@ static void release_memory_resource(struct resource *resource)
>> * bit set). Real size of added memory is established at page onlining stage.
>> */
>>
>> -static enum bp_state reserve_additional_memory(long credit)
>> +static enum bp_state reserve_additional_memory(void)
>> {
>> + long credit;
>> struct resource *resource;
>> int nid, rc;
>> unsigned long balloon_hotplug;
>>
>> + credit = balloon_stats.target_pages - balloon_stats.total_pages;
>> +
>> + /*
>> + * Already hotplugged enough pages? Wait for them to be
>> + * onlined.
>> + */
>
> Comment is wrong or at least misleading. Both values does not depend on onlining.

If we get here and credit <=0 then the balloon is empty and we have
already hotplugged enough sections to reach target. We need to wait for
userspace to online the sections that already exist.

>> + if (credit <= 0)
>> + return BP_EAGAIN;
>
> Not BP_EAGAIN for sure. It should be BP_DONE but then balloon_process() will go
> into loop until memory is onlined at least up to balloon_stats.target_pages.
> BP_ECANCELED does work but it is misleading because it is not an error. So, maybe
> we should introduce BP_STOP (or something like that) which works like BP_ECANCELED
> and is not BP_ECANCELED.

We don't want to spin while waiting for userspace to online a new
section so BP_EAGAIN is correct here as it causes the balloon process to
be rescheduled at a later time.

David

2015-06-26 08:59:54

by David Vrabel

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCHv1 5/8] xen/balloon: rationalize memory hotplug stats

On 25/06/15 22:31, Daniel Kiper wrote:
> On Thu, Jun 25, 2015 at 08:54:45PM +0200, Daniel Kiper wrote:
>
> It looks that balloon_stats.total_pages is used only in memory hotplug case.
> Please do references (and definition) to it inside #ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
> like it is done in balloon_stats.hotplug_pages and balloon_stats.balloon_hotplug case.

I don't think the space saving here really warrants sprinkling a bunch
of #ifdefs around.

David

2015-06-26 12:47:06

by Daniel Kiper

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCHv1 6/8] xen/balloon: only hotplug additional memory if required

On Fri, Jun 26, 2015 at 09:56:31AM +0100, David Vrabel wrote:
> On 25/06/15 22:18, Daniel Kiper wrote:
> > On Thu, Jun 25, 2015 at 06:11:01PM +0100, David Vrabel wrote:
> >> Now that we track the total number of pages (included hotplugged
> >> regions), it is easy to determine if more memory needs to be
> >> hotplugged.
> >>
> >> Signed-off-by: David Vrabel <[email protected]>
> >> ---
> >> drivers/xen/balloon.c | 16 +++++++++++++---
> >> 1 file changed, 13 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> >> index 960ac79..dd41da8 100644
> >> --- a/drivers/xen/balloon.c
> >> +++ b/drivers/xen/balloon.c
> >> @@ -241,12 +241,22 @@ static void release_memory_resource(struct resource *resource)
> >> * bit set). Real size of added memory is established at page onlining stage.
> >> */
> >>
> >> -static enum bp_state reserve_additional_memory(long credit)
> >> +static enum bp_state reserve_additional_memory(void)
> >> {
> >> + long credit;
> >> struct resource *resource;
> >> int nid, rc;
> >> unsigned long balloon_hotplug;
> >>
> >> + credit = balloon_stats.target_pages - balloon_stats.total_pages;
> >> +
> >> + /*
> >> + * Already hotplugged enough pages? Wait for them to be
> >> + * onlined.
> >> + */
> >
> > Comment is wrong or at least misleading. Both values does not depend on onlining.
>
> If we get here and credit <=0 then the balloon is empty and we have

Right.

> already hotplugged enough sections to reach target. We need to wait for

OK.

> userspace to online the sections that already exist.

This is not true. You do not need to online sections to reserve new
memory region. Onlining does not change balloon_stats.target_pages
nor balloon_stats.total_pages. You must increase balloon_stats.target_pages
above balloon_stats.total_pages to reserve new memory region. And
balloon_stats.target_pages increase is not related to onlining.

> >> + if (credit <= 0)
> >> + return BP_EAGAIN;
> >
> > Not BP_EAGAIN for sure. It should be BP_DONE but then balloon_process() will go
> > into loop until memory is onlined at least up to balloon_stats.target_pages.
> > BP_ECANCELED does work but it is misleading because it is not an error. So, maybe
> > we should introduce BP_STOP (or something like that) which works like BP_ECANCELED
> > and is not BP_ECANCELED.
>
> We don't want to spin while waiting for userspace to online a new

Right.

> section so BP_EAGAIN is correct here as it causes the balloon process to
> be rescheduled at a later time.

And this is wrong. We do not want that balloon process wakes up and
looks for onlined pages. Onlinig may happen long time after memory
reservation. So, it means that until all needed sections are not onlined
then balloon process will be woken up for nothing (I assume that nobody
changes balloon_stats.target_pages). xen_memory_notifier() does work
for us. It wakes up balloon process after onlinig and then it can
do relevant work.

Daniel

2015-06-26 13:15:07

by David Vrabel

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCHv1 6/8] xen/balloon: only hotplug additional memory if required

On 26/06/15 13:46, Daniel Kiper wrote:
> On Fri, Jun 26, 2015 at 09:56:31AM +0100, David Vrabel wrote:
>> On 25/06/15 22:18, Daniel Kiper wrote:
>>> On Thu, Jun 25, 2015 at 06:11:01PM +0100, David Vrabel wrote:
>>>> Now that we track the total number of pages (included hotplugged
>>>> regions), it is easy to determine if more memory needs to be
>>>> hotplugged.
>>>>
>>>> Signed-off-by: David Vrabel <[email protected]>
>>>> ---
>>>> drivers/xen/balloon.c | 16 +++++++++++++---
>>>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
>>>> index 960ac79..dd41da8 100644
>>>> --- a/drivers/xen/balloon.c
>>>> +++ b/drivers/xen/balloon.c
>>>> @@ -241,12 +241,22 @@ static void release_memory_resource(struct resource *resource)
>>>> * bit set). Real size of added memory is established at page onlining stage.
>>>> */
>>>>
>>>> -static enum bp_state reserve_additional_memory(long credit)
>>>> +static enum bp_state reserve_additional_memory(void)
>>>> {
>>>> + long credit;
>>>> struct resource *resource;
>>>> int nid, rc;
>>>> unsigned long balloon_hotplug;
>>>>
>>>> + credit = balloon_stats.target_pages - balloon_stats.total_pages;
>>>> +
>>>> + /*
>>>> + * Already hotplugged enough pages? Wait for them to be
>>>> + * onlined.
>>>> + */
>>>
>>> Comment is wrong or at least misleading. Both values does not depend on onlining.
>>
>> If we get here and credit <=0 then the balloon is empty and we have
>
> Right.
>
>> already hotplugged enough sections to reach target. We need to wait for
>
> OK.
>
>> userspace to online the sections that already exist.
>
> This is not true. You do not need to online sections to reserve new
> memory region. Onlining does not change balloon_stats.target_pages
> nor balloon_stats.total_pages. You must increase balloon_stats.target_pages
> above balloon_stats.total_pages to reserve new memory region. And
> balloon_stats.target_pages increase is not related to onlining.

We don't want to keep adding sections if onlining the existing ones
would be sufficient to reach the target.

David