* [PATCH v9 1/7] mm,page_alloc: Bail out earlier on -ENOMEM in alloc_contig_migrate_range
2021-04-16 7:00 [PATCH v9 0/7] Make alloc_contig_range handle Hugetlb pages Oscar Salvador
@ 2021-04-16 7:00 ` Oscar Salvador
2021-04-16 7:00 ` [PATCH v9 2/7] mm,compaction: Let isolate_migratepages_{range,block} return error codes Oscar Salvador
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Oscar Salvador @ 2021-04-16 7:00 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Kravetz, Vlastimil Babka, David Hildenbrand, Michal Hocko,
Muchun Song, linux-mm, linux-kernel, Oscar Salvador,
Michal Hocko
Currently, __alloc_contig_migrate_range can generate -EINTR, -ENOMEM or -EBUSY,
and report them down the chain.
The problem is that when migrate_pages() reports -ENOMEM, we keep going till we
exhaust all the try-attempts (5 at the moment) instead of bailing out.
migrate_pages() bails out right away on -ENOMEM because it is considered a fatal
error. Do the same here instead of keep going and retrying.
Note that this is not fixing a real issue, just a cosmetic change. Although we
can save some cycles by backing off ealier
Signed-off-by: Oscar Salvador <osalvador@suse.de>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: Mike Kravetz <mike.kravetz@oracle.com>
---
mm/page_alloc.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1c67c99603a3..689454692de1 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8697,7 +8697,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
}
tries = 0;
} else if (++tries == 5) {
- ret = ret < 0 ? ret : -EBUSY;
+ ret = -EBUSY;
break;
}
@@ -8707,6 +8707,13 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
ret = migrate_pages(&cc->migratepages, alloc_migration_target,
NULL, (unsigned long)&mtc, cc->mode, MR_CONTIG_RANGE);
+
+ /*
+ * On -ENOMEM, migrate_pages() bails out right away. It is pointless
+ * to retry again over this error, so do the same here.
+ */
+ if (ret == -ENOMEM)
+ break;
}
lru_cache_enable();
--
2.16.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v9 2/7] mm,compaction: Let isolate_migratepages_{range,block} return error codes
2021-04-16 7:00 [PATCH v9 0/7] Make alloc_contig_range handle Hugetlb pages Oscar Salvador
2021-04-16 7:00 ` [PATCH v9 1/7] mm,page_alloc: Bail out earlier on -ENOMEM in alloc_contig_migrate_range Oscar Salvador
@ 2021-04-16 7:00 ` Oscar Salvador
2021-04-16 7:00 ` [PATCH v9 3/7] mm,hugetlb: Drop clearing of flag from prep_new_huge_page Oscar Salvador
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Oscar Salvador @ 2021-04-16 7:00 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Kravetz, Vlastimil Babka, David Hildenbrand, Michal Hocko,
Muchun Song, linux-mm, linux-kernel, Oscar Salvador
Currently, isolate_migratepages_{range,block} and their callers use
a pfn == 0 vs pfn != 0 scheme to let the caller know whether there was
any error during isolation.
This does not work as soon as we need to start reporting different error
codes and make sure we pass them down the chain, so they are properly
interpreted by functions like e.g: alloc_contig_range.
Let us rework isolate_migratepages_{range,block} so we can report error
codes.
Since isolate_migratepages_block will stop returning the next pfn to be
scanned, we reuse the cc->migrate_pfn field to keep track of that.
Signed-off-by: Oscar Salvador <osalvador@suse.de>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Mike Kravetz <mike.kravetz@oracle.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
mm/compaction.c | 52 +++++++++++++++++++++++++---------------------------
mm/internal.h | 10 ++++++++--
mm/page_alloc.c | 7 +++----
3 files changed, 36 insertions(+), 33 deletions(-)
diff --git a/mm/compaction.c b/mm/compaction.c
index 8c5028bfbd56..110b90d5fff5 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -787,15 +787,14 @@ static bool too_many_isolated(pg_data_t *pgdat)
*
* Isolate all pages that can be migrated from the range specified by
* [low_pfn, end_pfn). The range is expected to be within same pageblock.
- * Returns zero if there is a fatal signal pending, otherwise PFN of the
- * first page that was not scanned (which may be both less, equal to or more
- * than end_pfn).
+ * Returns errno, like -EAGAIN or -EINTR in case e.g signal pending or congestion,
+ * or 0.
+ * cc->migrate_pfn will contain the next pfn to scan.
*
* The pages are isolated on cc->migratepages list (not required to be empty),
- * and cc->nr_migratepages is updated accordingly. The cc->migrate_pfn field
- * is neither read nor updated.
+ * and cc->nr_migratepages is updated accordingly.
*/
-static unsigned long
+static int
isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
unsigned long end_pfn, isolate_mode_t isolate_mode)
{
@@ -809,6 +808,9 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
bool skip_on_failure = false;
unsigned long next_skip_pfn = 0;
bool skip_updated = false;
+ int ret = 0;
+
+ cc->migrate_pfn = low_pfn;
/*
* Ensure that there are not too many pages isolated from the LRU
@@ -818,16 +820,16 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
while (unlikely(too_many_isolated(pgdat))) {
/* stop isolation if there are still pages not migrated */
if (cc->nr_migratepages)
- return 0;
+ return -EAGAIN;
/* async migration should just abort */
if (cc->mode == MIGRATE_ASYNC)
- return 0;
+ return -EAGAIN;
congestion_wait(BLK_RW_ASYNC, HZ/10);
if (fatal_signal_pending(current))
- return 0;
+ return -EINTR;
}
cond_resched();
@@ -875,8 +877,8 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
if (fatal_signal_pending(current)) {
cc->contended = true;
+ ret = -EINTR;
- low_pfn = 0;
goto fatal_pending;
}
@@ -1130,7 +1132,9 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
if (nr_isolated)
count_compact_events(COMPACTISOLATED, nr_isolated);
- return low_pfn;
+ cc->migrate_pfn = low_pfn;
+
+ return ret;
}
/**
@@ -1139,15 +1143,14 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
* @start_pfn: The first PFN to start isolating.
* @end_pfn: The one-past-last PFN.
*
- * Returns zero if isolation fails fatally due to e.g. pending signal.
- * Otherwise, function returns one-past-the-last PFN of isolated page
- * (which may be greater than end_pfn if end fell in a middle of a THP page).
+ * Returns -EAGAIN when contented, -EINTR in case of a signal pending or 0.
*/
-unsigned long
+int
isolate_migratepages_range(struct compact_control *cc, unsigned long start_pfn,
unsigned long end_pfn)
{
unsigned long pfn, block_start_pfn, block_end_pfn;
+ int ret = 0;
/* Scan block by block. First and last block may be incomplete */
pfn = start_pfn;
@@ -1166,17 +1169,17 @@ isolate_migratepages_range(struct compact_control *cc, unsigned long start_pfn,
block_end_pfn, cc->zone))
continue;
- pfn = isolate_migratepages_block(cc, pfn, block_end_pfn,
- ISOLATE_UNEVICTABLE);
+ ret = isolate_migratepages_block(cc, pfn, block_end_pfn,
+ ISOLATE_UNEVICTABLE);
- if (!pfn)
+ if (ret)
break;
if (cc->nr_migratepages >= COMPACT_CLUSTER_MAX)
break;
}
- return pfn;
+ return ret;
}
#endif /* CONFIG_COMPACTION || CONFIG_CMA */
@@ -1847,7 +1850,7 @@ static isolate_migrate_t isolate_migratepages(struct compact_control *cc)
*/
for (; block_end_pfn <= cc->free_pfn;
fast_find_block = false,
- low_pfn = block_end_pfn,
+ cc->migrate_pfn = low_pfn = block_end_pfn,
block_start_pfn = block_end_pfn,
block_end_pfn += pageblock_nr_pages) {
@@ -1889,10 +1892,8 @@ static isolate_migrate_t isolate_migratepages(struct compact_control *cc)
}
/* Perform the isolation */
- low_pfn = isolate_migratepages_block(cc, low_pfn,
- block_end_pfn, isolate_mode);
-
- if (!low_pfn)
+ if (isolate_migratepages_block(cc, low_pfn, block_end_pfn,
+ isolate_mode))
return ISOLATE_ABORT;
/*
@@ -1903,9 +1904,6 @@ static isolate_migrate_t isolate_migratepages(struct compact_control *cc)
break;
}
- /* Record where migration scanner will be restarted. */
- cc->migrate_pfn = low_pfn;
-
return cc->nr_migratepages ? ISOLATE_SUCCESS : ISOLATE_NONE;
}
diff --git a/mm/internal.h b/mm/internal.h
index f469f69309de..46eb82eaa195 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -244,7 +244,13 @@ struct compact_control {
unsigned int nr_freepages; /* Number of isolated free pages */
unsigned int nr_migratepages; /* Number of pages to migrate */
unsigned long free_pfn; /* isolate_freepages search base */
- unsigned long migrate_pfn; /* isolate_migratepages search base */
+ /*
+ * Acts as an in/out parameter to page isolation for migration.
+ * isolate_migratepages uses it as a search base.
+ * isolate_migratepages_block will update the value to the next pfn
+ * after the last isolated one.
+ */
+ unsigned long migrate_pfn;
unsigned long fast_start_pfn; /* a pfn to start linear scan from */
struct zone *zone;
unsigned long total_migrate_scanned;
@@ -280,7 +286,7 @@ struct capture_control {
unsigned long
isolate_freepages_range(struct compact_control *cc,
unsigned long start_pfn, unsigned long end_pfn);
-unsigned long
+int
isolate_migratepages_range(struct compact_control *cc,
unsigned long low_pfn, unsigned long end_pfn);
int find_suitable_fallback(struct free_area *area, unsigned int order,
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 689454692de1..b5a94de3cdde 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8690,11 +8690,10 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
if (list_empty(&cc->migratepages)) {
cc->nr_migratepages = 0;
- pfn = isolate_migratepages_range(cc, pfn, end);
- if (!pfn) {
- ret = -EINTR;
+ ret = isolate_migratepages_range(cc, pfn, end);
+ if (ret && ret != -EAGAIN)
break;
- }
+ pfn = cc->migrate_pfn;
tries = 0;
} else if (++tries == 5) {
ret = -EBUSY;
--
2.16.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v9 3/7] mm,hugetlb: Drop clearing of flag from prep_new_huge_page
2021-04-16 7:00 [PATCH v9 0/7] Make alloc_contig_range handle Hugetlb pages Oscar Salvador
2021-04-16 7:00 ` [PATCH v9 1/7] mm,page_alloc: Bail out earlier on -ENOMEM in alloc_contig_migrate_range Oscar Salvador
2021-04-16 7:00 ` [PATCH v9 2/7] mm,compaction: Let isolate_migratepages_{range,block} return error codes Oscar Salvador
@ 2021-04-16 7:00 ` Oscar Salvador
2021-04-16 7:00 ` [PATCH v9 4/7] mm,hugetlb: Split prep_new_huge_page functionality Oscar Salvador
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Oscar Salvador @ 2021-04-16 7:00 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Kravetz, Vlastimil Babka, David Hildenbrand, Michal Hocko,
Muchun Song, linux-mm, linux-kernel, Oscar Salvador,
Michal Hocko
Pages allocated via the page allocator or CMA get its private field cleared
by means of post_alloc_hook().
Pages allocated during boot, that is directly from the memblock allocator,
get cleared by paging_init()->..->memmap_init_zone->..->__init_single_page()
before any memblock allocation.
Based on this ground, let us remove the clearing of the flag from
prep_new_huge_page() as it is not needed.
This was a leftover from 6c0371490140 ("hugetlb: convert PageHugeFreed
to HPageFreed flag"). Previously the explicit clearing was necessary
because compound allocations do not get this initialization
(see prep_compound_page).
Signed-off-by: Oscar Salvador <osalvador@suse.de>
Acked-by: Michal Hocko <mhocko@suse.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
---
mm/hugetlb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 54d81d5947ed..2cb9fa79cbaa 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1493,7 +1493,6 @@ static void prep_new_huge_page(struct hstate *h, struct page *page, int nid)
spin_lock_irq(&hugetlb_lock);
h->nr_huge_pages++;
h->nr_huge_pages_node[nid]++;
- ClearHPageFreed(page);
spin_unlock_irq(&hugetlb_lock);
}
--
2.16.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v9 4/7] mm,hugetlb: Split prep_new_huge_page functionality
2021-04-16 7:00 [PATCH v9 0/7] Make alloc_contig_range handle Hugetlb pages Oscar Salvador
` (2 preceding siblings ...)
2021-04-16 7:00 ` [PATCH v9 3/7] mm,hugetlb: Drop clearing of flag from prep_new_huge_page Oscar Salvador
@ 2021-04-16 7:00 ` Oscar Salvador
2021-04-16 7:00 ` [PATCH v9 5/7] mm: Make alloc_contig_range handle free hugetlb pages Oscar Salvador
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Oscar Salvador @ 2021-04-16 7:00 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Kravetz, Vlastimil Babka, David Hildenbrand, Michal Hocko,
Muchun Song, linux-mm, linux-kernel, Oscar Salvador,
Michal Hocko
Currently, prep_new_huge_page() performs two functions.
It sets the right state for a new hugetlb, and increases the hstate's
counters to account for the new page.
Let us split its functionality into two separate functions, decoupling
the handling of the counters from initializing a hugepage.
The outcome is having __prep_new_huge_page(), which only
initializes the page , and __prep_account_new_huge_page(), which adds
the new page to the hstate's counters.
This allows us to be able to set a hugetlb without having to worry
about the counter/locking. It will prove useful in the next patch.
prep_new_huge_page() still calls both functions.
Signed-off-by: Oscar Salvador <osalvador@suse.de>
Acked-by: Michal Hocko <mhocko@suse.com>
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
mm/hugetlb.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2cb9fa79cbaa..6f39ec79face 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1483,16 +1483,30 @@ void free_huge_page(struct page *page)
}
}
-static void prep_new_huge_page(struct hstate *h, struct page *page, int nid)
+/*
+ * Must be called with the hugetlb lock held
+ */
+static void __prep_account_new_huge_page(struct hstate *h, int nid)
+{
+ lockdep_assert_held(&hugetlb_lock);
+ h->nr_huge_pages++;
+ h->nr_huge_pages_node[nid]++;
+}
+
+static void __prep_new_huge_page(struct page *page)
{
INIT_LIST_HEAD(&page->lru);
set_compound_page_dtor(page, HUGETLB_PAGE_DTOR);
hugetlb_set_page_subpool(page, NULL);
set_hugetlb_cgroup(page, NULL);
set_hugetlb_cgroup_rsvd(page, NULL);
+}
+
+static void prep_new_huge_page(struct hstate *h, struct page *page, int nid)
+{
+ __prep_new_huge_page(page);
spin_lock_irq(&hugetlb_lock);
- h->nr_huge_pages++;
- h->nr_huge_pages_node[nid]++;
+ __prep_account_new_huge_page(h, nid);
spin_unlock_irq(&hugetlb_lock);
}
--
2.16.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v9 5/7] mm: Make alloc_contig_range handle free hugetlb pages
2021-04-16 7:00 [PATCH v9 0/7] Make alloc_contig_range handle Hugetlb pages Oscar Salvador
` (3 preceding siblings ...)
2021-04-16 7:00 ` [PATCH v9 4/7] mm,hugetlb: Split prep_new_huge_page functionality Oscar Salvador
@ 2021-04-16 7:00 ` Oscar Salvador
2021-04-16 9:49 ` Baoquan He
2021-04-16 7:00 ` [PATCH v9 6/7] mm: Make alloc_contig_range handle in-use " Oscar Salvador
2021-04-16 7:00 ` [PATCH v9 7/7] mm,page_alloc: Drop unnecessary checks from pfn_range_valid_contig Oscar Salvador
6 siblings, 1 reply; 11+ messages in thread
From: Oscar Salvador @ 2021-04-16 7:00 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Kravetz, Vlastimil Babka, David Hildenbrand, Michal Hocko,
Muchun Song, linux-mm, linux-kernel, Oscar Salvador,
Michal Hocko
alloc_contig_range will fail if it ever sees a HugeTLB page within the
range we are trying to allocate, even when that page is free and can be
easily reallocated.
This has proved to be problematic for some users of alloc_contic_range,
e.g: CMA and virtio-mem, where those would fail the call even when those
pages lay in ZONE_MOVABLE and are free.
We can do better by trying to replace such page.
Free hugepages are tricky to handle so as to no userspace application
notices disruption, we need to replace the current free hugepage with
a new one.
In order to do that, a new function called alloc_and_dissolve_huge_page
is introduced.
This function will first try to get a new fresh hugepage, and if it
succeeds, it will replace the old one in the free hugepage pool.
The free page replacement is done under hugetlb_lock, so no external
users of hugetlb will notice the change.
To allocate the new huge page, we use alloc_buddy_huge_page(), so we
do not have to deal with any counters, and prep_new_huge_page() is not
called. This is valulable because in case we need to free the new page,
we only need to call __free_pages().
Once we know that the page to be replaced is a genuine 0-refcounted
huge page, we remove the old page from the freelist by remove_hugetlb_page().
Then, we can call __prep_new_huge_page() and __prep_account_new_huge_page()
for the new huge page to properly initialize it and increment the
hstate->nr_huge_pages counter (previously decremented by
remove_hugetlb_page()).
Once done, the page is enqueued by enqueue_huge_page() and it is ready
to be used.
There is one tricky case when
page's refcount is 0 because it is in the process of being released.
A missing PageHugeFreed bit will tell us that freeing is in flight so
we retry after dropping the hugetlb_lock. The race window should be
small and the next retry should make a forward progress.
E.g:
CPU0 CPU1
free_huge_page() isolate_or_dissolve_huge_page
PageHuge() == T
alloc_and_dissolve_huge_page
alloc_buddy_huge_page()
spin_lock_irq(hugetlb_lock)
// PageHuge() && !PageHugeFreed &&
// !PageCount()
spin_unlock_irq(hugetlb_lock)
spin_lock_irq(hugetlb_lock)
1) update_and_free_page
PageHuge() == F
__free_pages()
2) enqueue_huge_page
SetPageHugeFreed()
spin_unlock_irq(&hugetlb_lock)
spin_lock_irq(hugetlb_lock)
1) PageHuge() == F (freed by case#1 from CPU0)
2) PageHuge() == T
PageHugeFreed() == T
- proceed with replacing the page
In the case above we retry as the window race is quite small and we have high
chances to succeed next time.
With regard to the allocation, we restrict it to the node the page belongs
to with __GFP_THISNODE, meaning we do not fallback on other node's zones.
Note that gigantic hugetlb pages are fenced off since there is a cyclic
dependency between them and alloc_contig_range.
Signed-off-by: Oscar Salvador <osalvador@suse.de>
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
---
include/linux/hugetlb.h | 6 +++
mm/compaction.c | 33 ++++++++++++--
mm/hugetlb.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 152 insertions(+), 3 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 09f1fd12a6fa..b2d2118bfd1a 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -595,6 +595,7 @@ struct huge_bootmem_page {
struct hstate *hstate;
};
+int isolate_or_dissolve_huge_page(struct page *page);
struct page *alloc_huge_page(struct vm_area_struct *vma,
unsigned long addr, int avoid_reserve);
struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid,
@@ -877,6 +878,11 @@ static inline void huge_ptep_modify_prot_commit(struct vm_area_struct *vma,
#else /* CONFIG_HUGETLB_PAGE */
struct hstate {};
+static inline int isolate_or_dissolve_huge_page(struct page *page)
+{
+ return -ENOMEM;
+}
+
static inline struct page *alloc_huge_page(struct vm_area_struct *vma,
unsigned long addr,
int avoid_reserve)
diff --git a/mm/compaction.c b/mm/compaction.c
index 110b90d5fff5..af2e8e194e50 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -788,7 +788,7 @@ static bool too_many_isolated(pg_data_t *pgdat)
* Isolate all pages that can be migrated from the range specified by
* [low_pfn, end_pfn). The range is expected to be within same pageblock.
* Returns errno, like -EAGAIN or -EINTR in case e.g signal pending or congestion,
- * or 0.
+ * -ENOMEM in case we could not allocate a page, or 0.
* cc->migrate_pfn will contain the next pfn to scan.
*
* The pages are isolated on cc->migratepages list (not required to be empty),
@@ -906,6 +906,29 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
valid_page = page;
}
+ if (PageHuge(page) && cc->alloc_contig) {
+ ret = isolate_or_dissolve_huge_page(page);
+
+ /*
+ * Fail isolation in case isolate_or_dissolve_huge_page()
+ * reports an error. In case of -ENOMEM, abort right away.
+ */
+ if (ret < 0) {
+ /* Do not report -EBUSY down the chain */
+ if (ret == -EBUSY)
+ ret = 0;
+ low_pfn += (1UL << compound_order(page)) - 1;
+ goto isolate_fail;
+ }
+
+ /*
+ * Ok, the hugepage was dissolved. Now these pages are
+ * Buddy and cannot be re-allocated because they are
+ * isolated. Fall-through as the check below handles
+ * Buddy pages.
+ */
+ }
+
/*
* Skip if free. We read page order here without zone lock
* which is generally unsafe, but the race window is small and
@@ -1065,7 +1088,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
put_page(page);
isolate_fail:
- if (!skip_on_failure)
+ if (!skip_on_failure && ret != -ENOMEM)
continue;
/*
@@ -1091,6 +1114,9 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
*/
next_skip_pfn += 1UL << cc->order;
}
+
+ if (ret == -ENOMEM)
+ break;
}
/*
@@ -1143,7 +1169,8 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
* @start_pfn: The first PFN to start isolating.
* @end_pfn: The one-past-last PFN.
*
- * Returns -EAGAIN when contented, -EINTR in case of a signal pending or 0.
+ * Returns -EAGAIN when contented, -EINTR in case of a signal pending, -ENOMEM
+ * in case we could not allocate a page, or 0.
*/
int
isolate_migratepages_range(struct compact_control *cc, unsigned long start_pfn,
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 6f39ec79face..c588eed97c5a 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2266,6 +2266,122 @@ static void restore_reserve_on_error(struct hstate *h,
}
}
+/*
+ * alloc_and_dissolve_huge_page - Allocate a new page and dissolve the old one
+ * @h: struct hstate old page belongs to
+ * @old_page: Old page to dissolve
+ * Returns 0 on success, otherwise negated error.
+ */
+static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page)
+{
+ gfp_t gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE;
+ int nid = page_to_nid(old_page);
+ struct page *new_page;
+ int ret = 0;
+
+ /*
+ * Before dissolving the page, we need to allocate a new one for the
+ * pool to remain stable. Using alloc_buddy_huge_page() allows us to
+ * not having to deal with prep_new_page() and avoids dealing of any
+ * counters. This simplifies and let us do the whole thing under the
+ * lock.
+ */
+ new_page = alloc_buddy_huge_page(h, gfp_mask, nid, NULL, NULL);
+ if (!new_page)
+ return -ENOMEM;
+
+retry:
+ spin_lock_irq(&hugetlb_lock);
+ if (!PageHuge(old_page)) {
+ /*
+ * Freed from under us. Drop new_page too.
+ */
+ goto free_new;
+ } else if (page_count(old_page)) {
+ /*
+ * Someone has grabbed the page, fail for now.
+ */
+ ret = -EBUSY;
+ goto free_new;
+ } else if (!HPageFreed(old_page)) {
+ /*
+ * Page's refcount is 0 but it has not been enqueued in the
+ * freelist yet. Race window is small, so we can succeed here if
+ * we retry.
+ */
+ spin_unlock_irq(&hugetlb_lock);
+ cond_resched();
+ goto retry;
+ } else {
+ /*
+ * Ok, old_page is still a genuine free hugepage. Remove it from
+ * the freelist and decrease the counters. These will be
+ * incremented again when calling __prep_account_new_huge_page()
+ * and enqueue_huge_page() for new_page. The counters will remain
+ * stable since this happens under the lock.
+ */
+ remove_hugetlb_page(h, old_page, false);
+
+ /*
+ * new_page needs to be initialized with the standard hugetlb
+ * state. This is normally done by prep_new_huge_page() but
+ * that takes hugetlb_lock which is already held so we need to
+ * open code it here.
+ * Reference count trick is needed because allocator gives us
+ * referenced page but the pool requires pages with 0 refcount.
+ */
+ __prep_new_huge_page(new_page);
+ __prep_account_new_huge_page(h, nid);
+ page_ref_dec(new_page);
+ enqueue_huge_page(h, new_page);
+
+ /*
+ * Pages have been replaced, we can safely free the old one.
+ */
+ spin_unlock_irq(&hugetlb_lock);
+ update_and_free_page(h, old_page);
+ }
+
+ return ret;
+
+free_new:
+ spin_unlock_irq(&hugetlb_lock);
+ __free_pages(new_page, huge_page_order(h));
+
+ return ret;
+}
+
+int isolate_or_dissolve_huge_page(struct page *page)
+{
+ struct hstate *h;
+ struct page *head;
+
+ /*
+ * The page might have been dissolved from under our feet, so make sure
+ * to carefully check the state under the lock.
+ * Return success when racing as if we dissolved the page ourselves.
+ */
+ spin_lock_irq(&hugetlb_lock);
+ if (PageHuge(page)) {
+ head = compound_head(page);
+ h = page_hstate(head);
+ } else {
+ spin_unlock_irq(&hugetlb_lock);
+ return 0;
+ }
+ spin_unlock_irq(&hugetlb_lock);
+
+ /*
+ * Fence off gigantic pages as there is a cyclic dependency between
+ * alloc_contig_range and them. Return -ENOME as this has the effect
+ * of bailing out right away without further retrying.
+ */
+ if (hstate_is_gigantic(h))
+ return -ENOMEM;
+
+ return alloc_and_dissolve_huge_page(h, head);
+}
+
struct page *alloc_huge_page(struct vm_area_struct *vma,
unsigned long addr, int avoid_reserve)
{
--
2.16.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v9 5/7] mm: Make alloc_contig_range handle free hugetlb pages
2021-04-16 7:00 ` [PATCH v9 5/7] mm: Make alloc_contig_range handle free hugetlb pages Oscar Salvador
@ 2021-04-16 9:49 ` Baoquan He
2021-04-16 12:14 ` Oscar Salvador
0 siblings, 1 reply; 11+ messages in thread
From: Baoquan He @ 2021-04-16 9:49 UTC (permalink / raw)
To: Oscar Salvador
Cc: Andrew Morton, Mike Kravetz, Vlastimil Babka, David Hildenbrand,
Michal Hocko, Muchun Song, linux-mm, linux-kernel, Michal Hocko
On 04/16/21 at 09:00am, Oscar Salvador wrote:
...
> +/*
> + * alloc_and_dissolve_huge_page - Allocate a new page and dissolve the old one
> + * @h: struct hstate old page belongs to
> + * @old_page: Old page to dissolve
> + * Returns 0 on success, otherwise negated error.
> + */
> +static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page)
> +{
> + gfp_t gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE;
> + int nid = page_to_nid(old_page);
> + struct page *new_page;
> + int ret = 0;
> +
> + /*
> + * Before dissolving the page, we need to allocate a new one for the
> + * pool to remain stable. Using alloc_buddy_huge_page() allows us to
> + * not having to deal with prep_new_page() and avoids dealing of any
~ prep_new_huge_page() ?
> + * counters. This simplifies and let us do the whole thing under the
> + * lock.
> + */
> + new_page = alloc_buddy_huge_page(h, gfp_mask, nid, NULL, NULL);
> + if (!new_page)
> + return -ENOMEM;
> +
> +retry:
> + spin_lock_irq(&hugetlb_lock);
...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v9 5/7] mm: Make alloc_contig_range handle free hugetlb pages
2021-04-16 9:49 ` Baoquan He
@ 2021-04-16 12:14 ` Oscar Salvador
0 siblings, 0 replies; 11+ messages in thread
From: Oscar Salvador @ 2021-04-16 12:14 UTC (permalink / raw)
To: Baoquan He
Cc: Andrew Morton, Mike Kravetz, Vlastimil Babka, David Hildenbrand,
Michal Hocko, Muchun Song, linux-mm, linux-kernel, Michal Hocko
On Fri, Apr 16, 2021 at 05:49:20PM +0800, Baoquan He wrote:
> On 04/16/21 at 09:00am, Oscar Salvador wrote:
> ...
> > +/*
> > + * alloc_and_dissolve_huge_page - Allocate a new page and dissolve the old one
> > + * @h: struct hstate old page belongs to
> > + * @old_page: Old page to dissolve
> > + * Returns 0 on success, otherwise negated error.
> > + */
> > +static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page)
> > +{
> > + gfp_t gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE;
> > + int nid = page_to_nid(old_page);
> > + struct page *new_page;
> > + int ret = 0;
> > +
> > + /*
> > + * Before dissolving the page, we need to allocate a new one for the
> > + * pool to remain stable. Using alloc_buddy_huge_page() allows us to
> > + * not having to deal with prep_new_page() and avoids dealing of any
> ~ prep_new_huge_page() ?
Yep, clumsy.
@Andrew: could you please amend that or should I send a new version?
Thanks
--
Oscar Salvador
SUSE L3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v9 6/7] mm: Make alloc_contig_range handle in-use hugetlb pages
2021-04-16 7:00 [PATCH v9 0/7] Make alloc_contig_range handle Hugetlb pages Oscar Salvador
` (4 preceding siblings ...)
2021-04-16 7:00 ` [PATCH v9 5/7] mm: Make alloc_contig_range handle free hugetlb pages Oscar Salvador
@ 2021-04-16 7:00 ` Oscar Salvador
2021-04-16 8:36 ` David Hildenbrand
2021-04-16 7:00 ` [PATCH v9 7/7] mm,page_alloc: Drop unnecessary checks from pfn_range_valid_contig Oscar Salvador
6 siblings, 1 reply; 11+ messages in thread
From: Oscar Salvador @ 2021-04-16 7:00 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Kravetz, Vlastimil Babka, David Hildenbrand, Michal Hocko,
Muchun Song, linux-mm, linux-kernel, Oscar Salvador,
Michal Hocko
alloc_contig_range() will fail if it finds a HugeTLB page within the range,
without a chance to handle them. Since HugeTLB pages can be migrated as any
LRU or Movable page, it does not make sense to bail out without trying.
Enable the interface to recognize in-use HugeTLB pages so we can migrate
them, and have much better chances to succeed the call.
Signed-off-by: Oscar Salvador <osalvador@suse.de>
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Acked-by: Michal Hocko <mhocko@suse.com>
---
include/linux/hugetlb.h | 5 +++--
mm/compaction.c | 12 +++++++++++-
mm/hugetlb.c | 24 ++++++++++++++++++------
mm/vmscan.c | 5 +++--
4 files changed, 35 insertions(+), 11 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index b2d2118bfd1a..b92f25ccef58 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -595,7 +595,7 @@ struct huge_bootmem_page {
struct hstate *hstate;
};
-int isolate_or_dissolve_huge_page(struct page *page);
+int isolate_or_dissolve_huge_page(struct page *page, struct list_head *list);
struct page *alloc_huge_page(struct vm_area_struct *vma,
unsigned long addr, int avoid_reserve);
struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid,
@@ -878,7 +878,8 @@ static inline void huge_ptep_modify_prot_commit(struct vm_area_struct *vma,
#else /* CONFIG_HUGETLB_PAGE */
struct hstate {};
-static inline int isolate_or_dissolve_huge_page(struct page *page)
+static inline int isolate_or_dissolve_huge_page(struct page *page,
+ struct list_head *list)
{
return -ENOMEM;
}
diff --git a/mm/compaction.c b/mm/compaction.c
index af2e8e194e50..84fde270ae74 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -907,7 +907,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
}
if (PageHuge(page) && cc->alloc_contig) {
- ret = isolate_or_dissolve_huge_page(page);
+ ret = isolate_or_dissolve_huge_page(page, &cc->migratepages);
/*
* Fail isolation in case isolate_or_dissolve_huge_page()
@@ -921,6 +921,15 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
goto isolate_fail;
}
+ if (PageHuge(page)) {
+ /*
+ * Hugepage was successfully isolated and placed
+ * on the cc->migratepages list.
+ */
+ low_pfn += compound_nr(page) - 1;
+ goto isolate_success_no_list;
+ }
+
/*
* Ok, the hugepage was dissolved. Now these pages are
* Buddy and cannot be re-allocated because they are
@@ -1062,6 +1071,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
isolate_success:
list_add(&page->lru, &cc->migratepages);
+isolate_success_no_list:
cc->nr_migratepages += compound_nr(page);
nr_isolated += compound_nr(page);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index c588eed97c5a..6eb3b6ca4264 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2270,9 +2270,11 @@ static void restore_reserve_on_error(struct hstate *h,
* alloc_and_dissolve_huge_page - Allocate a new page and dissolve the old one
* @h: struct hstate old page belongs to
* @old_page: Old page to dissolve
+ * @list: List to isolate the page in case we need to
* Returns 0 on success, otherwise negated error.
*/
-static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page)
+static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page,
+ struct list_head *list)
{
gfp_t gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE;
int nid = page_to_nid(old_page);
@@ -2299,9 +2301,13 @@ static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page)
goto free_new;
} else if (page_count(old_page)) {
/*
- * Someone has grabbed the page, fail for now.
+ * Someone has grabbed the page, try to isolate it here.
+ * Fail with -EBUSY if not possible.
*/
- ret = -EBUSY;
+ spin_unlock_irq(&hugetlb_lock);
+ if (!isolate_huge_page(old_page, list))
+ ret = -EBUSY;
+ spin_lock_irq(&hugetlb_lock);
goto free_new;
} else if (!HPageFreed(old_page)) {
/*
@@ -2351,10 +2357,11 @@ static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page)
return ret;
}
-int isolate_or_dissolve_huge_page(struct page *page)
+int isolate_or_dissolve_huge_page(struct page *page, struct list_head *list)
{
struct hstate *h;
struct page *head;
+ int ret = -EBUSY;
/*
* The page might have been dissolved from under our feet, so make sure
@@ -2373,13 +2380,18 @@ int isolate_or_dissolve_huge_page(struct page *page)
/*
* Fence off gigantic pages as there is a cyclic dependency between
- * alloc_contig_range and them. Return -ENOME as this has the effect
+ * alloc_contig_range and them. Return -ENOMEM as this has the effect
* of bailing out right away without further retrying.
*/
if (hstate_is_gigantic(h))
return -ENOMEM;
- return alloc_and_dissolve_huge_page(h, head);
+ if (page_count(head) && isolate_huge_page(head, list))
+ ret = 0;
+ else if (!page_count(head))
+ ret = alloc_and_dissolve_huge_page(h, head, list);
+
+ return ret;
}
struct page *alloc_huge_page(struct vm_area_struct *vma,
diff --git a/mm/vmscan.c b/mm/vmscan.c
index bb8321026c0c..5199b9696bab 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1703,8 +1703,9 @@ unsigned int reclaim_clean_pages_from_list(struct zone *zone,
LIST_HEAD(clean_pages);
list_for_each_entry_safe(page, next, page_list, lru) {
- if (page_is_file_lru(page) && !PageDirty(page) &&
- !__PageMovable(page) && !PageUnevictable(page)) {
+ if (!PageHuge(page) && page_is_file_lru(page) &&
+ !PageDirty(page) && !__PageMovable(page) &&
+ !PageUnevictable(page)) {
ClearPageActive(page);
list_move(&page->lru, &clean_pages);
}
--
2.16.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v9 6/7] mm: Make alloc_contig_range handle in-use hugetlb pages
2021-04-16 7:00 ` [PATCH v9 6/7] mm: Make alloc_contig_range handle in-use " Oscar Salvador
@ 2021-04-16 8:36 ` David Hildenbrand
0 siblings, 0 replies; 11+ messages in thread
From: David Hildenbrand @ 2021-04-16 8:36 UTC (permalink / raw)
To: Oscar Salvador, Andrew Morton
Cc: Mike Kravetz, Vlastimil Babka, Michal Hocko, Muchun Song,
linux-mm, linux-kernel, Michal Hocko
> -int isolate_or_dissolve_huge_page(struct page *page)
> +int isolate_or_dissolve_huge_page(struct page *page, struct list_head *list)
> {
> struct hstate *h;
> struct page *head;
> + int ret = -EBUSY;
>
> /*
> * The page might have been dissolved from under our feet, so make sure
> @@ -2373,13 +2380,18 @@ int isolate_or_dissolve_huge_page(struct page *page)
>
> /*
> * Fence off gigantic pages as there is a cyclic dependency between
> - * alloc_contig_range and them. Return -ENOME as this has the effect
> + * alloc_contig_range and them. Return -ENOMEM as this has the effect
Nit: belongs into previous patch.
> * of bailing out right away without further retrying.
> */
> if (hstate_is_gigantic(h))
> return -ENOMEM;
>
> - return alloc_and_dissolve_huge_page(h, head);
> + if (page_count(head) && isolate_huge_page(head, list))
> + ret = 0;
> + else if (!page_count(head))
> + ret = alloc_and_dissolve_huge_page(h, head, list);
> +
> + return ret;
> }
>
> struct page *alloc_huge_page(struct vm_area_struct *vma,
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bb8321026c0c..5199b9696bab 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1703,8 +1703,9 @@ unsigned int reclaim_clean_pages_from_list(struct zone *zone,
> LIST_HEAD(clean_pages);
>
> list_for_each_entry_safe(page, next, page_list, lru) {
> - if (page_is_file_lru(page) && !PageDirty(page) &&
> - !__PageMovable(page) && !PageUnevictable(page)) {
> + if (!PageHuge(page) && page_is_file_lru(page) &&
> + !PageDirty(page) && !__PageMovable(page) &&
> + !PageUnevictable(page)) {
Nit: adding to the end of the list would require less modifications ;)
> ClearPageActive(page);
> list_move(&page->lru, &clean_pages);
> }
>
Acked-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v9 7/7] mm,page_alloc: Drop unnecessary checks from pfn_range_valid_contig
2021-04-16 7:00 [PATCH v9 0/7] Make alloc_contig_range handle Hugetlb pages Oscar Salvador
` (5 preceding siblings ...)
2021-04-16 7:00 ` [PATCH v9 6/7] mm: Make alloc_contig_range handle in-use " Oscar Salvador
@ 2021-04-16 7:00 ` Oscar Salvador
6 siblings, 0 replies; 11+ messages in thread
From: Oscar Salvador @ 2021-04-16 7:00 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Kravetz, Vlastimil Babka, David Hildenbrand, Michal Hocko,
Muchun Song, linux-mm, linux-kernel, Oscar Salvador,
Michal Hocko
pfn_range_valid_contig() bails out when it finds an in-use page or a
hugetlb page, among other things.
We can drop the in-use page check since __alloc_contig_pages can migrate
away those pages, and the hugetlb page check can go too since
isolate_migratepages_range is now capable of dealing with hugetlb pages.
Either way, those checks are racy so let the end function handle it
when the time comes.
Signed-off-by: Oscar Salvador <osalvador@suse.de>
Suggested-by: David Hildenbrand <david@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Mike Kravetz <mike.kravetz@oracle.com>
Acked-by: Michal Hocko <mhocko@suse.com>
---
mm/page_alloc.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index b5a94de3cdde..c5338e912ace 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8901,12 +8901,6 @@ static bool pfn_range_valid_contig(struct zone *z, unsigned long start_pfn,
if (PageReserved(page))
return false;
-
- if (page_count(page) > 0)
- return false;
-
- if (PageHuge(page))
- return false;
}
return true;
}
--
2.16.3
^ permalink raw reply related [flat|nested] 11+ messages in thread