David Hildenbrand
2022-Mar-14 17:13 UTC
[PATCH v7 2/5] mm: page_isolation: check specified range for unmovable pages
On 11.03.22 19:36, Zi Yan wrote:> From: Zi Yan <ziy at nvidia.com> > > Enable set_migratetype_isolate() to check specified sub-range for > unmovable pages during isolation. Page isolation is done > at max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) granularity, but not all > pages within that granularity are intended to be isolated. For example, > alloc_contig_range(), which uses page isolation, allows ranges without > alignment. This commit makes unmovable page check only look for > interesting pages, so that page isolation can succeed for any > non-overlapping ranges. > > Signed-off-by: Zi Yan <ziy at nvidia.com> > --- > include/linux/page-isolation.h | 10 ++++++++ > mm/page_alloc.c | 13 +--------- > mm/page_isolation.c | 47 +++++++++++++++++++++------------- > 3 files changed, 40 insertions(+), 30 deletions(-) > > diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h > index e14eddf6741a..eb4a208fe907 100644 > --- a/include/linux/page-isolation.h > +++ b/include/linux/page-isolation.h > @@ -15,6 +15,16 @@ static inline bool is_migrate_isolate(int migratetype) > { > return migratetype == MIGRATE_ISOLATE; > } > +static inline unsigned long pfn_max_align_down(unsigned long pfn) > +{ > + return ALIGN_DOWN(pfn, MAX_ORDER_NR_PAGES); > +} > + > +static inline unsigned long pfn_max_align_up(unsigned long pfn) > +{ > + return ALIGN(pfn, MAX_ORDER_NR_PAGES); > +} > + > #else > static inline bool has_isolate_pageblock(struct zone *zone) > { > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 6de57d058d3d..680580a40a35 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -8937,16 +8937,6 @@ void *__init alloc_large_system_hash(const char *tablename, > } > > #ifdef CONFIG_CONTIG_ALLOC > -static unsigned long pfn_max_align_down(unsigned long pfn) > -{ > - return ALIGN_DOWN(pfn, MAX_ORDER_NR_PAGES); > -} > - > -static unsigned long pfn_max_align_up(unsigned long pfn) > -{ > - return ALIGN(pfn, MAX_ORDER_NR_PAGES); > -} > - > #if defined(CONFIG_DYNAMIC_DEBUG) || \ > (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE)) > /* Usage: See admin-guide/dynamic-debug-howto.rst */ > @@ -9091,8 +9081,7 @@ int alloc_contig_range(unsigned long start, unsigned long end, > * put back to page allocator so that buddy can use them. > */ > > - ret = start_isolate_page_range(pfn_max_align_down(start), > - pfn_max_align_up(end), migratetype, 0); > + ret = start_isolate_page_range(start, end, migratetype, 0); > if (ret) > return ret; > > diff --git a/mm/page_isolation.c b/mm/page_isolation.c > index b34f1310aeaa..e0afc3ee8cf9 100644 > --- a/mm/page_isolation.c > +++ b/mm/page_isolation.c > @@ -16,7 +16,8 @@ > #include <trace/events/page_isolation.h> > > /* > - * This function checks whether pageblock includes unmovable pages or not. > + * This function checks whether pageblock within [start_pfn, end_pfn) includes > + * unmovable pages or not. > * > * PageLRU check without isolation or lru_lock could race so that > * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable > @@ -29,11 +30,14 @@ > * > */ > static struct page *has_unmovable_pages(struct zone *zone, struct page *page, > - int migratetype, int flags) > + int migratetype, int flags, > + unsigned long start_pfn, unsigned long end_pfn) > { > - unsigned long iter = 0; > - unsigned long pfn = page_to_pfn(page); > - unsigned long offset = pfn % pageblock_nr_pages; > + unsigned long first_pfn = max(page_to_pfn(page), start_pfn); > + unsigned long pfn = first_pfn; > + unsigned long last_pfn = min(ALIGN(pfn + 1, pageblock_nr_pages), end_pfn); > + > + page = pfn_to_page(pfn);I think we should get rid of the page argument completely. The caller should pass in a reasonable [start_pfn, end_pfn) range, and to any necessary fixups to the range outside of this function. The goal should be to have pfn = start_pfn and replacing last_pfn by end_pfn. Ideally we'd end up with "This function checks whether the range [start_pfn, end_pfn) contains unmovable pages or not." What would be missing to achieve that? -- Thanks, David / dhildenb