On 9 Feb 2024, at 14:36, Zi Yan wrote: > On 9 Feb 2024, at 11:37, Vlastimil Babka wrote: > >> On 2/2/24 17:15, Zi Yan wrote: >>> From: Zi Yan <ziy@xxxxxxxxxx> >>> >>> Before last commit, memory compaction only migrates order-0 folios and >>> skips >0 order folios. Last commit splits all >0 order folios during >>> compaction. This commit migrates >0 order folios during 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 NR_PAGE_ORDERS 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 using newly added >>> free_pages_prepare_fpi_none(). >>> >>> Step 3 is done for a latter optimization that splitting and/or merging free >>> pages during compaction becomes easier. >>> >>> Note: without splitting free pages, compaction can end prematurely due to >>> migration will return -ENOMEM even if there is free pages. This happens >>> when no order-0 free page exist and compaction_alloc() return NULL. >>> >>> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx> >> >> ... >> >>> /* >>> @@ -1835,9 +1857,17 @@ static struct folio *compaction_alloc(struct folio *src, unsigned long data) >>> static void compaction_free(struct folio *dst, unsigned long data) >>> { >>> struct compact_control *cc = (struct compact_control *)data; >>> + int order = folio_order(dst); >>> + struct page *page = &dst->page; >>> + >>> + folio_set_count(dst, 0); >> >> We can't change refcount to 0 like this, after it was already set to 1 and >> somebody else might have done get_page_unless_zero(). You need to either >> put_page_testzero() and if it's false, consider the page lost, or leave it >> refcounted and adjust the code to handle both refcounted and non-refcounted >> pages on the lists (the first option is simpler and shouldn't be too bad). > Got it. Will fix it with the first option. Thanks. Do you think we should have a WARN or WARN_ONCE if we lose a page here? -- Best Regards, Yan, Zi
Attachment:
signature.asc
Description: OpenPGP digital signature