mm: fix a potential infinite loop in start_isolate_page_range()
In isolate_single_pageblock() called by start_isolate_page_range(), there
are some pageblock isolation issues causing a potential infinite loop when
isolating a page range. This is reported by Qian Cai.
1. the pageblock was isolated by just changing pageblock migratetype
without checking unmovable pages. Calling set_migratetype_isolate() to
isolate pageblock properly.
2. an off-by-one error caused migrating pages unnecessarily, since the page
is not crossing pageblock boundary.
3. migrating a compound page across pageblock boundary then splitting the
free page later has a small race window that the free page might be
allocated again, so that the code will try again, causing an potential
infinite loop. Temporarily set the to-be-migrated page's pageblock to
MIGRATE_ISOLATE to prevent that and bail out early if no free page is
found after page migration.
An additional fix to split_free_page() aims to avoid crashing in
__free_one_page(). When the free page is split at the specified
split_pfn_offset, free_page_order should check both the first bit of
free_page_pfn and the last bit of split_pfn_offset and use the smaller
one. For example, if free_page_pfn=0x10000, split_pfn_offset=0xc000,
free_page_order should first be 0x8000 then 0x4000, instead of 0x4000 then
0x8000, which the original algorithm did.
[akpm@linux-foundation.org: suppress min() warning]
Link: https://lkml.kernel.org/r/20220524194756.1698351-1-zi.yan@sent.com
Fixes: b2c9e2fbba
("mm: make alloc_contig_range work at pageblock granularity")
Signed-off-by: Zi Yan <ziy@nvidia.com>
Reported-by: Qian Cai <quic_qiancai@quicinc.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: David Hildenbrand <david@redhat.com>
Cc: Eric Ren <renzhengeek@gmail.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
This commit is contained in:
parent
bb5ced41a6
commit
88ee134320
2 changed files with 46 additions and 13 deletions
|
@ -1114,13 +1114,18 @@ void split_free_page(struct page *free_page,
|
||||||
unsigned long flags;
|
unsigned long flags;
|
||||||
int free_page_order;
|
int free_page_order;
|
||||||
|
|
||||||
|
if (split_pfn_offset == 0)
|
||||||
|
return;
|
||||||
|
|
||||||
spin_lock_irqsave(&zone->lock, flags);
|
spin_lock_irqsave(&zone->lock, flags);
|
||||||
del_page_from_free_list(free_page, zone, order);
|
del_page_from_free_list(free_page, zone, order);
|
||||||
for (pfn = free_page_pfn;
|
for (pfn = free_page_pfn;
|
||||||
pfn < free_page_pfn + (1UL << order);) {
|
pfn < free_page_pfn + (1UL << order);) {
|
||||||
int mt = get_pfnblock_migratetype(pfn_to_page(pfn), pfn);
|
int mt = get_pfnblock_migratetype(pfn_to_page(pfn), pfn);
|
||||||
|
|
||||||
free_page_order = ffs(split_pfn_offset) - 1;
|
free_page_order = min_t(int,
|
||||||
|
pfn ? __ffs(pfn) : order,
|
||||||
|
__fls(split_pfn_offset));
|
||||||
__free_one_page(pfn_to_page(pfn), pfn, zone, free_page_order,
|
__free_one_page(pfn_to_page(pfn), pfn, zone, free_page_order,
|
||||||
mt, FPI_NONE);
|
mt, FPI_NONE);
|
||||||
pfn += 1UL << free_page_order;
|
pfn += 1UL << free_page_order;
|
||||||
|
|
|
@ -283,6 +283,7 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
|
||||||
* isolate_single_pageblock() -- tries to isolate a pageblock that might be
|
* isolate_single_pageblock() -- tries to isolate a pageblock that might be
|
||||||
* within a free or in-use page.
|
* within a free or in-use page.
|
||||||
* @boundary_pfn: pageblock-aligned pfn that a page might cross
|
* @boundary_pfn: pageblock-aligned pfn that a page might cross
|
||||||
|
* @flags: isolation flags
|
||||||
* @gfp_flags: GFP flags used for migrating pages
|
* @gfp_flags: GFP flags used for migrating pages
|
||||||
* @isolate_before: isolate the pageblock before the boundary_pfn
|
* @isolate_before: isolate the pageblock before the boundary_pfn
|
||||||
*
|
*
|
||||||
|
@ -298,14 +299,15 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
|
||||||
* either. The function handles this by splitting the free page or migrating
|
* either. The function handles this by splitting the free page or migrating
|
||||||
* the in-use page then splitting the free page.
|
* the in-use page then splitting the free page.
|
||||||
*/
|
*/
|
||||||
static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
|
static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
|
||||||
bool isolate_before)
|
gfp_t gfp_flags, bool isolate_before)
|
||||||
{
|
{
|
||||||
unsigned char saved_mt;
|
unsigned char saved_mt;
|
||||||
unsigned long start_pfn;
|
unsigned long start_pfn;
|
||||||
unsigned long isolate_pageblock;
|
unsigned long isolate_pageblock;
|
||||||
unsigned long pfn;
|
unsigned long pfn;
|
||||||
struct zone *zone;
|
struct zone *zone;
|
||||||
|
int ret;
|
||||||
|
|
||||||
VM_BUG_ON(!IS_ALIGNED(boundary_pfn, pageblock_nr_pages));
|
VM_BUG_ON(!IS_ALIGNED(boundary_pfn, pageblock_nr_pages));
|
||||||
|
|
||||||
|
@ -325,7 +327,11 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
|
||||||
zone->zone_start_pfn);
|
zone->zone_start_pfn);
|
||||||
|
|
||||||
saved_mt = get_pageblock_migratetype(pfn_to_page(isolate_pageblock));
|
saved_mt = get_pageblock_migratetype(pfn_to_page(isolate_pageblock));
|
||||||
set_pageblock_migratetype(pfn_to_page(isolate_pageblock), MIGRATE_ISOLATE);
|
ret = set_migratetype_isolate(pfn_to_page(isolate_pageblock), saved_mt, flags,
|
||||||
|
isolate_pageblock, isolate_pageblock + pageblock_nr_pages);
|
||||||
|
|
||||||
|
if (ret)
|
||||||
|
return ret;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Bail out early when the to-be-isolated pageblock does not form
|
* Bail out early when the to-be-isolated pageblock does not form
|
||||||
|
@ -374,7 +380,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
|
||||||
struct page *head = compound_head(page);
|
struct page *head = compound_head(page);
|
||||||
unsigned long head_pfn = page_to_pfn(head);
|
unsigned long head_pfn = page_to_pfn(head);
|
||||||
|
|
||||||
if (head_pfn + nr_pages < boundary_pfn) {
|
if (head_pfn + nr_pages <= boundary_pfn) {
|
||||||
pfn = head_pfn + nr_pages;
|
pfn = head_pfn + nr_pages;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
@ -386,7 +392,8 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
|
||||||
if (PageHuge(page) || PageLRU(page) || __PageMovable(page)) {
|
if (PageHuge(page) || PageLRU(page) || __PageMovable(page)) {
|
||||||
int order;
|
int order;
|
||||||
unsigned long outer_pfn;
|
unsigned long outer_pfn;
|
||||||
int ret;
|
int page_mt = get_pageblock_migratetype(page);
|
||||||
|
bool isolate_page = !is_migrate_isolate_page(page);
|
||||||
struct compact_control cc = {
|
struct compact_control cc = {
|
||||||
.nr_migratepages = 0,
|
.nr_migratepages = 0,
|
||||||
.order = -1,
|
.order = -1,
|
||||||
|
@ -399,9 +406,31 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
|
||||||
};
|
};
|
||||||
INIT_LIST_HEAD(&cc.migratepages);
|
INIT_LIST_HEAD(&cc.migratepages);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* XXX: mark the page as MIGRATE_ISOLATE so that
|
||||||
|
* no one else can grab the freed page after migration.
|
||||||
|
* Ideally, the page should be freed as two separate
|
||||||
|
* pages to be added into separate migratetype free
|
||||||
|
* lists.
|
||||||
|
*/
|
||||||
|
if (isolate_page) {
|
||||||
|
ret = set_migratetype_isolate(page, page_mt,
|
||||||
|
flags, head_pfn, head_pfn + nr_pages);
|
||||||
|
if (ret)
|
||||||
|
goto failed;
|
||||||
|
}
|
||||||
|
|
||||||
ret = __alloc_contig_migrate_range(&cc, head_pfn,
|
ret = __alloc_contig_migrate_range(&cc, head_pfn,
|
||||||
head_pfn + nr_pages);
|
head_pfn + nr_pages);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* restore the page's migratetype so that it can
|
||||||
|
* be split into separate migratetype free lists
|
||||||
|
* later.
|
||||||
|
*/
|
||||||
|
if (isolate_page)
|
||||||
|
unset_migratetype_isolate(page, page_mt);
|
||||||
|
|
||||||
if (ret)
|
if (ret)
|
||||||
goto failed;
|
goto failed;
|
||||||
/*
|
/*
|
||||||
|
@ -417,10 +446,9 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
|
||||||
order = 0;
|
order = 0;
|
||||||
outer_pfn = pfn;
|
outer_pfn = pfn;
|
||||||
while (!PageBuddy(pfn_to_page(outer_pfn))) {
|
while (!PageBuddy(pfn_to_page(outer_pfn))) {
|
||||||
if (++order >= MAX_ORDER) {
|
/* stop if we cannot find the free page */
|
||||||
outer_pfn = pfn;
|
if (++order >= MAX_ORDER)
|
||||||
break;
|
goto failed;
|
||||||
}
|
|
||||||
outer_pfn &= ~0UL << order;
|
outer_pfn &= ~0UL << order;
|
||||||
}
|
}
|
||||||
pfn = outer_pfn;
|
pfn = outer_pfn;
|
||||||
|
@ -435,7 +463,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
|
||||||
return 0;
|
return 0;
|
||||||
failed:
|
failed:
|
||||||
/* restore the original migratetype */
|
/* restore the original migratetype */
|
||||||
set_pageblock_migratetype(pfn_to_page(isolate_pageblock), saved_mt);
|
unset_migratetype_isolate(pfn_to_page(isolate_pageblock), saved_mt);
|
||||||
return -EBUSY;
|
return -EBUSY;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -496,12 +524,12 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
/* isolate [isolate_start, isolate_start + pageblock_nr_pages) pageblock */
|
/* isolate [isolate_start, isolate_start + pageblock_nr_pages) pageblock */
|
||||||
ret = isolate_single_pageblock(isolate_start, gfp_flags, false);
|
ret = isolate_single_pageblock(isolate_start, flags, gfp_flags, false);
|
||||||
if (ret)
|
if (ret)
|
||||||
return ret;
|
return ret;
|
||||||
|
|
||||||
/* isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock */
|
/* isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock */
|
||||||
ret = isolate_single_pageblock(isolate_end, gfp_flags, true);
|
ret = isolate_single_pageblock(isolate_end, flags, gfp_flags, true);
|
||||||
if (ret) {
|
if (ret) {
|
||||||
unset_migratetype_isolate(pfn_to_page(isolate_start), migratetype);
|
unset_migratetype_isolate(pfn_to_page(isolate_start), migratetype);
|
||||||
return ret;
|
return ret;
|
||||||
|
|
Loading…
Add table
Reference in a new issue