memory_hotplug: Free pages as higher order

PATCH Superseded Arun KS 3 participants 10 messages Nov 6, 2018 – Jan 4, 2019

Subsystems (6)

Memory management MM core Page allocator Xen hypervisor interface +2 more subsystems

lore

Share & tools

lore

Arun KS

author

2018-11-06 06:03 UTC #0 lore raw

PATCH v6 1/2 memory_hotplug: Free pages as higher order

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.

This patch modifies totalram_pages, zone->managed_pages and totalhigh_pages outside managed_page_count_lock. A follow up series will be send to convert these variable to atomic to avoid readers potentially seeing a store tear.

Signed-off-by Arun KS <[email protected]>

---
Changes since v5:
- Rebased to 4.20-rc1.
- Changelog updated.

Changes since v4:
- As suggested by Michal Hocko,
- Simplify logic in online_pages_block() by using get_order().
- Seperate out removal of prefetch from __free_pages_core().

Changes since v3:
- Renamed _free_pages_boot_core -> __free_pages_core.
- Removed prefetch from __free_pages_core.
- Removed xen_online_page().

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.

v5: https://lore.kernel.org/patchwork/patch/995739/
v4: https://lore.kernel.org/patchwork/patch/995111/
v3: https://lore.kernel.org/patchwork/patch/992348/
v2: https://lore.kernel.org/patchwork/patch/991363/
v1: https://lore.kernel.org/patchwork/patch/989445/
RFC: https://lore.kernel.org/patchwork/patch/984754/

---

Signed-off-by Arun KS <[email protected]>

---
 drivers/hv/hv_balloon.c        |  6 ++++--
 drivers/xen/balloon.c          | 23 +++++++++++++++--------
 include/linux/memory_hotplug.h |  2 +-
 mm/internal.h                  |  1 +
 mm/memory_hotplug.c            | 42 ++++++++++++++++++++++++++++++------------
 mm/page_alloc.c                |  8 ++++----
 6 files changed, 55 insertions(+), 27 deletions(-)

drivers/hv/hv_balloon.c +4 -2

index 4163151..5728dc4 100644
@@ -771,7 +771,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
 	}
 }
 
-static void hv_online_page(struct page *pg)
+static int hv_online_page(struct page *pg, unsigned int order)
 {
 	struct hv_hotadd_state *has;
 	unsigned long flags;

@@ -783,10 +783,12 @@ static void hv_online_page(struct page *pg)
 		if ((pfn < has->start_pfn) || (pfn >= has->end_pfn))
 			continue;
 
-		hv_page_online_one(has, pg);
+		hv_bring_pgs_online(has, pfn, (1UL << order));
 		break;
 	}
 	spin_unlock_irqrestore(&dm_device.ha_lock, flags);
+
+	return 0;
 }
 
 static int pfn_covered(unsigned long start_pfn, unsigned long pfn_cnt)

drivers/xen/balloon.c +15 -8

index fdfc64f..1214828 100644
@@ -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(&balloon_mutex);

@@ -414,15 +414,22 @@ static enum bp_state reserve_additional_memory(void)
 	return BP_ECANCELED;
 }
 
-static void xen_online_page(struct page *page)
+static int xen_bring_pgs_online(struct page *pg, unsigned int order)
 {
-	__online_page_set_limits(page);
+	unsigned long i, size = (1 << order);
+	unsigned long start_pfn = page_to_pfn(pg);
+	struct page *p;
 
+	pr_debug("Online %lu pages starting at pfn 0x%lx\n", size, start_pfn);
 	mutex_lock(&balloon_mutex);
-
-	__balloon_append(page);
-
+	for (i = 0; i < size; i++) {
+		p = pfn_to_page(start_pfn + i);
+		__online_page_set_limits(p);
+		__balloon_append(p);
+	}
 	mutex_unlock(&balloon_mutex);
+
+	return 0;
 }
 
 static int xen_memory_notifier(struct notifier_block *nb, unsigned long val, void *v)

@@ -747,7 +754,7 @@ static int __init balloon_init(void)
 	balloon_stats.max_retry_count = RETRY_UNLIMITED;
 
 #ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
-	set_online_page_callback(&xen_online_page);
+	set_online_page_callback(&xen_bring_pgs_online);
 	register_memory_notifier(&xen_memory_nb);
 	register_sysctl_table(xen_root);

include/linux/memory_hotplug.h +1 -1

