| Message ID | 20220420095906.27349-2-mgorman@techsingularity.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Drain remote per-cpu directly | expand |
On Wed, Apr 20, 2022 at 10:59:01AM +0100, Mel Gorman wrote: > The page allocator uses page->lru for storing pages on either buddy or > PCP lists. Create page->buddy_list and page->pcp_list as a union with > page->lru. This is simply to clarify what type of list a page is on > in the page allocator. Hi Mel, No objection to this change, and I certainly don't want to hold up fixing this (or any other) problem in the page allocator. I would like to talk about splitting out free page management from struct page. Maybe you'd like to discuss that in person at LSFMM, but a quick sketch of a data structure might look like ... struct free_mem { unsigned long __page_flags; union { struct list_head buddy_list; struct list_head pcp_list; }; unsigned long __rsvd4; unsigned long pcp_migratetype_and_order; unsigned long buddy_order; unsigned int __page_type; atomic_t _refcount; }; Am I missing anything there? (Would you like to use separate types for pcp and buddy? That might be overkill, or it might help keep the different stages of "free" memory separate from each other)
On Wed, Apr 20, 2022 at 09:43:05PM +0100, Matthew Wilcox wrote: > On Wed, Apr 20, 2022 at 10:59:01AM +0100, Mel Gorman wrote: > > The page allocator uses page->lru for storing pages on either buddy or > > PCP lists. Create page->buddy_list and page->pcp_list as a union with > > page->lru. This is simply to clarify what type of list a page is on > > in the page allocator. > > Hi Mel, > > No objection to this change, and I certainly don't want to hold up > fixing this (or any other) problem in the page allocator. Minimally, I think the patch makes it easier to implement your suggestion and it can happen before or after the rest of the series. > I would > like to talk about splitting out free page management from struct page. > Maybe you'd like to discuss that in person at LSFMM, but a quick > sketch of a data structure might look like ... > Unfortunately, I am unable to attend LSF/MM (or any other conference) physically but I have no objection to splitting this out as a separate structure. I assume the basis for the change would be for type checking. > struct free_mem { > unsigned long __page_flags; page->flags, ok > union { > struct list_head buddy_list; > struct list_head pcp_list; > }; page->lru, ok > unsigned long __rsvd4; page->mapping, we need that > unsigned long pcp_migratetype_and_order; page->index, ok but more on this later > unsigned long buddy_order; page->private, ok. > unsigned int __page_type; page->_mapcount, we need that too. > atomic_t _refcount; page->_refcount, ok. > }; > > Am I missing anything there? > s/__page_flags/flags/ The allocator checks and modifies these bits so it has awareness of page->flags s/__rsvd4/mapping/ The mapping is checked for debugging and the allocator is responsible for clearing page->mapping s/pcp_migratetype_and_order/pcp_migratetype/ Commit 8b10b465d0e1 ("mm/page_alloc: free pages in a single pass during bulk free") removed the migratetype and order stuffing in page->index. The order is inferred from the array index via order_to_pindex and pindex_to_order but migratetype is still stored in page->index by set_pcppage_migratetype s/__page_type/_mapcount/ because _mapcount if checked for debugging memcg_data needs to be accessible for debugging checks _last_cpupid needs to be accessible as it's reset during prepare via page_cpupid_reset_last. Rather than putting in a dummy field for virtual, maybe virtual can move. > (Would you like to use separate types for pcp and buddy? That might be > overkill, or it might help keep the different stages of "free" memory > separate from each other) I think it's overkill because there is too much overlap between a PCP page and buddy page due to page checks and preparation. The distinguishing factor between a free pcp page and free buddy page is the PageBuddy bit.
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 8834e38c06a4..a2782e8af307 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -87,6 +87,7 @@ struct page { */ union { struct list_head lru; + /* Or, for the Unevictable "LRU list" slot */ struct { /* Always even, to negate PageTail */ @@ -94,6 +95,10 @@ struct page { /* Count page's or folio's mlocks */ unsigned int mlock_count; }; + + /* Or, free page */ + struct list_head buddy_list; + struct list_head pcp_list; }; /* See page-flags.h for PAGE_MAPPING_FLAGS */ struct address_space *mapping; diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 2db95780e003..63976ad4b7f1 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -780,7 +780,7 @@ static inline bool set_page_guard(struct zone *zone, struct page *page, return false; __SetPageGuard(page); - INIT_LIST_HEAD(&page->lru); + INIT_LIST_HEAD(&page->buddy_list); set_page_private(page, order); /* Guard pages are not available for any usage */ __mod_zone_freepage_state(zone, -(1 << order), migratetype); @@ -957,7 +957,7 @@ static inline void add_to_free_list(struct page *page, struct zone *zone, { struct free_area *area = &zone->free_area[order]; - list_add(&page->lru, &area->free_list[migratetype]); + list_add(&page->buddy_list, &area->free_list[migratetype]); area->nr_free++; } @@ -967,7 +967,7 @@ static inline void add_to_free_list_tail(struct page *page, struct zone *zone, { struct free_area *area = &zone->free_area[order]; - list_add_tail(&page->lru, &area->free_list[migratetype]); + list_add_tail(&page->buddy_list, &area->free_list[migratetype]); area->nr_free++; } @@ -981,7 +981,7 @@ static inline void move_to_free_list(struct page *page, struct zone *zone, { struct free_area *area = &zone->free_area[order]; - list_move_tail(&page->lru, &area->free_list[migratetype]); + list_move_tail(&page->buddy_list, &area->free_list[migratetype]); } static inline void del_page_from_free_list(struct page *page, struct zone *zone, @@ -991,7 +991,7 @@ static inline void del_page_from_free_list(struct page *page, struct zone *zone, if (page_reported(page)) __ClearPageReported(page); - list_del(&page->lru); + list_del(&page->buddy_list); __ClearPageBuddy(page); set_page_private(page, 0); zone->free_area[order].nr_free--; @@ -1493,7 +1493,7 @@ static void free_pcppages_bulk(struct zone *zone, int count, mt = get_pcppage_migratetype(page); /* must delete to avoid corrupting pcp list */ - list_del(&page->lru); + list_del(&page->pcp_list); count -= nr_pages; pcp->count -= nr_pages; @@ -3053,7 +3053,7 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order, * for IO devices that can merge IO requests if the physical * pages are ordered properly. */ - list_add_tail(&page->lru, list); + list_add_tail(&page->pcp_list, list); allocated++; if (is_migrate_cma(get_pcppage_migratetype(page))) __mod_zone_page_state(zone, NR_FREE_CMA_PAGES, @@ -3392,7 +3392,7 @@ static void free_unref_page_commit(struct page *page, int migratetype, __count_vm_event(PGFREE); pcp = this_cpu_ptr(zone->per_cpu_pageset); pindex = order_to_pindex(migratetype, order); - list_add(&page->lru, &pcp->lists[pindex]); + list_add(&page->pcp_list, &pcp->lists[pindex]); pcp->count += 1 << order; /* @@ -3656,7 +3656,7 @@ struct page *__rmqueue_pcplist(struct zone *zone, unsigned int order, } page = list_first_entry(list, struct page, lru); - list_del(&page->lru); + list_del(&page->pcp_list); pcp->count -= 1 << order; } while (check_new_pcp(page, order));
The page allocator uses page->lru for storing pages on either buddy or PCP lists. Create page->buddy_list and page->pcp_list as a union with page->lru. This is simply to clarify what type of list a page is on in the page allocator. No functional change intended. Signed-off-by: Mel Gorman <mgorman@techsingularity.net> --- include/linux/mm_types.h | 5 +++++ mm/page_alloc.c | 18 +++++++++--------- 2 files changed, 14 insertions(+), 9 deletions(-)