On 14 Mar 2022, at 13:13, David Hildenbrand wrote:
> On 11.03.22 19:36, Zi Yan wrote:
>> From: Zi Yan <ziy@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> ---
>> 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?
>
Sure, I will do that in the next version. Thanks for pointing this out.
The intersection of the pageblock of the “page” and [start_pfn, end_pfn)
will be moved to set_migratetype_isolate() and the resulting range will
be passed into has_unmovable_pages() to avoid redundant unmovable page checks.
--
Best Regards,
Yan, Zi
Attachment:
signature.asc
Description: OpenPGP digital signature