index ffd9cd1..84e9ae2 100644
@@ -87,7 +87,7 @@ extern int test_pages_in_a_zone(unsigned long start_pfn, unsigned long end_pfn,
 	unsigned long *valid_start, unsigned long *valid_end);
 extern void __offline_isolated_pages(unsigned long, unsigned long);
 
-typedef void (*online_page_callback_t)(struct page *page);
+typedef int (*online_page_callback_t)(struct page *page, unsigned int order);
 
 extern int set_online_page_callback(online_page_callback_t callback);
 extern int restore_online_page_callback(online_page_callback_t callback);

mm/internal.h +1 -0

index 291eb2b..3b1ec14 100644
@@ -163,6 +163,7 @@ static inline struct page *pageblock_pfn_to_page(unsigned long start_pfn,
 extern int __isolate_free_page(struct page *page, unsigned int order);
 extern void memblock_free_pages(struct page *page, unsigned long pfn,
 					unsigned int order);
+extern void __free_pages_core(struct page *page, unsigned int order);
 extern void prep_compound_page(struct page *page, unsigned int order);
 extern void post_alloc_hook(struct page *page, unsigned int order,
 					gfp_t gfp_flags);

mm/memory_hotplug.c +30 ~~-12~~

63 lines not on this page · raw message

mm/page_alloc.c +4 -4

index a919ba5..7cf503f 100644
@@ -1264,7 +1264,7 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	local_irq_restore(flags);
 }
 
