Re: [PATCH v1 2/4] mm/compaction: add support for >0 order folio memory compaction.

[Zi Yan <ziy@nvidia.com>]

[-- Attachment #1: Type: text/plain, Size: 3648 bytes --]

On 9 Jan 2024, at 10:18, Ryan Roberts wrote:

> On 13/11/2023 17:01, Zi Yan wrote:
>> From: Zi Yan <ziy@nvidia.com>
>>
>> Before, memory compaction only migrates order-0 folios and skips >0 order
>> folios. This commit adds support for >0 order folio compaction by keeping
>> isolated free pages at their original size without splitting them into
>> order-0 pages and using them directly during migration process.
>>
>> What is different from the prior implementation:
>> 1. All isolated free pages are kept in a MAX_ORDER+1 array of page lists,
>>    where each page list stores free pages in the same order.
>> 2. All free pages are not post_alloc_hook() processed nor buddy pages,
>>    although their orders are stored in first page's private like buddy
>>    pages.
>> 3. During migration, in new page allocation time (i.e., in
>>    compaction_alloc()), free pages are then processed by post_alloc_hook().
>>    When migration fails and a new page is returned (i.e., in
>>    compaction_free()), free pages are restored by reversing the
>>    post_alloc_hook() operations.
>>
>> Step 3 is done for a latter optimization that splitting and/or merging free
>> pages during compaction becomes easier.
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>>  mm/compaction.c | 160 ++++++++++++++++++++++++++++++------------------
>>  mm/internal.h   |   7 ++-
>>  2 files changed, 108 insertions(+), 59 deletions(-)
>>
>> diff --git a/mm/compaction.c b/mm/compaction.c
>> index 5217dd35b493..ec6b5cc7e907 100644
>> --- a/mm/compaction.c
>> +++ b/mm/compaction.c
>> @@ -66,45 +66,64 @@ static inline void count_compact_events(enum vm_event_item item, long delta)
>>  #define COMPACTION_HPAGE_ORDER	(PMD_SHIFT - PAGE_SHIFT)
>>  #endif
>>
>> -static unsigned long release_freepages(struct list_head *freelist)
>> +static void init_page_list(struct page_list *p)
>>  {
>> -	struct page *page, *next;
>> -	unsigned long high_pfn = 0;
>> -
>> -	list_for_each_entry_safe(page, next, freelist, lru) {
>> -		unsigned long pfn = page_to_pfn(page);
>> -		list_del(&page->lru);
>> -		__free_page(page);
>> -		if (pfn > high_pfn)
>> -			high_pfn = pfn;
>> -	}
>> -
>> -	return high_pfn;
>> +	INIT_LIST_HEAD(&p->pages);
>> +	p->nr_pages = 0;
>>  }
>>
>> -static void split_map_pages(struct list_head *list)
>> +static void split_map_pages(struct page_list *freepages)
>>  {
>>  	unsigned int i, order, nr_pages;
>>  	struct page *page, *next;
>>  	LIST_HEAD(tmp_list);
>>
>> -	list_for_each_entry_safe(page, next, list, lru) {
>> -		list_del(&page->lru);
>> +	for (order = 0; order <= MAX_ORDER; order++) {
>> +		freepages[order].nr_pages = 0;
>> +
>> +		list_for_each_entry_safe(page, next, &freepages[order].pages, lru) {
>> +			list_del(&page->lru);
>>
>> -		order = page_private(page);
>> -		nr_pages = 1 << order;
>> +			nr_pages = 1 << order;
>>
>> -		post_alloc_hook(page, order, __GFP_MOVABLE);
>> -		if (order)
>> -			split_page(page, order);
>> +			post_alloc_hook(page, order, __GFP_MOVABLE);
>> +			if (order)
>> +				split_page(page, order);
>>
>> -		for (i = 0; i < nr_pages; i++) {
>> -			list_add(&page->lru, &tmp_list);
>> -			page++;
>> +			for (i = 0; i < nr_pages; i++) {
>> +				list_add(&page->lru, &tmp_list);
>> +				page++;
>> +			}
>> +			freepages[order].nr_pages += nr_pages;
>>  		}
>> +		list_splice(&tmp_list, &freepages[order].pages);
>
> I think this should be list_splice_init() since you are reusing tmp_list in each
> iteration of the outer loop?

Right. Will fix it in the next version. Thanks.

--
Best Regards,
Yan, Zi

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]