On 16.08.24 13:30, Kefeng Wang wrote:
On 2024/8/16 18:11, David Hildenbrand wrote:
On 16.08.24 06:06, Kefeng Wang wrote:
The gigantic page size may larger than memory block size, so memory
offline always fails in this case after commit b2c9e2fbba32 ("mm: make
alloc_contig_range work at pageblock granularity"),
offline_pages
start_isolate_page_range
start_isolate_page_range(isolate_before=true)
isolate [isolate_start, isolate_start + pageblock_nr_pages)
start_isolate_page_range(isolate_before=false)
isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock
__alloc_contig_migrate_range
isolate_migratepages_range
isolate_migratepages_block
isolate_or_dissolve_huge_page
if (hstate_is_gigantic(h))
return -ENOMEM;
In fact, we don't need to migrate page in page range isolation, for
memory offline path, there is do_migrate_range() to move the pages.
For contig allocation, there is another __alloc_contig_migrate_range()
after isolation to migrate the pages. So fix issue by skipping the
__alloc_contig_migrate_range() in isolate_single_pageblock().
Fixes: b2c9e2fbba32 ("mm: make alloc_contig_range work at pageblock
granularity")
Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
---
mm/page_isolation.c | 28 +++-------------------------
1 file changed, 3 insertions(+), 25 deletions(-)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 39fb8c07aeb7..7e04047977cf 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -403,30 +403,8 @@ static int isolate_single_pageblock(unsigned long
boundary_pfn, int flags,
unsigned long head_pfn = page_to_pfn(head);
unsigned long nr_pages = compound_nr(head);
- if (head_pfn + nr_pages <= boundary_pfn) {
- pfn = head_pfn + nr_pages;
- continue;
- }
-
-#if defined CONFIG_COMPACTION || defined CONFIG_CMA
- if (PageHuge(page)) {
- int page_mt = get_pageblock_migratetype(page);
- struct compact_control cc = {
- .nr_migratepages = 0,
- .order = -1,
- .zone = page_zone(pfn_to_page(head_pfn)),
- .mode = MIGRATE_SYNC,
- .ignore_skip_hint = true,
- .no_set_skip_hint = true,
- .gfp_mask = gfp_flags,
- .alloc_contig = true,
- };
- INIT_LIST_HEAD(&cc.migratepages);
-
- ret = __alloc_contig_migrate_range(&cc, head_pfn,
- head_pfn + nr_pages, page_mt);
- if (ret)
- goto failed;
But won't this break alloc_contig_range() then? I would have expected
that you have to special-case here on the migration reason
(MEMORY_OFFLINE).
Yes, this is what I did in rfc, only skip migration for offline path.
but Zi Yan suggested to remove migration totally[1]
Please distill some of that in the patch description. Right now you only
talk about memory offlining and don't cover why alloc_contig_range() is
fine as well with this change.
Let me explore the details in the meantime ... :)
--
Cheers,
David / dhildenb