-static void __init __free_pages_boot_core(struct page *page, unsigned int order)
+void __free_pages_core(struct page *page, unsigned int order)
 {
 	unsigned int nr_pages = 1 << order;
 	struct page *p = page;

@@ -1343,7 +1343,7 @@ void __init memblock_free_pages(struct page *page, unsigned long pfn,
 {
 	if (early_page_uninitialised(pfn))
 		return;
-	return __free_pages_boot_core(page, order);
+	return __free_pages_core(page, order);
 }
 
 /*

@@ -1433,14 +1433,14 @@ static void __init deferred_free_range(unsigned long pfn,
 	if (nr_pages == pageblock_nr_pages &&
 	    (pfn & (pageblock_nr_pages - 1)) == 0) {
 		set_pageblock_migratetype(page, MIGRATE_MOVABLE);
-		__free_pages_boot_core(page, pageblock_order);
+		__free_pages_core(page, pageblock_order);
 		return;
 	}
 
 	for (i = 0; i < nr_pages; i++, page++, pfn++) {
 		if ((pfn & (pageblock_nr_pages - 1)) == 0)
 			set_pageblock_migratetype(page, MIGRATE_MOVABLE);
-		__free_pages_boot_core(page, 0);
+		__free_pages_core(page, 0);
 	}
 }

Signature · 1 line

1.9.1

Arun KS

author

2018-11-06 06:03 UTC #1 lore raw

PATCH v6 2/2 mm/page_alloc: remove software prefetching in __free_pages_core

They not only increase the code footprint, they actually make things slower rather than faster. Remove them as contemporary hardware doesn't need any hint.

Signed-off-by Arun KS <[email protected]>

---
 mm/page_alloc.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

mm/page_alloc.c +1 -5

index 7cf503f..a1b9a6a 100644
@@ -1270,14 +1270,10 @@ void __free_pages_core(struct page *page, unsigned int order)
 	struct page *p = page;
 	unsigned int loop;
 
-	prefetchw(p);
-	for (loop = 0; loop < (nr_pages - 1); loop++, p++) {
-		prefetchw(p + 1);
+	for (loop = 0; loop < nr_pages ; loop++, p++) {
 		__ClearPageReserved(p);
 		set_page_count(p, 0);
 	}
-	__ClearPageReserved(p);
-	set_page_count(p, 0);
 
 	page_zone(page)->managed_pages += nr_pages;
 	set_page_refcounted(page);

Signature · 1 line

1.9.1

Michal Hocko

2018-11-06 14:08 UTC #2 lore raw

Re: [PATCH v6 2/2] mm/page_alloc: remove software prefetching in __free_pages_core

Quoted 3 lines · Arun KS

They not only increase the code footprint, they actually make things slower rather than faster. Remove them as contemporary hardware doesn't need any hint.

I guess I have already asked for that. When you argue about performance then always add some numbers.

I do agree we want to get rid of the prefetching because it is just too of an micro-optimization without any reasonable story behind.

Quoted 28 lines

Signed-off-by Arun KS <[email protected]>

---
 mm/page_alloc.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

mm/page_alloc.c +1 -5

index 7cf503f..a1b9a6a 100644
@@ -1270,14 +1270,10 @@ void __free_pages_core(struct page *page, unsigned int order)
 	struct page *p = page;
 	unsigned int loop;
 
-	prefetchw(p);
-	for (loop = 0; loop < (nr_pages - 1); loop++, p++) {
-		prefetchw(p + 1);
+	for (loop = 0; loop < nr_pages ; loop++, p++) {
 		__ClearPageReserved(p);
 		set_page_count(p, 0);
 	}
-	__ClearPageReserved(p);
-	set_page_count(p, 0);
 
 	page_zone(page)->managed_pages += nr_pages;
 	set_page_refcounted(page);

Signature · 1 line

1.9.1

Signature · 2 lines

Michal Hocko
SUSE Labs

Michal Hocko

2018-11-06 14:06 UTC #3 lore raw

In reply to Arun KS

Quoted 11 lines · Arun KS

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.
This patch modifies totalram_pages, zone->managed_pages and totalhigh_pages outside managed_page_count_lock. A follow up series will be send to convert these variable to atomic to avoid readers potentially seeing a store tear.

Is there any reason to rush this through rather than wait for counters conversion first?

The patch as is looks good to me - modulo atomic counters of course. I cannot really judge whether existing updaters do really race in practice to take this riskless.

The improvement is nice of course but this is a rare operation and 50ms vs 1ms is hardly noticeable. So I would rather wait for the preparatory work to settle. Btw. is there anything blocking that? It seems to be mostly automated.

Signature · 2 lines

Michal Hocko
SUSE Labs

Arun KS

author

2018-11-06 15:31 UTC #4 lore raw

Quoted 8 lines · Michal Hocko

Quoted 7 lines · Arun KS
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.

Quoted 7 lines

Quoted 4 lines
This patch modifies totalram_pages, zone->managed_pages and totalhigh_pages outside managed_page_count_lock. A follow up series will be send to convert these variable to atomic to avoid readers potentially seeing a store tear.
Is there any reason to rush this through rather than wait for counters conversion first?

Sure Michal.

Conversion patch, https://patchwork.kernel.org/cover/10657217/ is currently incremental to this patch. I ll change the order. Will wait for preparatory patch to settle first.

Regards,
Arun.

Quoted 9 lines

The patch as is looks good to me - modulo atomic counters of course. I cannot really judge whether existing updaters do really race in practice to take this riskless.
The improvement is nice of course but this is a rare operation and 50ms vs 1ms is hardly noticeable. So I would rather wait for the preparatory work to settle. Btw. is there anything blocking that? It seems to be mostly automated.

Michal Hocko

2018-11-06 20:08 UTC #5 lore raw

Quoted 14 lines · Arun KS

Quoted 13 lines · Michal Hocko
Quoted 11 lines · Arun KS
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.
This patch modifies totalram_pages, zone->managed_pages and totalhigh_pages outside managed_page_count_lock. A follow up series will be send to convert these variable to atomic to avoid readers potentially seeing a store tear.

Quoted 7 lines

Quoted 2 lines
Is there any reason to rush this through rather than wait for counters conversion first?
Sure Michal.
Conversion patch, https://patchwork.kernel.org/cover/10657217/ is currently incremental to this patch.

The ordering should be other way around. Because as things stand with this patch first it is possible to introduce a subtle race prone updates. As I've said I am skeptical the race would matter, really, but there is no real reason to risk for that. Especially when you have the other (first) half ready.

Signature · 2 lines

Michal Hocko
SUSE Labs

Arun KS

author

2018-11-07 06:21 UTC #6 lore raw

Quoted 20 lines · Michal Hocko

Quoted 19 lines · Arun KS
Quoted 8 lines · Michal Hocko
Quoted 7 lines · Arun KS
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.
Quoted 7 lines
Quoted 4 lines
This patch modifies totalram_pages, zone->managed_pages and totalhigh_pages outside managed_page_count_lock. A follow up series will be send to convert these variable to atomic to avoid readers potentially seeing a store tear.
Is there any reason to rush this through rather than wait for counters conversion first?
Sure Michal.

Quoted 8 lines

Quoted 2 lines
Conversion patch, https://patchwork.kernel.org/cover/10657217/ is currently incremental to this patch.
The ordering should be other way around. Because as things stand with this patch first it is possible to introduce a subtle race prone updates. As I've said I am skeptical the race would matter, really, but there is no real reason to risk for that. Especially when you have the other (first) half ready.

Makes sense. I have rebased the preparatory patch on top of -rc1.
https://patchwork.kernel.org/patch/10670787/

Regards,
Arun

Arun KS

author

2019-01-04 05:06 UTC #7 lore raw

Quoted 24 lines · Arun KS

Quoted 23 lines · Michal Hocko
Quoted 21 lines · Arun KS
Quoted 8 lines · Michal Hocko
Quoted 7 lines · Arun KS
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.
Quoted 7 lines
Quoted 4 lines
This patch modifies totalram_pages, zone->managed_pages and totalhigh_pages outside managed_page_count_lock. A follow up series will be send to convert these variable to atomic to avoid readers potentially seeing a store tear.
Is there any reason to rush this through rather than wait for counters conversion first?
Sure Michal.
Conversion patch, https://patchwork.kernel.org/cover/10657217/ is currently incremental to this patch.

Quoted 8 lines

Quoted 5 lines
The ordering should be other way around. Because as things stand with this patch first it is possible to introduce a subtle race prone updates. As I've said I am skeptical the race would matter, really, but there is no real reason to risk for that. Especially when you have the other (first) half ready.
Makes sense. I have rebased the preparatory patch on top of -rc1.
https://patchwork.kernel.org/patch/10670787/

Hello Michal,

Please review version 7 sent,
https://lore.kernel.org/patchwork/patch/1028908/

Regards,
Arun

Quoted 3 lines

Regards,
Arun

Michal Hocko

2019-01-04 08:58 UTC #8 lore raw

Quoted 33 lines · Arun KS

Quoted 32 lines · Arun KS
Quoted 30 lines · Michal Hocko
Quoted 14 lines · Arun KS
Quoted 13 lines · Michal Hocko
Quoted 11 lines · Arun KS
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.
This patch modifies totalram_pages, zone->managed_pages and totalhigh_pages outside managed_page_count_lock. A follow up series will be send to convert these variable to atomic to avoid readers potentially seeing a store tear.
Quoted 8 lines
Quoted 2 lines
Is there any reason to rush this through rather than wait for counters conversion first?
Sure Michal.
Conversion patch, https://patchwork.kernel.org/cover/10657217/
is currently
incremental to this patch.
The ordering should be other way around. Because as things stand with this patch first it is possible to introduce a subtle race prone updates. As I've said I am skeptical the race would matter, really, but
there is no real reason to risk for that. Especially when you have the other (first) half ready.

Quoted 7 lines

Quoted 2 lines
Makes sense. I have rebased the preparatory patch on top of -rc1.
https://patchwork.kernel.org/patch/10670787/
Hello Michal,
Please review version 7 sent,
https://lore.kernel.org/patchwork/patch/1028908/

I believe I have give my Acked-by to this version already, and v7 indeed has it. Are there any relevant changes since v6 for me to do the review again. If yes you should have dropped the Acked-by.

Signature · 2 lines

Michal Hocko
SUSE Labs

Arun Sudhilal

2019-01-04 09:01 UTC #9 lore raw

Quoted 38 lines · Michal Hocko

Quoted 36 lines · Arun KS
Quoted 34 lines · Arun KS
Quoted 24 lines · Michal Hocko
Quoted 14 lines · Arun KS
Quoted 13 lines · Michal Hocko
Quoted 11 lines · Arun KS
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.
This patch modifies totalram_pages, zone->managed_pages and totalhigh_pages outside managed_page_count_lock. A follow up series will be send to convert these variable to atomic to avoid readers potentially seeing a store tear.
Quoted 8 lines
Quoted 2 lines
Is there any reason to rush this through rather than wait for counters conversion first?
Sure Michal.
Conversion patch, https://patchwork.kernel.org/cover/10657217/
is currently
incremental to this patch.
Quoted 6 lines
The ordering should be other way around. Because as things stand with this patch first it is possible to introduce a subtle race prone updates. As I've said I am skeptical the race would matter, really, but
there is no real reason to risk for that. Especially when you have the other (first) half ready.
Makes sense. I have rebased the preparatory patch on top of -rc1.
https://patchwork.kernel.org/patch/10670787/

Quoted 8 lines

Quoted 4 lines
Hello Michal,
Please review version 7 sent,
https://lore.kernel.org/patchwork/patch/1028908/
I believe I have give my Acked-by to this version already, and v7 indeed has it. Are there any relevant changes since v6 for me to do the review again. If yes you should have dropped the Acked-by.

No Michal. Patch is same. Only difference is this patch is re-based on top of preparatory patch.
https://patchwork.kernel.org/patch/10670787/

Regards,
Arun

Quoted 3 lines

--
Michal Hocko
SUSE Labs