forked from mirrors/linux
		
	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…
	
		Reference in a new issue
	
	 Zi Yan
						Zi Yan