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 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [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 david.vra...@citrix.com
---
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;
Re: [Xen-devel] [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 david.vra...@citrix.com 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 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [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 david.vra...@citrix.com 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 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [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 david.vra...@citrix.com 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 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
