Re: [Xen-devel] [PATCH v3] memory_hotplug: Free pages as higher order
On 2018-09-27 12:41, Juergen Gross wrote:
On 27/09/18 08:58, Arun KS wrote:
When free pages are done with higher order, time spend on
coalescing pages by buddy allocator can be reduced. With
section size of 256MB, hot add latency of a single section
shows improvement from 50-60 ms to less than 1 ms, hence
improving the hot add latency by 60%.
Modify external providers of online callback to align with
the change.
Signed-off-by: Arun KS
---
Changes since v2:
reuse code from __free_pages_boot_core()
Changes since v1:
- Removed prefetch()
Changes since RFC:
- Rebase.
- As suggested by Michal Hocko remove pages_per_block.
- Modifed external providers of online_page_callback.
v2: https://lore.kernel.org/patchwork/patch/991363/
v1: https://lore.kernel.org/patchwork/patch/989445/
RFC: https://lore.kernel.org/patchwork/patch/984754/
---
drivers/hv/hv_balloon.c| 6 --
drivers/xen/balloon.c | 18 ++---
include/linux/memory_hotplug.h | 2 +-
mm/internal.h | 1 +
mm/memory_hotplug.c| 44
++
mm/page_alloc.c| 2 +-
6 files changed, 54 insertions(+), 19 deletions(-)
...
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index e12bb25..010cf4d 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -390,8 +390,8 @@ static enum bp_state
reserve_additional_memory(void)
/*
* add_memory_resource() will call online_pages() which in its turn
-* will call xen_online_page() callback causing deadlock if we don't
-* release balloon_mutex here. Unlocking here is safe because the
+* will call xen_bring_pgs_online() callback causing deadlock if we
+ * don't release balloon_mutex here. Unlocking here is safe because
the
* callers drop the mutex before trying again.
*/
mutex_unlock(_mutex);
@@ -422,6 +422,18 @@ static void xen_online_page(struct page *page)
mutex_unlock(_mutex);
}
+static int xen_bring_pgs_online(struct page *pg, unsigned int order)
+{
+ unsigned long i, size = (1 << order);
+ unsigned long start_pfn = page_to_pfn(pg);
+
+ pr_debug("Online %lu pages starting at pfn 0x%lx\n", size,
start_pfn);
+ for (i = 0; i < size; i++)
+ xen_online_page(pfn_to_page(start_pfn + i));
Hi,
xen_online_page() isn't very complex and this is the only user.
Why don't you move its body in here and drop the extra function?
And now you can execute the loop with balloon_mutex held instead of
taking and releasing it in each iteration of the loop.
Point taken. Will incorporate them.
Regards,
Arun
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] memory_hotplug: Free pages as higher order
On 2018-09-27 12:39, Oscar Salvador wrote: On Thu, Sep 27, 2018 at 12:28:50PM +0530, Arun KS wrote: + __free_pages_boot_core(page, order); Hi, I am not sure, but if we are going to use that function from the memory-hotplug code, we might want to rename that function to something more generic? The word "boot" suggests that this is only called from the boot stage. I ll rename it to __free_pages_core() And what about the prefetch operations? I saw that you removed them in your previous patch and that had some benefits [1]. Should we remove them here as well? Sure. Will update this as well. Thanks, Arun [1] https://patchwork.kernel.org/patch/10613359/ Thanks ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] memory_hotplug: Free pages as higher order
On 27/09/18 08:58, Arun KS wrote:
> When free pages are done with higher order, time spend on
> coalescing pages by buddy allocator can be reduced. With
> section size of 256MB, hot add latency of a single section
> shows improvement from 50-60 ms to less than 1 ms, hence
> improving the hot add latency by 60%.
>
> Modify external providers of online callback to align with
> the change.
>
> Signed-off-by: Arun KS
> ---
> Changes since v2:
> reuse code from __free_pages_boot_core()
>
> Changes since v1:
> - Removed prefetch()
>
> Changes since RFC:
> - Rebase.
> - As suggested by Michal Hocko remove pages_per_block.
> - Modifed external providers of online_page_callback.
>
> v2: https://lore.kernel.org/patchwork/patch/991363/
> v1: https://lore.kernel.org/patchwork/patch/989445/
> RFC: https://lore.kernel.org/patchwork/patch/984754/
>
> ---
> drivers/hv/hv_balloon.c| 6 --
> drivers/xen/balloon.c | 18 ++---
> include/linux/memory_hotplug.h | 2 +-
> mm/internal.h | 1 +
> mm/memory_hotplug.c| 44
> ++
> mm/page_alloc.c| 2 +-
> 6 files changed, 54 insertions(+), 19 deletions(-)
>
...
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index e12bb25..010cf4d 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -390,8 +390,8 @@ static enum bp_state reserve_additional_memory(void)
>
> /*
>* add_memory_resource() will call online_pages() which in its turn
> - * will call xen_online_page() callback causing deadlock if we don't
> - * release balloon_mutex here. Unlocking here is safe because the
> + * will call xen_bring_pgs_online() callback causing deadlock if we
> + * don't release balloon_mutex here. Unlocking here is safe because the
>* callers drop the mutex before trying again.
>*/
> mutex_unlock(_mutex);
> @@ -422,6 +422,18 @@ static void xen_online_page(struct page *page)
> mutex_unlock(_mutex);
> }
>
> +static int xen_bring_pgs_online(struct page *pg, unsigned int order)
> +{
> + unsigned long i, size = (1 << order);
> + unsigned long start_pfn = page_to_pfn(pg);
> +
> + pr_debug("Online %lu pages starting at pfn 0x%lx\n", size, start_pfn);
> + for (i = 0; i < size; i++)
> + xen_online_page(pfn_to_page(start_pfn + i));
xen_online_page() isn't very complex and this is the only user.
Why don't you move its body in here and drop the extra function?
And now you can execute the loop with balloon_mutex held instead of
taking and releasing it in each iteration of the loop.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] memory_hotplug: Free pages as higher order
On Thu, Sep 27, 2018 at 12:28:50PM +0530, Arun KS wrote: > + __free_pages_boot_core(page, order); I am not sure, but if we are going to use that function from the memory-hotplug code, we might want to rename that function to something more generic? The word "boot" suggests that this is only called from the boot stage. And what about the prefetch operations? I saw that you removed them in your previous patch and that had some benefits [1]. Should we remove them here as well? [1] https://patchwork.kernel.org/patch/10613359/ Thanks -- Oscar Salvador SUSE L3 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
