memory_hotplug: Free pages as pageblock_order

RFC In review Arun KS 3 participants 8 messages Sep 12, 2018 – Sep 19, 2018

Subsystems (2)

Memory management Memory hot(un)plug

lore

Share & tools

lore

Arun KS

author

2018-09-12 09:27 UTC #0 lore raw

RFC 1/1 memory_hotplug: Free pages as pageblock_order

When free pages are done with pageblock_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%.

If this looks okey, I'll modify users of set_online_page_callback and resend clean patch.

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

---
 include/linux/memory_hotplug.h |  1 +
 mm/memory_hotplug.c            | 52 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 46 insertions(+), 7 deletions(-)

include/linux/memory_hotplug.h +1 -0

index 34a2822..447047d 100644
@@ -88,6 +88,7 @@ extern int test_pages_in_a_zone(unsigned long start_pfn, unsigned long end_pfn,
 extern void __offline_isolated_pages(unsigned long, unsigned long);
 
 typedef void (*online_page_callback_t)(struct page *page);
+typedef int (*online_pages_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/memory_hotplug.c +45 -7

64 lines not on this page · raw message

Signature · 1 line

1.9.1

Michal Hocko

2018-09-12 10:38 UTC #1 lore raw

Quoted 5 lines · Arun KS

When free pages are done with pageblock_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%.

Where does the improvement come from? You are still doing the same amount of work except that the number of callbacks is lower. Is this the real source of 60% improvement?

Quoted 3 lines

If this looks okey, I'll modify users of set_online_page_callback and resend clean patch.

[...]

Quoted 26 lines

mm/memory_hotplug.c +26 -0

+static int generic_online_pages(struct page *page, unsigned int order);
+static online_pages_callback_t online_pages_callback = generic_online_pages;
+
+static int generic_online_pages(struct page *page, unsigned int order)
+{
+	unsigned long nr_pages = 1 << order;
+	struct page *p = page;
+	unsigned int loop;
+
+	for (loop = 0 ; loop < nr_pages ; loop++, p++) {
+		__ClearPageReserved(p);
+		set_page_count(p, 0);
+	}
+	adjust_managed_page_count(page, nr_pages);
+	init_page_count(page);
+	__free_pages(page, order);
+
+	return 0;
+}
+
+static int online_pages_blocks(unsigned long start_pfn, unsigned long nr_pages)
+{
+	unsigned long pages_per_block = (1 << pageblock_order);
+	unsigned long nr_pageblocks = nr_pages / pages_per_block;
+//	unsigned long rem_pages = nr_pages % pages_per_block;
+	int i, ret, onlined_pages = 0;

mm/memory_hotplug.c static void generic_online_page(struct page *page) · line 700

Quoted 10 lines

mm/memory_hotplug.c +10 -0

+	struct page *page;
+
+	for (i = 0 ; i < nr_pageblocks ; i++) {
+		page = pfn_to_page(start_pfn + (i * pages_per_block));
+		ret = (*online_pages_callback)(page, pageblock_order);
+		if (!ret)
+			onlined_pages += pages_per_block;
+		else if (ret > 0)
+			onlined_pages += ret;
+	}

Could you explain why does the pages_per_block step makes any sense? Why don't you simply apply handle the full nr_pages worth of memory range instead?

Quoted 28 lines

mm/memory_hotplug.c +9 -7

+/*
+	if (rem_pages)
+		onlined_pages += online_page_single(start_pfn + i, rem_pages);
+*/
+
+	return onlined_pages;
+}
+
 static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
 			void *arg)
 {
-	unsigned long i;
 	unsigned long onlined_pages = *(unsigned long *)arg;
-	struct page *page;
 
 	if (PageReserved(pfn_to_page(start_pfn)))
-		for (i = 0; i < nr_pages; i++) {
-			page = pfn_to_page(start_pfn + i);
-			(*online_page_callback)(page);
-			onlined_pages++;
-		}
+		onlined_pages = online_pages_blocks(start_pfn, nr_pages);
 
 	online_mem_sections(start_pfn, start_pfn + nr_pages);

Signature · 2 lines

-- 
1.9.1

Signature · 2 lines

Michal Hocko
SUSE Labs

Balbir Singh

2018-09-12 12:57 UTC #2 lore raw

Quoted 11 lines · Michal Hocko

Quoted 5 lines · Arun KS
When free pages are done with pageblock_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%.
Where does the improvement come from? You are still doing the same amount of work except that the number of callbacks is lower. Is this the real source of 60% improvement?

It looks like only the first page of the pageblock is initialized, is some of the cost amortized in terms of doing one initialization for the page with order (order) and then relying on split_page and helpers to do the rest? Of course the number of callbacks reduce by a significant number as well.

Quoted 47 lines

Quoted 3 lines

If this looks okey, I'll modify users of set_online_page_callback and resend clean patch.

[...]

Quoted 26 lines

mm/memory_hotplug.c +26 -0

+static int generic_online_pages(struct page *page, unsigned int order);
+static online_pages_callback_t online_pages_callback = generic_online_pages;
+
+static int generic_online_pages(struct page *page, unsigned int order)
+{
+	unsigned long nr_pages = 1 << order;
+	struct page *p = page;
+	unsigned int loop;
+
+	for (loop = 0 ; loop < nr_pages ; loop++, p++) {
+		__ClearPageReserved(p);
+		set_page_count(p, 0);
+	}
+	adjust_managed_page_count(page, nr_pages);
+	init_page_count(page);
+	__free_pages(page, order);
+
+	return 0;
+}
+
+static int online_pages_blocks(unsigned long start_pfn, unsigned long nr_pages)
+{
+	unsigned long pages_per_block = (1 << pageblock_order);
+	unsigned long nr_pageblocks = nr_pages / pages_per_block;
+//	unsigned long rem_pages = nr_pages % pages_per_block;
+	int i, ret, onlined_pages = 0;

mm/memory_hotplug.c static void generic_online_page(struct page *page) · line 700

Quoted 10 lines

mm/memory_hotplug.c +10 -0

+	struct page *page;
+
+	for (i = 0 ; i < nr_pageblocks ; i++) {
+		page = pfn_to_page(start_pfn + (i * pages_per_block));
+		ret = (*online_pages_callback)(page, pageblock_order);
+		if (!ret)
+			onlined_pages += pages_per_block;
+		else if (ret > 0)
+			onlined_pages += ret;
+	}

Could you explain why does the pages_per_block step makes any sense? Why don't you simply apply handle the full nr_pages worth of memory range instead?

Quoted 4 lines

mm/memory_hotplug.c static void generic_online_page(struct page *page) · line 704
Quoted 4 lines
mm/memory_hotplug.c +4 -0
+/*
+	if (rem_pages)
+		onlined_pages += online_page_single(start_pfn + i, rem_pages);
+*/

Do we expect no rem_pages with this patch?

Quoted 19 lines

mm/memory_hotplug.c +5 -7

+
+	return onlined_pages;
+}
+
 static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
 			void *arg)
 {
-	unsigned long i;
 	unsigned long onlined_pages = *(unsigned long *)arg;
-	struct page *page;
 
 	if (PageReserved(pfn_to_page(start_pfn)))
-		for (i = 0; i < nr_pages; i++) {
-			page = pfn_to_page(start_pfn + i);
-			(*online_page_callback)(page);
-			onlined_pages++;
-		}
+		onlined_pages = online_pages_blocks(start_pfn, nr_pages);

Quoted 1 line

Quoted 1 line
online_mem_sections(start_pfn, start_pfn + nr_pages);

Balbir Singh.

Michal Hocko

2018-09-12 13:17 UTC #3 lore raw

Quoted 13 lines · Balbir Singh

Quoted 11 lines · Michal Hocko
Quoted 5 lines · Arun KS
When free pages are done with pageblock_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%.
Where does the improvement come from? You are still doing the same amount of work except that the number of callbacks is lower. Is this the real source of 60% improvement?

Quoted 5 lines

It looks like only the first page of the pageblock is initialized, is some of the cost amortized in terms of doing one initialization for the page with order (order) and then relying on split_page and helpers to do the rest? Of course the number of callbacks reduce by a significant number as well.

Ohh, I have missed that part. Now when re-reading I can see the reason for the perf improvement. It is most likely the higher order free which ends up being much cheaper. This part makes some sense.

How much is this feasible is another question. Do not forget we have those external providers of the online callback and those would need to be updated as well.

Btw. the normal memmap init code path does the same per-page free as well. If we really want to speed the hotplug path then I guess the init one would see a bigger improvement and those two should be in sync.

Quoted 6 lines

Quoted 6 lines
Quoted 3 lines
If this looks okey, I'll modify users of set_online_page_callback and resend clean patch.
[...]

Quoted 12 lines

Quoted 4 lines

mm/memory_hotplug.c +4 -0

+static int generic_online_pages(struct page *page, unsigned int order);
+static online_pages_callback_t online_pages_callback = generic_online_pages;
+
+static int generic_online_pages(struct page *page, unsigned int order)

Quoted 8 lines

mm/memory_hotplug.c static void generic_online_page(struct page *page) · line 676
Quoted 8 lines
mm/memory_hotplug.c +8 -0
+{
+	unsigned long nr_pages = 1 << order;
+	struct page *p = page;
+	unsigned int loop;
+
+	for (loop = 0 ; loop < nr_pages ; loop++, p++) {
+		__ClearPageReserved(p);
+		set_page_count(p, 0);

btw. you want init_page_count here.

Signature · 2 lines

Michal Hocko
SUSE Labs

Arun KS

author

2018-09-12 14:42 UTC #4 lore raw

Quoted 20 lines · Michal Hocko

Quoted 13 lines · Balbir Singh
Quoted 11 lines · Michal Hocko
Quoted 5 lines · Arun KS
When free pages are done with pageblock_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%.
Where does the improvement come from? You are still doing the same amount of work except that the number of callbacks is lower. Is this the real source of 60% improvement?
Quoted 5 lines
It looks like only the first page of the pageblock is initialized, is some of the cost amortized in terms of doing one initialization for the page with order (order) and then relying on split_page and helpers to do the rest? Of course the number of callbacks reduce by a significant number as well.

Quoted 7 lines

Ohh, I have missed that part. Now when re-reading I can see the reason for the perf improvement. It is most likely the higher order free which ends up being much cheaper. This part makes some sense.
How much is this feasible is another question. Do not forget we have those external providers of the online callback and those would need to be updated as well.

Sure Michal, I ll look into this.

Quoted 4 lines

Btw. the normal memmap init code path does the same per-page free as well. If we really want to speed the hotplug path then I guess the init one would see a bigger improvement and those two should be in sync.

Thanks for pointers, Will look further.

Quoted 7 lines

Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
If this looks okey, I'll modify users of set_online_page_callback and resend clean patch.
[...]

Quoted 14 lines

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
mm/memory_hotplug.c +4 -0
+static int generic_online_pages(struct page *page, unsigned int order);
+static online_pages_callback_t online_pages_callback = generic_online_pages;
+
+static int generic_online_pages(struct page *page, unsigned int order)
Quoted 8 lines
Quoted 8 lines
mm/memory_hotplug.c static void generic_online_page(struct page *page) · line 676
Quoted 8 lines
mm/memory_hotplug.c +8 -0
+{
+	unsigned long nr_pages = 1 << order;
+	struct page *p = page;
+	unsigned int loop;
+
+	for (loop = 0 ; loop < nr_pages ; loop++, p++) {
+		__ClearPageReserved(p);
+		set_page_count(p, 0);
btw. you want init_page_count here.

Do you mean replace set_page_count(p, 0) with init_page_count(page)? Because init_page_count is setting the page _refcount to 1

static inline void init_page_count(struct page *page)

{
        set_page_count(page, 1);
}

I thought in case of higher order pages only the first struct page should have _refcount to 1 before calling __free_pages(). Please correct me if wrong.

Regards,
Arun

Michal Hocko

2018-09-14 09:11 UTC #5 lore raw

Quoted 29 lines · Arun KS

Quoted 21 lines · Michal Hocko
Quoted 13 lines · Balbir Singh
Quoted 11 lines · Michal Hocko
Quoted 5 lines · Arun KS
When free pages are done with pageblock_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%.
Where does the improvement come from? You are still doing the same amount of work except that the number of callbacks is lower. Is this the real source of 60% improvement?
Quoted 6 lines
It looks like only the first page of the pageblock is initialized, is some of the cost amortized in terms of doing one initialization for the page with order (order) and then relying on split_page and helpers to do the rest? Of course the number of callbacks reduce by a significant
number as well.
Quoted 7 lines
Ohh, I have missed that part. Now when re-reading I can see the reason for the perf improvement. It is most likely the higher order free which ends up being much cheaper. This part makes some sense.
How much is this feasible is another question. Do not forget we have those external providers of the online callback and those would need to be updated as well.

Quoted 7 lines

Sure Michal, I ll look into this.
Quoted 4 lines
Btw. the normal memmap init code path does the same per-page free as well. If we really want to speed the hotplug path then I guess the init one would see a bigger improvement and those two should be in sync.
Thanks for pointers, Will look further.

I haven't looked closer and I will be travelling next week so just hint. Have a look at the nobootmem and how it frees pages to the page allocator in __free_pages_boot_core. Seems exactly what you want and it also answers your question about reference counting.

Signature · 2 lines

Michal Hocko
SUSE Labs

Arun KS

author

2018-09-19 01:18 UTC #6 lore raw

Quoted 36 lines · Michal Hocko

Quoted 35 lines · Arun KS
Quoted 21 lines · Michal Hocko
Quoted 13 lines · Balbir Singh
Quoted 11 lines · Michal Hocko
Quoted 5 lines · Arun KS
When free pages are done with pageblock_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%.
Where does the improvement come from? You are still doing the same amount of work except that the number of callbacks is lower. Is this the real source of 60% improvement?
Quoted 6 lines
It looks like only the first page of the pageblock is initialized, is some of the cost amortized in terms of doing one initialization for the page with order (order) and then relying on split_page and helpers to do the rest? Of course the number of callbacks reduce by a significant
number as well.
Quoted 7 lines
Ohh, I have missed that part. Now when re-reading I can see the reason for the perf improvement. It is most likely the higher order free which ends up being much cheaper. This part makes some sense.
How much is this feasible is another question. Do not forget we have those external providers of the online callback and those would need to be updated as well.
Sure Michal, I ll look into this.
Quoted 4 lines
Btw. the normal memmap init code path does the same per-page free as well. If we really want to speed the hotplug path then I guess the init one would see a bigger improvement and those two should be in sync.

Quoted 6 lines

Quoted 1 line
Thanks for pointers, Will look further.
I haven't looked closer and I will be travelling next week so just hint. Have a look at the nobootmem and how it frees pages to the page allocator in __free_pages_boot_core. Seems exactly what you want and it also answers your question about reference counting.

Thanks Michal. Will send a new version after testing.

Regards,
Arun

Arun KS

author

2018-09-12 14:09 UTC #7 lore raw

In reply to Balbir Singh

Hello Michal and Balbir,

Thanks for reviewing.

Quoted 13 lines · Balbir Singh

Quoted 11 lines · Michal Hocko
Quoted 5 lines · Arun KS
When free pages are done with pageblock_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%.
Where does the improvement come from? You are still doing the same amount of work except that the number of callbacks is lower. Is this the real source of 60% improvement?

Quoted 5 lines

It looks like only the first page of the pageblock is initialized, is some of the cost amortized in terms of doing one initialization for the page with order (order) and then relying on split_page and helpers to do the rest? Of course the number of callbacks reduce by a significant number as well.

Currently, order zero pages are freed one by one, they goes to pcp list and later when pcp->count >= pcp->high, kernel calls __free_one_page() in a loop. __free_one_page() tries to merge these pages to create bigger order page.

But when we free with higher order page(pageblock_order), this merging is not done. AFAIU, this is the reason for improvement in hot add latency.

Quoted 40 lines

Quoted 5 lines

If this looks okey, I'll modify users of set_online_page_callback and resend clean patch.

[...]

Quoted 32 lines

mm/memory_hotplug.c +32 -0

+static int generic_online_pages(struct page *page, unsigned int order);
+static online_pages_callback_t online_pages_callback = generic_online_pages;
+
+static int generic_online_pages(struct page *page, unsigned int order)
+{
+	unsigned long nr_pages = 1 << order;
+	struct page *p = page;
+	unsigned int loop;
+
+	for (loop = 0 ; loop < nr_pages ; loop++, p++) {
+		__ClearPageReserved(p);
+		set_page_count(p, 0);
+	}
+	adjust_managed_page_count(page, nr_pages);
+	init_page_count(page);
+	__free_pages(page, order);
+
+	return 0;
+}
+
+static int online_pages_blocks(unsigned long start_pfn, unsigned long nr_pages)
+{
+	unsigned long pages_per_block = (1 << pageblock_order);
+	unsigned long nr_pageblocks = nr_pages / pages_per_block;
+//	unsigned long rem_pages = nr_pages % pages_per_block;
+	int i, ret, onlined_pages = 0;
+	struct page *page;
+
+	for (i = 0 ; i < nr_pageblocks ; i++) {
+		page = pfn_to_page(start_pfn + (i * pages_per_block));
+		ret = (*online_pages_callback)(page, pageblock_order);
+		if (!ret)

Quoted 8 lines

Quoted 8 lines
mm/memory_hotplug.c static void generic_online_page(struct page *page) · line 700
Quoted 4 lines
mm/memory_hotplug.c +4 -0
+			onlined_pages += pages_per_block;
+		else if (ret > 0)
+			onlined_pages += ret;
+	}
Could you explain why does the pages_per_block step makes any sense? Why don't you simply apply handle the full nr_pages worth of memory range instead?

Yes. We can move the this loop to generic_online_pages and do __free_pages() of pageblock_order.

Quoted 7 lines

Quoted 5 lines
mm/memory_hotplug.c static void generic_online_page(struct page *page) · line 704
Quoted 5 lines
mm/memory_hotplug.c +4 -0
+/*
+	if (rem_pages)
+		onlined_pages += online_page_single(start_pfn + i, rem_pages);
+*/
Do we expect no rem_pages with this patch?

I ll remove this code, in assumption that section size will be always multiple of pageblock_order.

Regards,
Arun

Quoted 24 lines

Quoted 20 lines

mm/memory_hotplug.c +5 -7

+
+	return onlined_pages;
+}
+
 static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
 			void *arg)
 {
-	unsigned long i;
 	unsigned long onlined_pages = *(unsigned long *)arg;
-	struct page *page;
 
 	if (PageReserved(pfn_to_page(start_pfn)))
-		for (i = 0; i < nr_pages; i++) {
-			page = pfn_to_page(start_pfn + i);
-			(*online_page_callback)(page);
-			onlined_pages++;
-		}
+		onlined_pages = online_pages_blocks(start_pfn, nr_pages);

Quoted 1 line

Quoted 1 line
online_mem_sections(start_pfn, start_pfn + nr_pages);

Balbir Singh.