mm/memory_hotplug: online_pages()/offline_pages() cleanups
Subsystems (4)
cover mm/memory_hotplug: online_pages()/offline_pages() cleanups
These are a bunch of cleanups for online_pages()/offline_pages() and related code, mostly getting rid of memory hole handling that is no longer necessary. There is only a single walk_system_ram_range() call left in offline_pages(), to make sure we don't have any memory holes. I had some of these patches lying around for a longer time but didn't have time to polish them.
In addition, the last patch marks all pageblocks of memory to get onlined MIGRATE_ISOLATE, so pages that have just been exposed to the buddy cannot get allocated before onlining is complete. Once heavy lifting is done, the pageblocks are set to MIGRATE_MOVABLE, such that allocations are possible.
I played with DIMMs and virtio-mem on x86-64 and didn't spot any surprises. I verified that the numer of isolated pageblocks is correctly handled when onlining/offlining.
David Hildenbrand (11):
mm/memory_hotplug: inline __offline_pages() into offline_pages() mm/memory_hotplug: enforce section granularity when onlining/offlining mm/memory_hotplug: simplify checking if all pages are isolated in
offline_pages()
mm/memory_hotplug: simplify offlining of pages in offline_pages() mm/page_alloc: simplify __offline_isolated_pages() mm/memory_hotplug: drop nr_isolate_pageblock in offline_pages() mm/page_isolation: simplify return value of start_isolate_page_range() mm/memory_hotplug: simplify page onlining
mm/page_alloc: drop stale pageblock comment in memmap_init_zone*()
mm: pass migratetype into memmap_init_zone() and
move_pfn_range_to_zone()
mm/memory_hotplug: mark pageblocks MIGRATE_ISOLATE while onlining
memory
arch/ia64/mm/init.c | 4 +- include/linux/memory_hotplug.h | 7 +- include/linux/mm.h | 3 +- mm/Kconfig | 2 +- mm/memory_hotplug.c | 156 ++++++++++++++------------------- mm/memremap.c | 3 +- mm/page_alloc.c | 64 ++++---------- mm/page_isolation.c | 7 +- 8 files changed, 98 insertions(+), 148 deletions(-)
Signature · 1 line
2.26.2
PATCH 1/11 mm/memory_hotplug: inline __offline_pages() into offline_pages()
There is only a single user, offline_pages(). Let's inline, to make it look more similar to online_pages().
--- mm/memory_hotplug.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
mm/memory_hotplug.c +5 -11
@@ -1473,11 +1473,10 @@ static int count_system_ram_pages_cb(unsigned long start_pfn,return 0 ; } -static int __ref __offline_pages (unsigned long start_pfn, -unsigned long end_pfn) +int __ref offline_pages (unsigned long start_pfn,unsigned long nr_pages) { -unsigned long pfn, nr_pages =0 ; -unsigned long offlined_pages =0 ; +const unsigned long end_pfn = start_pfn + nr_pages; +unsigned long pfn, system_ram_pages =0 , offlined_pages =0 ;int ret, node, nr_isolate_pageblock;unsigned long flags;struct zone *zone;
@@ -1494,9 +1493,9 @@ static int __ref __offline_pages(unsigned long start_pfn,* memory holes PG_reserved, don't need pfn_valid() checks, and can * avoid using walk_system_ram_range() later. */ -walk_system_ram_range (start_pfn, end_pfn - start_pfn, &nr_pages, +walk_system_ram_range (start_pfn, nr_pages, &system_ram_pages, count_system_ram_pages_cb); -if (nr_pages != end_pfn - start_pfn) { +if (system_ram_pages != nr_pages) { ret = -EINVAL ; reason ="memory holes" ; goto failed_removal;
@@ -1631,11 +1630,6 @@ static int __ref __offline_pages(unsigned long start_pfn,return ret; } -int offline_pages (unsigned long start_pfn,unsigned long nr_pages) -{ -return __offline_pages (start_pfn, start_pfn + nr_pages); -} -static int check_memblock_offlined_cb (struct memory_block *mem,void *arg) {int ret = !is_memblock_offlined (mem);
Signature · 1 line
2.26.2
Re: [PATCH v1 01/11] mm/memory_hotplug: inline __offline_pages() into offline_pages()
Quoted 10 lines · David Hildenbrand
There is only a single user,
offline_pages(). Let's inline, to make it look more similar toonline_pages().Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 50 lines
--- mm/memory_hotplug.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)mm/memory_hotplug.c +5
-11@@ -1473,11 +1473,10 @@ static int count_system_ram_pages_cb(unsigned long start_pfn,return 0 ; } -static int __ref __offline_pages (unsigned long start_pfn, -unsigned long end_pfn) +int __ref offline_pages (unsigned long start_pfn,unsigned long nr_pages) { -unsigned long pfn, nr_pages =0 ; -unsigned long offlined_pages =0 ; +const unsigned long end_pfn = start_pfn + nr_pages; +unsigned long pfn, system_ram_pages =0 , offlined_pages =0 ;int ret, node, nr_isolate_pageblock;unsigned long flags;struct zone *zone;@@ -1494,9 +1493,9 @@ static int __ref __offline_pages(unsigned long start_pfn,* memory holes PG_reserved, don't need pfn_valid() checks, and can * avoid using walk_system_ram_range() later. */ -walk_system_ram_range (start_pfn, end_pfn - start_pfn, &nr_pages, +walk_system_ram_range (start_pfn, nr_pages, &system_ram_pages, count_system_ram_pages_cb); -if (nr_pages != end_pfn - start_pfn) { +if (system_ram_pages != nr_pages) { ret = -EINVAL ; reason ="memory holes" ; goto failed_removal;@@ -1631,11 +1630,6 @@ static int __ref __offline_pages(unsigned long start_pfn,return ret; } -int offline_pages (unsigned long start_pfn,unsigned long nr_pages) -{ -return __offline_pages (start_pfn, start_pfn + nr_pages); -} -static int check_memblock_offlined_cb (struct memory_block *mem,void *arg) {int ret = !is_memblock_offlined (mem);Signature · 1 line
2.26.2
Signature · 2 lines
Michal Hocko SUSE Labs
PATCH 2/11 mm/memory_hotplug: enforce section granularity when onlining/offlining
In reply to David HildenbrandAlready two people (including me) tried to offline subsections, because the function looks like it can deal with it. But we really can only online/offline full sections (e.g., we can only mark full sections online/offline via SECTION_IS_ONLINE).
Add a simple safety net that to document the restriction now. Current users (core and powernv/memtrace) respect these restrictions.
--- mm/memory_hotplug.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
mm/memory_hotplug.c +10 -0
@@ -801,6 +801,11 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,int ret;struct memory_notify arg; +/* We can only online full sections (e.g., SECTION_IS_ONLINE) */ +if (WARN_ON_ONCE (!nr_pages || + !IS_ALIGNED (pfn | nr_pages,PAGES_PER_SECTION ))) +return -EINVAL ; +mem_hotplug_begin ();/* associate pfn range with the zone */
@@ -1483,6 +1488,11 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages)struct memory_notify arg;char *reason; +/* We can only offline full sections (e.g., SECTION_IS_ONLINE) */ +if (WARN_ON_ONCE (!nr_pages || + !IS_ALIGNED (start_pfn | nr_pages,PAGES_PER_SECTION ))) +return -EINVAL ; +mem_hotplug_begin ();/*
Signature · 1 line
2.26.2
Michal Hocko
reviewerRe: [PATCH v1 02/11] mm/memory_hotplug: enforce section granularity when onlining/offlining
Quoted 7 lines · David Hildenbrand
Already two people (including me) tried to offline subsections, because the function looks like it can deal with it. But we really can only online/offline full sections (e.g., we can only mark full sections online/offline via
SECTION_IS_ONLINE).Add a simple safety net that to document the restriction now. Current users (core and powernv/memtrace) respect these restrictions.
I do agree with the warning because it clarifies our expectations indeed. Se below for more questions.
Quoted 16 lines
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- mm/memory_hotplug.c | 10 ++++++++++ 1 file changed, 10 insertions(+)mm/memory_hotplug.c +0
-0
mm/memory_hotplug.c int __ref online_pages(unsigned long pfn, unsigned long nr_pages, · line 807
Quoted 7 lines
mm/memory_hotplug.c +4
-0@@ -801,6 +801,11 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,int ret;struct memory_notify arg; +/* We can only online full sections (e.g., SECTION_IS_ONLINE) */ +if (WARN_ON_ONCE (!nr_pages || + !IS_ALIGNED (pfn | nr_pages,PAGES_PER_SECTION ))) +return -EINVAL ;
This looks looks unnecessarily cryptic to me. Do you want to catch full section operation that doesn't start at the usual section boundary? If yes the above doesn't work work unless I am missing something.
Why don't you simply WARN_ON_ONCE(nr_pages % PAGES_PER_SECTION). !nr_pages doesn't sound like something interesting to care about or why do we care?
Signature · 2 lines
Michal Hocko SUSE Labs
Re: [PATCH v1 02/11] mm/memory_hotplug: enforce section granularity when onlining/offlining
Quoted 40 lines · Michal Hocko
Quoted 7 lines · David Hildenbrand
Already two people (including me) tried to offline subsections, because the function looks like it can deal with it. But we really can only online/offline full sections (e.g., we can only mark full sections online/offline via
SECTION_IS_ONLINE).Add a simple safety net that to document the restriction now. Current users (core and powernv/memtrace) respect these restrictions.
I do agree with the warning because it clarifies our expectations indeed. Se below for more questions.
Quoted 16 lines
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- mm/memory_hotplug.c | 10 ++++++++++ 1 file changed, 10 insertions(+)mm/memory_hotplug.c +0
-0mm/memory_hotplug.c int __ref online_pages(unsigned long pfn, unsigned long nr_pages, · line 807
Quoted 7 lines
mm/memory_hotplug.c +4
-0@@ -801,6 +801,11 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,int ret;struct memory_notify arg; +/* We can only online full sections (e.g., SECTION_IS_ONLINE) */ +if (WARN_ON_ONCE (!nr_pages || + !IS_ALIGNED (pfn | nr_pages,PAGES_PER_SECTION ))) +return -EINVAL ;This looks looks unnecessarily cryptic to me. Do you want to catch full section operation that doesn't start at the usual section boundary? If yes the above doesn't work work unless I am missing something.
Quoted 4 lines
Why don't you simply
WARN_ON_ONCE(nr_pages % PAGES_PER_SECTION). !nr_pages doesn't sound like something interesting to care about or why do we care?
Also the start pfn has to be section aligned, so we always cover fully aligned sections (e.g., not two partial ones).
It's essentially a compressed version of
!nr_pages || !IS_ALIGNED(pfn, PAGES_PER_SECTION) || !IS_ALIGN(nr_pages, PAGES_PER_SECTION)
which is the same as
!nr_pages || pfn % PAGES_PER_SECTION) || nr_pages % PAGES_PER_SECTION
or
!nr_pages || (pfn | nr_pages) % PAGES_PER_SECTION
I consider IS_ALIGNED easier to read than % PAGES_PER_SECTION. I can certainly un-compress, whatever you prefer, thanks.
Signature · 3 lines
Thanks, David / dhildenb
Re: [PATCH v1 02/11] mm/memory_hotplug: enforce section granularity when onlining/offlining
Quoted 41 lines · David Hildenbrand
Quoted 40 lines · Michal Hocko
Quoted 7 lines · David Hildenbrand
Already two people (including me) tried to offline subsections, because the function looks like it can deal with it. But we really can only online/offline full sections (e.g., we can only mark full sections online/offline via
SECTION_IS_ONLINE).Add a simple safety net that to document the restriction now. Current users (core and powernv/memtrace) respect these restrictions.
I do agree with the warning because it clarifies our expectations indeed. Se below for more questions.
Quoted 16 lines
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- mm/memory_hotplug.c | 10 ++++++++++ 1 file changed, 10 insertions(+)mm/memory_hotplug.c +0
-0mm/memory_hotplug.c int __ref online_pages(unsigned long pfn, unsigned long nr_pages, · line 807
Quoted 7 lines
mm/memory_hotplug.c +4
-0@@ -801,6 +801,11 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,int ret;struct memory_notify arg; +/* We can only online full sections (e.g., SECTION_IS_ONLINE) */ +if (WARN_ON_ONCE (!nr_pages || + !IS_ALIGNED (pfn | nr_pages,PAGES_PER_SECTION ))) +return -EINVAL ;This looks looks unnecessarily cryptic to me. Do you want to catch full section operation that doesn't start at the usual section boundary? If yes the above doesn't work work unless I am missing something.
Quoted 7 lines
Quoted 4 lines
Why don't you simply
WARN_ON_ONCE(nr_pages % PAGES_PER_SECTION). !nr_pages doesn't sound like something interesting to care about or why do we care?Also the start pfn has to be section aligned, so we always cover fully aligned sections (e.g., not two partial ones).
OK, I've misread your intention. I thought that we check for the start pfn prior to this warning but we only do that after.
Signature · 2 lines
Michal Hocko SUSE Labs
PATCH 3/11 mm/memory_hotplug: simplify checking if all pages are isolated in offline_pages()
In reply to David HildenbrandWe make sure that we cannot have any memory holes right at the beginning of offline_pages(). We no longer need walk_system_ram_range() and can call test_pages_isolated() directly.
--- mm/memory_hotplug.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-)
mm/memory_hotplug.c +1 -15
@@ -1384,17 +1384,6 @@ offline_isolated_pages_cb(unsigned long start, unsigned long nr_pages,return 0 ; } -/* -* Check all pages in range, recorded as memory resource, are isolated. -*/ -static int -check_pages_isolated_cb (unsigned long start_pfn,unsigned long nr_pages, -void *data) -{ -return test_pages_isolated (start_pfn, start_pfn + nr_pages, -MEMORY_OFFLINE ); -} -static int __init cmdline_parse_movable_node (char *p) { movable_node_enabled = true;
@@ -1579,10 +1568,7 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages) reason ="failure to dissolve huge pages" ; goto failed_removal_isolated; } -/* check again */ - ret =walk_system_ram_range (start_pfn, end_pfn - start_pfn, -NULL , check_pages_isolated_cb); - }while (ret); + }while (test_pages_isolated (start_pfn, end_pfn,MEMORY_OFFLINE ));/* Ok, all of our target is isolated. We cannot do rollback at this point. */
Signature · 1 line
2.26.2
Re: [PATCH v1 03/11] mm/memory_hotplug: simplify checking if all pages are isolated in offline_pages()
Quoted 11 lines · David Hildenbrand
We make sure that we cannot have any memory holes right at the beginning of
offline_pages(). We no longer needwalk_system_ram_range()and can calltest_pages_isolated()directly.Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 40 lines
--- mm/memory_hotplug.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-)mm/memory_hotplug.c +1
-15@@ -1384,17 +1384,6 @@ offline_isolated_pages_cb(unsigned long start, unsigned long nr_pages,return 0 ; } -/* -* Check all pages in range, recorded as memory resource, are isolated. -*/ -static int -check_pages_isolated_cb (unsigned long start_pfn,unsigned long nr_pages, -void *data) -{ -return test_pages_isolated (start_pfn, start_pfn + nr_pages, -MEMORY_OFFLINE ); -} -static int __init cmdline_parse_movable_node (char *p) { movable_node_enabled = true;@@ -1579,10 +1568,7 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages) reason ="failure to dissolve huge pages" ; goto failed_removal_isolated; } -/* check again */ - ret =walk_system_ram_range (start_pfn, end_pfn - start_pfn, -NULL , check_pages_isolated_cb); - }while (ret); + }while (test_pages_isolated (start_pfn, end_pfn,MEMORY_OFFLINE ));/* Ok, all of our target is isolated. We cannot do rollback at this point. */ Signature · 1 line
2.26.2
Signature · 2 lines
Michal Hocko SUSE Labs
Re: [PATCH v1 03/11] mm/memory_hotplug: simplify checking if all pages are isolated in offline_pages()
In reply to David HildenbrandQuoted 50 lines
We make sure that we cannot have any memory holes right at the beginning of
offline_pages(). We no longer needwalk_system_ram_range()and can calltest_pages_isolated()directly.Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Signed-off-by David Hildenbrand <[email protected]>--- mm/memory_hotplug.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-)mm/memory_hotplug.c +1
-16@@ -1384,17 +1384,6 @@ offline_isolated_pages_cb(unsigned long start, unsigned long nr_pages,return 0 ; } -/* -* Check all pages in range, recorded as memory resource, are isolated. -*/ -static int -check_pages_isolated_cb (unsigned long start_pfn,unsigned long nr_pages, -void *data) -{ -return test_pages_isolated (start_pfn, start_pfn + nr_pages, -MEMORY_OFFLINE ); -} -static int __init cmdline_parse_movable_node (char *p) { movable_node_enabled = true;@@ -1579,10 +1568,7 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages) reason ="failure to dissolve huge pages" ; goto failed_removal_isolated; } -/* check again */ - ret =walk_system_ram_range (start_pfn, end_pfn - start_pfn, -NULL , check_pages_isolated_cb); - }while (ret); + }while (test_pages_isolated (start_pfn, end_pfn,MEMORY_OFFLINE ));/* Ok, all of our target is isolated. We cannot do rollback at this point. */ --
Quoted 1 line
2.26.2
PATCH 4/11 mm/memory_hotplug: simplify offlining of pages in offline_pages()
In reply to David HildenbrandWe make sure that we cannot have any memory holes right at the beginning of offline_pages(). We no longer need walk_system_ram_range() and can call __offline_isolated_pages() directly.
offlined_pages always corresponds to nr_pages, so we can simplify that.
--- mm/memory_hotplug.c | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-)
mm/memory_hotplug.c +8 -20
@@ -1373,17 +1373,6 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)return ret; } -/* Mark all sections offline and remove all free pages from the buddy. */ -static int -offline_isolated_pages_cb (unsigned long start,unsigned long nr_pages, -void *data) -{ -unsigned long *offlined_pages = (unsigned long *)data; - - *offlined_pages +=__offline_isolated_pages (start, start + nr_pages); -return 0 ; -} -static int __init cmdline_parse_movable_node (char *p) { movable_node_enabled = true;
@@ -1470,7 +1459,7 @@ static int count_system_ram_pages_cb(unsigned long start_pfn,int __ref offline_pages (unsigned long start_pfn,unsigned long nr_pages) {const unsigned long end_pfn = start_pfn + nr_pages; -unsigned long pfn, system_ram_pages =0 , offlined_pages =0 ; +unsigned long pfn, system_ram_pages =0 ;int ret, node, nr_isolate_pageblock;unsigned long flags;struct zone *zone;
@@ -1570,11 +1559,10 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages) } }while (test_pages_isolated (start_pfn, end_pfn,MEMORY_OFFLINE )); -/* Ok, all of our target is isolated. -We cannot do rollback at this point. */ -walk_system_ram_range (start_pfn, end_pfn - start_pfn, - &offlined_pages, offline_isolated_pages_cb); -pr_info ("Offlined Pages %ld\n" , offlined_pages); +/* Mark all sections offline and remove free pages from the buddy. */ +__offline_isolated_pages (start_pfn, end_pfn); +pr_info ("Offlined Pages %ld\n" , nr_pages); +/* * Onlining will reset pagetype flags and makes migrate type * MOVABLE, so just need to decrease the number of isolated
@@ -1585,11 +1573,11 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages)spin_unlock_irqrestore (&zone->lock, flags);/* removal success */ -adjust_managed_page_count (pfn_to_page (start_pfn), -offlined_pages); - zone->present_pages -= offlined_pages; +adjust_managed_page_count (pfn_to_page (start_pfn), -nr_pages); + zone->present_pages -= nr_pages;pgdat_resize_lock (zone->zone_pgdat, &flags); - zone->zone_pgdat->node_present_pages -= offlined_pages; + zone->zone_pgdat->node_present_pages -= nr_pages;pgdat_resize_unlock (zone->zone_pgdat, &flags);init_per_zone_wmark_min ();
Signature · 1 line
2.26.2
Re: [PATCH v1 04/11] mm/memory_hotplug: simplify offlining of pages in offline_pages()
Quoted 5 lines · David Hildenbrand
We make sure that we cannot have any memory holes right at the beginning of
offline_pages(). We no longer needwalk_system_ram_range()and can call__offline_isolated_pages()directly.
offlined_pagesalways corresponds tonr_pages, so we can simplify that.
I would probably fold this into the previous patch as they are dealing with the same thing.
Quoted 7 lines
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 69 lines
--- mm/memory_hotplug.c | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-)mm/memory_hotplug.c +8
-20@@ -1373,17 +1373,6 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)return ret; } -/* Mark all sections offline and remove all free pages from the buddy. */ -static int -offline_isolated_pages_cb (unsigned long start,unsigned long nr_pages, -void *data) -{ -unsigned long *offlined_pages = (unsigned long *)data; - - *offlined_pages +=__offline_isolated_pages (start, start + nr_pages); -return 0 ; -} -static int __init cmdline_parse_movable_node (char *p) { movable_node_enabled = true;@@ -1470,7 +1459,7 @@ static int count_system_ram_pages_cb(unsigned long start_pfn,int __ref offline_pages (unsigned long start_pfn,unsigned long nr_pages) {const unsigned long end_pfn = start_pfn + nr_pages; -unsigned long pfn, system_ram_pages =0 , offlined_pages =0 ; +unsigned long pfn, system_ram_pages =0 ;int ret, node, nr_isolate_pageblock;unsigned long flags;struct zone *zone;@@ -1570,11 +1559,10 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages) } }while (test_pages_isolated (start_pfn, end_pfn,MEMORY_OFFLINE )); -/* Ok, all of our target is isolated. -We cannot do rollback at this point. */ -walk_system_ram_range (start_pfn, end_pfn - start_pfn, - &offlined_pages, offline_isolated_pages_cb); -pr_info ("Offlined Pages %ld\n" , offlined_pages); +/* Mark all sections offline and remove free pages from the buddy. */ +__offline_isolated_pages (start_pfn, end_pfn); +pr_info ("Offlined Pages %ld\n" , nr_pages); +/* * Onlining will reset pagetype flags and makes migrate type * MOVABLE, so just need to decrease the number of isolated @@ -1585,11 +1573,11 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages)spin_unlock_irqrestore (&zone->lock, flags);/* removal success */ -adjust_managed_page_count (pfn_to_page (start_pfn), -offlined_pages); - zone->present_pages -= offlined_pages; +adjust_managed_page_count (pfn_to_page (start_pfn), -nr_pages); + zone->present_pages -= nr_pages;pgdat_resize_lock (zone->zone_pgdat, &flags); - zone->zone_pgdat->node_present_pages -= offlined_pages; + zone->zone_pgdat->node_present_pages -= nr_pages;pgdat_resize_unlock (zone->zone_pgdat, &flags);init_per_zone_wmark_min ();Signature · 1 line
2.26.2
Signature · 2 lines
Michal Hocko SUSE Labs
Re: [PATCH v1 04/11] mm/memory_hotplug: simplify offlining of pages in offline_pages()
Quoted 9 lines · Michal Hocko
Quoted 5 lines · David Hildenbrand
We make sure that we cannot have any memory holes right at the beginning of
offline_pages(). We no longer needwalk_system_ram_range()and can call__offline_isolated_pages()directly.
offlined_pagesalways corresponds tonr_pages, so we can simplify that.I would probably fold this into the previous patch as they are dealing with the same thing.
Will do, shouldn't harm readability. Thanks!
---
Thanks,
David / dhildenb
PATCH 5/11 mm/page_alloc: simplify __offline_isolated_pages()
In reply to David Hildenbrandoffline_pages() is the only user. __offline_isolated_pages() never gets called with ranges that contain memory holes and we no longer care about the return value. Drop the return value handling and all pfn_valid() checks.
Update the documentation.
--- include/linux/memory_hotplug.h | 4 ++-- mm/page_alloc.c | 27 ++++----------------------- 2 files changed, 6 insertions(+), 25 deletions(-)
include/linux/memory_hotplug.h +2 -2
@@ -103,8 +103,8 @@ extern int online_pages(unsigned long pfn, unsigned long nr_pages, int online_type, int nid);extern struct zone *test_pages_in_a_zone (unsigned long start_pfn,unsigned long end_pfn); -extern unsigned long __offline_isolated_pages (unsigned long start_pfn, -unsigned long end_pfn); +extern void __offline_isolated_pages (unsigned long start_pfn, +unsigned long end_pfn);typedef void (*online_page_callback_t )(struct page *page,unsigned int order);
mm/page_alloc.c +4 -23
@@ -8692,35 +8692,21 @@ void zone_pcp_reset(struct zone *zone)#ifdef CONFIG_MEMORY_HOTREMOVE /* -* All pages in the range must be in a single zone and isolated -* before calling this. +* All pages in the range must be in a single zone, must not contain holes, +* must span full sections, and must be isolated before calling this function. */ -unsigned long -__offline_isolated_pages (unsigned long start_pfn,unsigned long end_pfn) +void __offline_isolated_pages (unsigned long start_pfn,unsigned long end_pfn) { +unsigned long pfn = start_pfn;struct page *page;struct zone *zone;unsigned int order; -unsigned long pfn;unsigned long flags; -unsigned long offlined_pages =0 ; - -/* find the first valid pfn */ -for (pfn = start_pfn; pfn < end_pfn; pfn++) -if (pfn_valid (pfn)) -break ; -if (pfn == end_pfn) -return offlined_pages;offline_mem_sections (pfn, end_pfn); zone =page_zone (pfn_to_page (pfn));spin_lock_irqsave (&zone->lock, flags); - pfn = start_pfn;while (pfn < end_pfn) { -if (!pfn_valid (pfn)) { - pfn++; -continue ; - } page =pfn_to_page (pfn);/* * The HWPoisoned page may be not in buddy system, and
@@ -8728,7 +8714,6 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)*/ if (unlikely (!PageBuddy (page) &&PageHWPoison (page))) { pfn++; - offlined_pages++;continue ; }/*
@@ -8739,20 +8724,16 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)BUG_ON (page_count (page));BUG_ON (PageBuddy (page)); pfn++; - offlined_pages++; continue; }BUG_ON (page_count (page));BUG_ON (!PageBuddy (page)); order =page_order (page); - offlined_pages +=1 << order;del_page_from_free_list (page, zone, order); pfn += (1 << order); }spin_unlock_irqrestore (&zone->lock, flags); - -return offlined_pages; }#endif
Signature · 1 line
2.26.2
Re: [PATCH v1 05/11] mm/page_alloc: simplify __offline_isolated_pages()
Quoted 7 lines · David Hildenbrand
offline_pages()is the only user.__offline_isolated_pages()never gets called with ranges that contain memory holes and we no longer care about the return value. Drop the return value handling and allpfn_valid()checks.Update the documentation.
Quoted 7 lines
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 96 lines
--- include/linux/memory_hotplug.h | 4 ++-- mm/page_alloc.c | 27 ++++----------------------- 2 files changed, 6 insertions(+), 25 deletions(-)include/linux/memory_hotplug.h +2
-2@@ -103,8 +103,8 @@ extern int online_pages(unsigned long pfn, unsigned long nr_pages, int online_type, int nid);extern struct zone *test_pages_in_a_zone (unsigned long start_pfn,unsigned long end_pfn); -extern unsigned long __offline_isolated_pages (unsigned long start_pfn, -unsigned long end_pfn); +extern void __offline_isolated_pages (unsigned long start_pfn, +unsigned long end_pfn);typedef void (*online_page_callback_t )(struct page *page,unsigned int order);mm/page_alloc.c +4
-23@@ -8692,35 +8692,21 @@ void zone_pcp_reset(struct zone *zone)#ifdef CONFIG_MEMORY_HOTREMOVE /* -* All pages in the range must be in a single zone and isolated -* before calling this. +* All pages in the range must be in a single zone, must not contain holes, +* must span full sections, and must be isolated before calling this function. */ -unsigned long -__offline_isolated_pages (unsigned long start_pfn,unsigned long end_pfn) +void __offline_isolated_pages (unsigned long start_pfn,unsigned long end_pfn) { +unsigned long pfn = start_pfn;struct page *page;struct zone *zone;unsigned int order; -unsigned long pfn;unsigned long flags; -unsigned long offlined_pages =0 ; - -/* find the first valid pfn */ -for (pfn = start_pfn; pfn < end_pfn; pfn++) -if (pfn_valid (pfn)) -break ; -if (pfn == end_pfn) -return offlined_pages;offline_mem_sections (pfn, end_pfn); zone =page_zone (pfn_to_page (pfn));spin_lock_irqsave (&zone->lock, flags); - pfn = start_pfn;while (pfn < end_pfn) { -if (!pfn_valid (pfn)) { - pfn++; -continue ; - } page =pfn_to_page (pfn);/* * The HWPoisoned page may be not in buddy system, and @@ -8728,7 +8714,6 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)*/ if (unlikely (!PageBuddy (page) &&PageHWPoison (page))) { pfn++; - offlined_pages++;continue ; }/* @@ -8739,20 +8724,16 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)BUG_ON (page_count (page));BUG_ON (PageBuddy (page)); pfn++; - offlined_pages++; continue; }BUG_ON (page_count (page));BUG_ON (!PageBuddy (page)); order =page_order (page); - offlined_pages +=1 << order;del_page_from_free_list (page, zone, order); pfn += (1 << order); }spin_unlock_irqrestore (&zone->lock, flags); - -return offlined_pages; }#endif Signature · 1 line
2.26.2
Signature · 2 lines
Michal Hocko SUSE Labs
PATCH 6/11 mm/memory_hotplug: drop nr_isolate_pageblock in offline_pages()
In reply to David HildenbrandWe make sure that we cannot have any memory holes right at the beginning of offline_pages() and we only support to online/offline full sections. Both, sections and pageblocks are a power of two in size, and sections always span full pageblocks.
We can directly calculate the number of isolated pageblocks from nr_pages.
--- mm/memory_hotplug.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
mm/memory_hotplug.c +2 -3
@@ -1460,10 +1460,10 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages) {const unsigned long end_pfn = start_pfn + nr_pages;unsigned long pfn, system_ram_pages =0 ; -int ret, node, nr_isolate_pageblock;unsigned long flags;struct zone *zone;struct memory_notify arg; +int ret, node;char *reason;/* We can only offline full sections (e.g., SECTION_IS_ONLINE) */
@@ -1507,7 +1507,6 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages) reason ="failure to isolate range" ; goto failed_removal; } - nr_isolate_pageblock = ret; arg.start_pfn = start_pfn; arg.nr_pages = nr_pages;
@@ -1569,7 +1568,7 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages)* pageblocks zone counter here. */ spin_lock_irqsave (&zone->lock, flags); - zone->nr_isolate_pageblock -= nr_isolate_pageblock; + zone->nr_isolate_pageblock -= nr_pages / pageblock_nr_pages;spin_unlock_irqrestore (&zone->lock, flags);/* removal success */
Signature · 1 line
2.26.2
Re: [PATCH v1 06/11] mm/memory_hotplug: drop nr_isolate_pageblock in offline_pages()
Quoted 7 lines · David Hildenbrand
We make sure that we cannot have any memory holes right at the beginning of
offline_pages()and we only support to online/offline full sections. Both, sections and pageblocks are a power of two in size, and sections always span full pageblocks.We can directly calculate the number of isolated pageblocks from
nr_pages.
Quoted 7 lines
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 40 lines
--- mm/memory_hotplug.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)mm/memory_hotplug.c +2
-3@@ -1460,10 +1460,10 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages) {const unsigned long end_pfn = start_pfn + nr_pages;unsigned long pfn, system_ram_pages =0 ; -int ret, node, nr_isolate_pageblock;unsigned long flags;struct zone *zone;struct memory_notify arg; +int ret, node;char *reason;/* We can only offline full sections (e.g., SECTION_IS_ONLINE) */ @@ -1507,7 +1507,6 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages) reason ="failure to isolate range" ; goto failed_removal; } - nr_isolate_pageblock = ret; arg.start_pfn = start_pfn; arg.nr_pages = nr_pages;@@ -1569,7 +1568,7 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages)* pageblocks zone counter here. */ spin_lock_irqsave (&zone->lock, flags); - zone->nr_isolate_pageblock -= nr_isolate_pageblock; + zone->nr_isolate_pageblock -= nr_pages / pageblock_nr_pages;spin_unlock_irqrestore (&zone->lock, flags);/* removal success */ Signature · 1 line
2.26.2
Signature · 2 lines
Michal Hocko SUSE Labs
PATCH 7/11 mm/page_isolation: simplify return value of start_isolate_page_range()
In reply to David HildenbrandCallers no longer need the number of isolated pageblocks. Let's simplify.
--- mm/memory_hotplug.c | 2 +- mm/page_alloc.c | 2 +- mm/page_isolation.c | 7 ++----- 3 files changed, 4 insertions(+), 7 deletions(-)
mm/memory_hotplug.c +1 -1
@@ -1503,7 +1503,7 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages) ret =start_isolate_page_range (start_pfn, end_pfn,MIGRATE_MOVABLE ,MEMORY_OFFLINE |REPORT_FAILURE ); -if (ret <0 ) { +if (ret) { reason ="failure to isolate range" ; goto failed_removal; }
mm/page_alloc.c +1 -1
@@ -8456,7 +8456,7 @@ int alloc_contig_range(unsigned long start, unsigned long end, ret =start_isolate_page_range (pfn_max_align_down (start),pfn_max_align_up (end), migratetype,0 ); -if (ret <0 ) +if (ret)return ret;/*
mm/page_isolation.c +2 -5
@@ -170,8 +170,7 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)* pageblocks we may have modified and return -EBUSY to caller. This * prevents two threads from simultaneously working on overlapping ranges. * -* Return: the number of isolated pageblocks on success and -EBUSY if any part -* of range cannot be isolated. +* Return: 0 on success and -EBUSY if any part of range cannot be isolated. */ int start_isolate_page_range (unsigned long start_pfn,unsigned long end_pfn,unsigned migratetype,int flags)
@@ -179,7 +178,6 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,unsigned long pfn;unsigned long undo_pfn;struct page *page; -int nr_isolate_pageblock =0 ;BUG_ON (!IS_ALIGNED (start_pfn, pageblock_nr_pages));BUG_ON (!IS_ALIGNED (end_pfn, pageblock_nr_pages));
@@ -193,10 +191,9 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn, undo_pfn = pfn; goto undo; } - nr_isolate_pageblock++; } } -return nr_isolate_pageblock; +return 0 ; undo: for (pfn = start_pfn; pfn < undo_pfn;
Signature · 1 line
2.26.2
Re: [PATCH v1 07/11] mm/page_isolation: simplify return value of start_isolate_page_range()
Quoted 10 lines · David Hildenbrand
Callers no longer need the number of isolated pageblocks. Let's simplify.
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 68 lines
--- mm/memory_hotplug.c | 2 +- mm/page_alloc.c | 2 +- mm/page_isolation.c | 7 ++----- 3 files changed, 4 insertions(+), 7 deletions(-)mm/memory_hotplug.c +1
-1@@ -1503,7 +1503,7 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages) ret =start_isolate_page_range (start_pfn, end_pfn,MIGRATE_MOVABLE ,MEMORY_OFFLINE |REPORT_FAILURE ); -if (ret <0 ) { +if (ret) { reason ="failure to isolate range" ; goto failed_removal; }mm/page_alloc.c +1
-1@@ -8456,7 +8456,7 @@ int alloc_contig_range(unsigned long start, unsigned long end, ret =start_isolate_page_range (pfn_max_align_down (start),pfn_max_align_up (end), migratetype,0 ); -if (ret <0 ) +if (ret)return ret;/* mm/page_isolation.c +2
-5@@ -170,8 +170,7 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)* pageblocks we may have modified and return -EBUSY to caller. This * prevents two threads from simultaneously working on overlapping ranges. * -* Return: the number of isolated pageblocks on success and -EBUSY if any part -* of range cannot be isolated. +* Return: 0 on success and -EBUSY if any part of range cannot be isolated. */ int start_isolate_page_range (unsigned long start_pfn,unsigned long end_pfn,unsigned migratetype,int flags)@@ -179,7 +178,6 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,unsigned long pfn;unsigned long undo_pfn;struct page *page; -int nr_isolate_pageblock =0 ;BUG_ON (!IS_ALIGNED (start_pfn, pageblock_nr_pages));BUG_ON (!IS_ALIGNED (end_pfn, pageblock_nr_pages));@@ -193,10 +191,9 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn, undo_pfn = pfn; goto undo; } - nr_isolate_pageblock++; } } -return nr_isolate_pageblock; +return 0 ; undo: for (pfn = start_pfn; pfn < undo_pfn;Signature · 1 line
2.26.2
Signature · 2 lines
Michal Hocko SUSE Labs
PATCH 8/11 mm/memory_hotplug: simplify page onlining
In reply to David HildenbrandWe don't allow to offline memory with holes, all boot memory is online, and all hotplugged memory cannot have holes.
We can now simplify onlining of pages. As we only allow to online/offline full sections and sections always span full MAX_ORDER_NR_PAGES, we can just process MAX_ORDER - 1 pages without further special handling.
The number of onlined pages simply corresponds to the number of pages we were requested to online.
While at it, refine the comment regarding the callback not exposing all pages to the buddy.
--- mm/memory_hotplug.c | 38 ++++++++++---------------------------- 1 file changed, 10 insertions(+), 28 deletions(-)
mm/memory_hotplug.c +10 -28
@@ -617,31 +617,22 @@ void generic_online_page(struct page *page, unsigned int order) }EXPORT_SYMBOL_GPL (generic_online_page); -static int online_pages_range (unsigned long start_pfn,unsigned long nr_pages, -void *arg) +static void online_pages_range (unsigned long start_pfn,unsigned long nr_pages) {const unsigned long end_pfn = start_pfn + nr_pages;unsigned long pfn; -int order;/* -* Online the pages. The callback might decide to keep some pages -* PG_reserved (to add them to the buddy later), but we still account -* them as being online/belonging to this zone ("present"). +* Online the pages in MAX_ORDER - 1 aligned chunks. The callback might +* decide to not expose all pages to the buddy (e.g., expose them +* later). We account all pages as being online and belonging to this +* zone ("present"). */ -for (pfn = start_pfn; pfn < end_pfn; pfn +=1ul << order) { - order =min (MAX_ORDER -1 ,get_order (PFN_PHYS (end_pfn - pfn))); -/* __free_pages_core() wants pfns to be aligned to the order */ -if (WARN_ON_ONCE (!IS_ALIGNED (pfn,1ul << order))) - order =0 ; - (*online_page_callback)(pfn_to_page (pfn), order); - } +for (pfn = start_pfn; pfn < end_pfn; pfn +=MAX_ORDER_NR_PAGES ) + (*online_page_callback)(pfn_to_page (pfn),MAX_ORDER -1 );/* mark all involved sections as online */ online_mem_sections (start_pfn, end_pfn); - - *(unsigned long *)arg += nr_pages; -return 0 ; }/* check which state of node_states will be changed when online memory */
@@ -795,7 +786,6 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_type, int nid) {unsigned long flags; -unsigned long onlined_pages =0 ;struct zone *zone;int need_zonelists_rebuild =0 ;int ret;
@@ -831,19 +821,11 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,setup_zone_pageset (zone); } - ret =walk_system_ram_range (pfn, nr_pages, &onlined_pages, - online_pages_range); -if (ret) { -/* not a single memory resource was applicable */ -if (need_zonelists_rebuild) -zone_pcp_reset (zone); - goto failed_addition; - } - - zone->present_pages += onlined_pages; +online_pages_range (pfn, nr_pages); + zone->present_pages += nr_pages;pgdat_resize_lock (zone->zone_pgdat, &flags); - zone->zone_pgdat->node_present_pages += onlined_pages; + zone->zone_pgdat->node_present_pages += nr_pages;pgdat_resize_unlock (zone->zone_pgdat, &flags);/*
Signature · 1 line
2.26.2
Re: [PATCH v1 08/11] mm/memory_hotplug: simplify page onlining
Quoted 13 lines · David Hildenbrand
We don't allow to offline memory with holes, all boot memory is online, and all hotplugged memory cannot have holes.
We can now simplify onlining of pages. As we only allow to online/offline full sections and sections always span full
MAX_ORDER_NR_PAGES, we can just processMAX_ORDER- 1 pages without further special handling.The number of onlined pages simply corresponds to the number of pages we were requested to online.
While at it, refine the comment regarding the callback not exposing all pages to the buddy.
Quoted 7 lines
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 81 lines
--- mm/memory_hotplug.c | 38 ++++++++++---------------------------- 1 file changed, 10 insertions(+), 28 deletions(-)mm/memory_hotplug.c +10
-28@@ -617,31 +617,22 @@ void generic_online_page(struct page *page, unsigned int order) }EXPORT_SYMBOL_GPL (generic_online_page); -static int online_pages_range (unsigned long start_pfn,unsigned long nr_pages, -void *arg) +static void online_pages_range (unsigned long start_pfn,unsigned long nr_pages) {const unsigned long end_pfn = start_pfn + nr_pages;unsigned long pfn; -int order;/* -* Online the pages. The callback might decide to keep some pages -* PG_reserved (to add them to the buddy later), but we still account -* them as being online/belonging to this zone ("present"). +* Online the pages in MAX_ORDER - 1 aligned chunks. The callback might +* decide to not expose all pages to the buddy (e.g., expose them +* later). We account all pages as being online and belonging to this +* zone ("present"). */ -for (pfn = start_pfn; pfn < end_pfn; pfn +=1ul << order) { - order =min (MAX_ORDER -1 ,get_order (PFN_PHYS (end_pfn - pfn))); -/* __free_pages_core() wants pfns to be aligned to the order */ -if (WARN_ON_ONCE (!IS_ALIGNED (pfn,1ul << order))) - order =0 ; - (*online_page_callback)(pfn_to_page (pfn), order); - } +for (pfn = start_pfn; pfn < end_pfn; pfn +=MAX_ORDER_NR_PAGES ) + (*online_page_callback)(pfn_to_page (pfn),MAX_ORDER -1 );/* mark all involved sections as online */ online_mem_sections (start_pfn, end_pfn); - - *(unsigned long *)arg += nr_pages; -return 0 ; }/* check which state of node_states will be changed when online memory */ @@ -795,7 +786,6 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_type, int nid) {unsigned long flags; -unsigned long onlined_pages =0 ;struct zone *zone;int need_zonelists_rebuild =0 ;int ret;@@ -831,19 +821,11 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,setup_zone_pageset (zone); } - ret =walk_system_ram_range (pfn, nr_pages, &onlined_pages, - online_pages_range); -if (ret) { -/* not a single memory resource was applicable */ -if (need_zonelists_rebuild) -zone_pcp_reset (zone); - goto failed_addition; - } - - zone->present_pages += onlined_pages; +online_pages_range (pfn, nr_pages); + zone->present_pages += nr_pages;pgdat_resize_lock (zone->zone_pgdat, &flags); - zone->zone_pgdat->node_present_pages += onlined_pages; + zone->zone_pgdat->node_present_pages += nr_pages;pgdat_resize_unlock (zone->zone_pgdat, &flags);/* Signature · 1 line
2.26.2
Signature · 2 lines
Michal Hocko SUSE Labs
PATCH 9/11 mm/page_alloc: drop stale pageblock comment in memmap_init_zone*()
In reply to David HildenbrandCommit ac5d2539b238 ("mm: meminit: reduce number of times pageblocks are set during struct page init") moved the actual zone range check, leaving only the alignment check for pageblocks.
Let's drop the stale comment and make the pageblock check easier to read.
--- mm/page_alloc.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)
mm/page_alloc.c +2 -12
@@ -6022,13 +6022,8 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,* to reserve their blocks rather than leaking throughout * the address space during boot when many long-lived * kernel allocations are made. -* -* bitmap is created for zone's valid pfn range. but memmap -* can be created for invalid pages (for alignment) -* check here not to call set_pageblock_migratetype() against -* pfn out of zone. */ -if (!(pfn & (pageblock_nr_pages -1 ))) { +if (IS_ALIGNED (pfn, pageblock_nr_pages)) {set_pageblock_migratetype (page,MIGRATE_MOVABLE );cond_resched (); }
@@ -6091,15 +6086,10 @@ void __ref memmap_init_zone_device(struct zone *zone,* the address space during boot when many long-lived * kernel allocations are made. * -* bitmap is created for zone's valid pfn range. but memmap -* can be created for invalid pages (for alignment) -* check here not to call set_pageblock_migratetype() against -* pfn out of zone. -* * Please note that MEMMAP_HOTPLUG path doesn't clear memmap * because this is done early in section_activate() */ -if (!(pfn & (pageblock_nr_pages -1 ))) { +if (IS_ALIGNED (pfn, pageblock_nr_pages)) {set_pageblock_migratetype (page,MIGRATE_MOVABLE );cond_resched (); }
Signature · 1 line
2.26.2
Re: [PATCH v1 09/11] mm/page_alloc: drop stale pageblock comment in memmap_init_zone*()
Quoted 5 lines · David Hildenbrand
Commit
ac5d2539b238("mm: meminit: reduce number of times pageblocks are set during struct page init") moved the actual zone range check, leaving only the alignment check for pageblocks.Let's drop the stale comment and make the pageblock check easier to read.
I do agree athat IS_ALIGNED is easier to read in this case.
Quoted 8 lines
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Cc Mel Gorman <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Quoted 43 lines
--- mm/page_alloc.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)mm/page_alloc.c +2
-12@@ -6022,13 +6022,8 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,* to reserve their blocks rather than leaking throughout * the address space during boot when many long-lived * kernel allocations are made. -* -* bitmap is created for zone's valid pfn range. but memmap -* can be created for invalid pages (for alignment) -* check here not to call set_pageblock_migratetype() against -* pfn out of zone. */ -if (!(pfn & (pageblock_nr_pages -1 ))) { +if (IS_ALIGNED (pfn, pageblock_nr_pages)) {set_pageblock_migratetype (page,MIGRATE_MOVABLE );cond_resched (); }@@ -6091,15 +6086,10 @@ void __ref memmap_init_zone_device(struct zone *zone,* the address space during boot when many long-lived * kernel allocations are made. * -* bitmap is created for zone's valid pfn range. but memmap -* can be created for invalid pages (for alignment) -* check here not to call set_pageblock_migratetype() against -* pfn out of zone. -* * Please note that MEMMAP_HOTPLUG path doesn't clear memmap * because this is done early in section_activate() */ -if (!(pfn & (pageblock_nr_pages -1 ))) { +if (IS_ALIGNED (pfn, pageblock_nr_pages)) {set_pageblock_migratetype (page,MIGRATE_MOVABLE );cond_resched (); }Signature · 1 line
2.26.2
Signature · 2 lines
Michal Hocko SUSE Labs
PATCH 10/11 mm: pass migratetype into memmap_init_zone() and move_pfn_range_to_zone()
In reply to David HildenbrandOn the memory hotplug path, we want to start with MIGRATE_ISOLATE, to un-isolate the pages after memory onlining is complete. Let's allow passing in the migratetype.
--- arch/ia64/mm/init.c | 4 ++-- include/linux/memory_hotplug.h | 3 ++- include/linux/mm.h | 3 ++- mm/memory_hotplug.c | 11 ++++++++--- mm/memremap.c | 3 ++- mm/page_alloc.c | 21 ++++++++++++--------- 6 files changed, 28 insertions(+), 17 deletions(-)
arch/ia64/mm/init.c +2 -2
@@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)if (map_start < map_end)memmap_init_zone ((unsigned long )(map_end - map_start), args->nid, args->zone,page_to_pfn (map_start), -MEMMAP_EARLY ,NULL ); +MEMMAP_EARLY ,NULL ,MIGRATE_MOVABLE );return 0 ; }
@@ -548,7 +548,7 @@ memmap_init (unsigned long size, int nid, unsigned long zone, {if (!vmem_map) {memmap_init_zone (size, nid, zone, start_pfn,MEMMAP_EARLY , -NULL ); +NULL ,MIGRATE_MOVABLE ); }else {struct page *start;struct memmap_init_callback_data args;
include/linux/memory_hotplug.h +2 -1
@@ -346,7 +346,8 @@ extern int add_memory_resource(int nid, struct resource *resource);extern int add_memory_driver_managed (int nid,u64 start,u64 size,const char *resource_name);extern void move_pfn_range_to_zone (struct zone *zone,unsigned long start_pfn, -unsigned long nr_pages,struct vmem_altmap *altmap); +unsigned long nr_pages, +struct vmem_altmap *altmap,int migratetype);extern void remove_pfn_range_from_zone (struct zone *zone,unsigned long start_pfn,unsigned long nr_pages);
include/linux/mm.h +2 -1
@@ -2409,7 +2409,8 @@ extern int __meminit __early_pfn_to_nid(unsigned long pfn,extern void set_dma_reserve (unsigned long new_dma_reserve);extern void memmap_init_zone (unsigned long ,int ,unsigned long ,unsigned long , -enum memmap_context ,struct vmem_altmap *); +enum memmap_context ,struct vmem_altmap *, +int migratetype);extern void setup_per_zone_wmarks (void );extern int __meminit init_per_zone_wmark_min (void );extern void mem_init (void );
mm/memory_hotplug.c +8 -3
@@ -693,9 +693,14 @@ static void __meminit resize_pgdat_range(struct pglist_data *pgdat, unsigned lon* Associate the pfn range with the given zone, initializing the memmaps * and resizing the pgdat/zone data to span the added pages. After this * call, all affected pages are PG_reserved. +* +* All aligned pageblocks are initialized to the specified migratetype +* (usually MIGRATE_MOVABLE). Besides setting the migratetype, no related +* zone stats (e.g., nr_isolate_pageblock) are touched. */ void __ref move_pfn_range_to_zone (struct zone *zone,unsigned long start_pfn, -unsigned long nr_pages,struct vmem_altmap *altmap) +unsigned long nr_pages, +struct vmem_altmap *altmap,int migratetype) {struct pglist_data *pgdat = zone->zone_pgdat;int nid = pgdat->node_id;
@@ -720,7 +725,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,* are reserved so nobody should be touching them so we should be safe */ memmap_init_zone (nr_pages, nid,zone_idx (zone), start_pfn, -MEMMAP_HOTPLUG , altmap); +MEMMAP_HOTPLUG , altmap, migratetype);set_zone_contiguous (zone); }
@@ -800,7 +805,7 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,/* associate pfn range with the zone */ zone =zone_for_pfn_range (online_type, nid, pfn, nr_pages); -move_pfn_range_to_zone (zone, pfn, nr_pages,NULL ); +move_pfn_range_to_zone (zone, pfn, nr_pages,NULL ,MIGRATE_MOVABLE ); arg.start_pfn = pfn; arg.nr_pages = nr_pages;
mm/memremap.c +2 -1
@@ -342,7 +342,8 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid) zone = &NODE_DATA (nid)->node_zones[ZONE_DEVICE ];move_pfn_range_to_zone (zone,PHYS_PFN (res->start), -PHYS_PFN (resource_size (res)), params.altmap); +PHYS_PFN (resource_size (res)), + params.altmap,MIGRATE_MOVABLE ); }mem_hotplug_done ();
mm/page_alloc.c +12 -9
@@ -5970,10 +5970,15 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)* Initially all pages are reserved - free ones are freed * up by memblock_free_all() once the early boot process is * done. Non-atomic initialization, single-pass. +* +* All aligned pageblocks are initialized to the specified migratetype +* (usually MIGRATE_MOVABLE). Besides setting the migratetype, no related +* zone stats (e.g., nr_isolate_pageblock) are touched. */ void __meminit memmap_init_zone (unsigned long size,int nid,unsigned long zone, -unsigned long start_pfn,enum memmap_context context, -struct vmem_altmap *altmap) +unsigned long start_pfn, +enum memmap_context context, +struct vmem_altmap *altmap,int migratetype) {unsigned long pfn, end_pfn = start_pfn + size;struct page *page;
@@ -6017,14 +6022,12 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,__SetPageReserved (page);/* -* Mark the block movable so that blocks are reserved for -* movable at startup. This will force kernel allocations -* to reserve their blocks rather than leaking throughout -* the address space during boot when many long-lived -* kernel allocations are made. +* Usually, we want to mark the pageblock MIGRATE_MOVABLE, +* such that unmovable allocations won't be scattered all +* over the place during system boot. */ if (IS_ALIGNED (pfn, pageblock_nr_pages)) { -set_pageblock_migratetype (page,MIGRATE_MOVABLE ); +set_pageblock_migratetype (page, migratetype);cond_resched (); } pfn++;
@@ -6124,7 +6127,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,if (end_pfn > start_pfn) { size = end_pfn - start_pfn;memmap_init_zone (size, nid, zone, start_pfn, -MEMMAP_EARLY ,NULL ); +MEMMAP_EARLY ,NULL ,MIGRATE_MOVABLE ); } } }
Signature · 1 line
2.26.2
Re: [PATCH v1 10/11] mm: pass migratetype into memmap_init_zone() and move_pfn_range_to_zone()
Quoted 4 lines · David Hildenbrand
On the memory hotplug path, we want to start with
MIGRATE_ISOLATE, to un-isolate the pages after memory onlining is complete. Let's allow passing in the migratetype.
Quoted 15 lines
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Cc Tony Luck <[email protected]>Cc Fenghua Yu <[email protected]>Cc Logan Gunthorpe <[email protected]>Cc Dan Williams <[email protected]>Cc Mike Rapoport <[email protected]>Cc "Matthew Wilcox (Oracle)" <[email protected]>Cc Michel Lespinasse <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Signature · 2 lines
Michal Hocko SUSE Labs
PATCH 11/11 mm/memory_hotplug: mark pageblocks MIGRATE_ISOLATE while onlining memory
In reply to David HildenbrandCurrently, it can happen that pages are allocated (and freed) via the buddy before we finished basic memory onlining.
For example, pages are exposed to the buddy and can be allocated before we actually mark the sections online. Allocated pages could suddenly fail pfn_to_online_page() checks. We had similar issues with pcp handling, when pages are allocated+freed before we reach zone_pcp_update() in online_pages() [1].
Instead, mark all pageblocks MIGRATE_ISOLATE, such that allocations are impossible. Once done with the heavy lifting, use undo_isolate_page_range() to move the pages to the MIGRATE_MOVABLE freelist, marking them ready for allocation. Similar to offline_pages(), we have to manually adjust zone->nr_isolate_pageblock.
[1] https://lkml.kernel.org/r/[email protected]
--- mm/Kconfig | 2 +- mm/memory_hotplug.c | 32 ++++++++++++++++++++++---------- 2 files changed, 23 insertions(+), 11 deletions(-)
mm/Kconfig +1 -1
@@ -152,6 +152,7 @@ config HAVE_BOOTMEM_INFO_NODE # eventually, we can have this option just 'select SPARSEMEM' config MEMORY_HOTPLUG bool "Allow for memory hot-add" + select MEMORY_ISOLATION depends on SPARSEMEM || X86_64_ACPI_NUMA depends on ARCH_ENABLE_MEMORY_HOTPLUG depends on 64BIT || BROKEN
@@ -178,7 +179,6 @@ config MEMORY_HOTPLUG_DEFAULT_ONLINE config MEMORY_HOTREMOVE bool "Allow for memory hot remove" - select MEMORY_ISOLATION select HAVE_BOOTMEM_INFO_NODE if (X86_64 || PPC64) depends on MEMORY_HOTPLUG && ARCH_ENABLE_MEMORY_HOTREMOVE depends on MIGRATION
mm/memory_hotplug.c +22 -10
@@ -805,7 +805,7 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,/* associate pfn range with the zone */ zone =zone_for_pfn_range (online_type, nid, pfn, nr_pages); -move_pfn_range_to_zone (zone, pfn, nr_pages,NULL ,MIGRATE_MOVABLE ); +move_pfn_range_to_zone (zone, pfn, nr_pages,NULL ,MIGRATE_ISOLATE ); arg.start_pfn = pfn; arg.nr_pages = nr_pages;
@@ -816,6 +816,14 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,if (ret) goto failed_addition; +/* +* Fixup the number of isolated pageblocks before marking the sections +* onlining, such that undo_isolate_page_range() works correctly. +*/ +spin_lock_irqsave (&zone->lock, flags); + zone->nr_isolate_pageblock += nr_pages / pageblock_nr_pages; +spin_unlock_irqrestore (&zone->lock, flags); +/* * If this zone is not populated, then it is not in zonelist. * This means the page allocator ignores this zone.
@@ -833,21 +841,25 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, zone->zone_pgdat->node_present_pages += nr_pages;pgdat_resize_unlock (zone->zone_pgdat, &flags); +node_states_set_node (nid, &arg); +if (need_zonelists_rebuild) +build_all_zonelists (NULL ); +zone_pcp_update (zone); + +/* Basic onlining is complete, allow allocation of onlined pages. */ +undo_isolate_page_range (pfn, pfn + nr_pages,MIGRATE_MOVABLE ); +/* * When exposing larger, physically contiguous memory areas to the * buddy, shuffling in the buddy (when freeing onlined pages, putting * them either to the head or the tail of the freelist) is only helpful * for maintaining the shuffle, but not for creating the initial * shuffle. Shuffle the whole zone to make sure the just onlined pages -* are properly distributed across the whole freelist. +* are properly distributed across the whole freelist. Make sure to +* shuffle once pageblocks are no longer isolated. */ shuffle_zone (zone); -node_states_set_node (nid, &arg); -if (need_zonelists_rebuild) -build_all_zonelists (NULL ); -zone_pcp_update (zone); -init_per_zone_wmark_min ();kswapd_run (nid);
@@ -1550,9 +1562,9 @@ int __ref offline_pages(unsigned long start_pfn, unsigned long nr_pages)pr_info ("Offlined Pages %ld\n" , nr_pages);/* -* Onlining will reset pagetype flags and makes migrate type -* MOVABLE, so just need to decrease the number of isolated -* pageblocks zone counter here. +* The memory sections are marked offline, and the pageblock flags +* effectively stale; nobody should be touching them. Fixup the number +* of isolated pageblocks, memory onlining will properly revert this. */ spin_lock_irqsave (&zone->lock, flags); zone->nr_isolate_pageblock -= nr_pages / pageblock_nr_pages;
Signature · 1 line
2.26.2
Re: [PATCH v1 11/11] mm/memory_hotplug: mark pageblocks MIGRATE_ISOLATE while onlining memory
Quoted 17 lines · David Hildenbrand
Currently, it can happen that pages are allocated (and freed) via the buddy before we finished basic memory onlining.
For example, pages are exposed to the buddy and can be allocated before we actually mark the sections online. Allocated pages could suddenly fail
pfn_to_online_page()checks. We had similar issues with pcp handling, when pages are allocated+freed before we reachzone_pcp_update()inonline_pages()[1].Instead, mark all pageblocks
MIGRATE_ISOLATE, such that allocations are impossible. Once done with the heavy lifting, useundo_isolate_page_range()to move the pages to theMIGRATE_MOVABLEfreelist, marking them ready for allocation. Similar tooffline_pages(), we have to manually adjustzone->nr_isolate_pageblock.
Quoted 8 lines
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Cc Charan Teja Reddy <[email protected]>Signed-off-by David Hildenbrand <[email protected]>
Yes this looks very sensible and we should have done that from the beginning. I just have one minor comment below
mm/memory_hotplug.c int __ref online_pages(unsigned long pfn, unsigned long nr_pages, · line 826
Quoted 12 lines
mm/memory_hotplug.c +8
-0@@ -816,6 +816,14 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,if (ret) goto failed_addition; +/* +* Fixup the number of isolated pageblocks before marking the sections +* onlining, such that undo_isolate_page_range() works correctly. +*/ +spin_lock_irqsave (&zone->lock, flags); + zone->nr_isolate_pageblock += nr_pages / pageblock_nr_pages; +spin_unlock_irqrestore (&zone->lock, flags); +
I am not entirely happy about this. I am wondering whether it would make more sense to keep the counter in sync already in memmap_init_zone. Sure we add a branch to the boot time initialization - and it always fails there - but the code would be cleaner and we wouldn't have to do tricks like this in caller(s).
Signature · 2 lines
Michal Hocko SUSE Labs
Re: [PATCH v1 11/11] mm/memory_hotplug: mark pageblocks MIGRATE_ISOLATE while onlining memory
Quoted 35 lines · Michal Hocko
Quoted 17 lines · David Hildenbrand
Currently, it can happen that pages are allocated (and freed) via the buddy before we finished basic memory onlining.
For example, pages are exposed to the buddy and can be allocated before we actually mark the sections online. Allocated pages could suddenly fail
pfn_to_online_page()checks. We had similar issues with pcp handling, when pages are allocated+freed before we reachzone_pcp_update()inonline_pages()[1].Instead, mark all pageblocks
MIGRATE_ISOLATE, such that allocations are impossible. Once done with the heavy lifting, useundo_isolate_page_range()to move the pages to theMIGRATE_MOVABLEfreelist, marking them ready for allocation. Similar tooffline_pages(), we have to manually adjustzone->nr_isolate_pageblock.Quoted 8 lines
Cc Andrew Morton <[email protected]>Cc Michal Hocko <[email protected]>Cc Wei Yang <[email protected]>Cc Baoquan He <[email protected]>Cc Pankaj Gupta <[email protected]>Cc Oscar Salvador <[email protected]>Cc Charan Teja Reddy <[email protected]>Signed-off-by David Hildenbrand <[email protected]>Acked-by Michal Hocko <[email protected]>Yes this looks very sensible and we should have done that from the beginning. I just have one minor comment below
Quoted 4 lines
mm/memory_hotplug.c +0
-0@@ -816,6 +816,14 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,if (ret) goto failed_addition;
Quoted 14 lines
mm/memory_hotplug.c int __ref online_pages(unsigned long pfn, unsigned long nr_pages, · line 826
Quoted 8 lines
mm/memory_hotplug.c +8
-0+/* +* Fixup the number of isolated pageblocks before marking the sections +* onlining, such that undo_isolate_page_range() works correctly. +*/ +spin_lock_irqsave (&zone->lock, flags); + zone->nr_isolate_pageblock += nr_pages / pageblock_nr_pages; +spin_unlock_irqrestore (&zone->lock, flags); +I am not entirely happy about this. I am wondering whether it would make more sense to keep the counter in sync already in
memmap_init_zone. Sure we add a branch to the boot time initialization - and it always fails there - but the code would be cleaner and we wouldn't have to do tricks like this in caller(s).
I had that in mind initially. The issue is that we have to fixup in case onlining fails, which I consider even more ugly. Also
1. It's the complete reverse of the offlining path now.
2. pageblock flags are essentially stale unless the section is online, my approach moves the handling to the point where nothing else will go wrong and we are just about to mark sections online. That looks a little cleaner to me.
Unless there are strong opinions, I'd prefer to keep it like this.
Thanks for the very fast review Michal!
Signature · 3 lines
Thanks, David / dhildenb