| Message ID | 20230319220008.2138576-3-rppt@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: move core MM initialization to mm/mm_init.c | expand |
On 19.03.23 22:59, Mike Rapoport wrote: > From: "Mike Rapoport (IBM)" <rppt@kernel.org> > > init_cma_reserved_pages() only used in cma.c, no point of having it in > page_alloc.c. > > Move init_cma_reserved_pages() to cma.c and make it static. I guess the motivation is to avoid letting too many subsystems mess with pageblock migratetypes, managed pages, PG_reserved ... So it kind of makes sense to have these low-level details out of common CMA code, no?
On Mon, Mar 20, 2023 at 11:30:20AM +0100, David Hildenbrand wrote: > On 19.03.23 22:59, Mike Rapoport wrote: > > From: "Mike Rapoport (IBM)" <rppt@kernel.org> > > > > init_cma_reserved_pages() only used in cma.c, no point of having it in > > page_alloc.c. > > > > Move init_cma_reserved_pages() to cma.c and make it static. > > I guess the motivation is to avoid letting too many subsystems mess with > pageblock migratetypes, managed pages, PG_reserved ... Judging by the git log it just ended up in page_alloc.c because set_pageblock_migratetype() was static back then ;) > So it kind of makes sense to have these low-level details out of common CMA > code, no? I don't mind keeping it out of cma and folding this into "grand move" patch. > -- > Thanks, > > David / dhildenb >
On 20.03.23 12:11, Mike Rapoport wrote: > On Mon, Mar 20, 2023 at 11:30:20AM +0100, David Hildenbrand wrote: >> On 19.03.23 22:59, Mike Rapoport wrote: >>> From: "Mike Rapoport (IBM)" <rppt@kernel.org> >>> >>> init_cma_reserved_pages() only used in cma.c, no point of having it in >>> page_alloc.c. >>> >>> Move init_cma_reserved_pages() to cma.c and make it static. >> >> I guess the motivation is to avoid letting too many subsystems mess with >> pageblock migratetypes, managed pages, PG_reserved ... > > Judging by the git log it just ended up in page_alloc.c because > set_pageblock_migratetype() was static back then ;) Yeah, it leaked into page_isolation, which heavily relies on migratetype handling :)
On 3/20/2023 4:11 AM, Mike Rapoport wrote: > On Mon, Mar 20, 2023 at 11:30:20AM +0100, David Hildenbrand wrote: >> On 19.03.23 22:59, Mike Rapoport wrote: >>> From: "Mike Rapoport (IBM)" <rppt@kernel.org> >>> >>> init_cma_reserved_pages() only used in cma.c, no point of having it in >>> page_alloc.c. >>> >>> Move init_cma_reserved_pages() to cma.c and make it static. >> >> I guess the motivation is to avoid letting too many subsystems mess with >> pageblock migratetypes, managed pages, PG_reserved ... > > Judging by the git log it just ended up in page_alloc.c because > set_pageblock_migratetype() was static back then ;) > >> So it kind of makes sense to have these low-level details out of common CMA >> code, no? > > I don't mind keeping it out of cma and folding this into "grand move" > patch. Just an FYI, this conflicts with my patch: https://lore.kernel.org/lkml/20230311003855.645684-6-opendmb@gmail.com/ So it would work better for me if it was folded into your "grand move" (assuming that refers to your patch 4) and init_cma_reserved_pageblock() could be retained as a wrapper there in my patch if we want to still keep set_pageblock_migratetype() out of the common CMA code. -- Thanks, Doug
diff --git a/include/linux/gfp.h b/include/linux/gfp.h index 65a78773dcca..7c554e4bd49f 100644 --- a/include/linux/gfp.h +++ b/include/linux/gfp.h @@ -361,9 +361,4 @@ extern struct page *alloc_contig_pages(unsigned long nr_pages, gfp_t gfp_mask, #endif void free_contig_range(unsigned long pfn, unsigned long nr_pages); -#ifdef CONFIG_CMA -/* CMA stuff */ -extern void init_cma_reserved_pageblock(struct page *page); -#endif - #endif /* __LINUX_GFP_H */ diff --git a/mm/cma.c b/mm/cma.c index a7263aa02c92..ce08fb9825b4 100644 --- a/mm/cma.c +++ b/mm/cma.c @@ -31,8 +31,10 @@ #include <linux/highmem.h> #include <linux/io.h> #include <linux/kmemleak.h> +#include <linux/page-isolation.h> #include <trace/events/cma.h> +#include "internal.h" #include "cma.h" struct cma cma_areas[MAX_CMA_AREAS]; @@ -93,6 +95,25 @@ static void cma_clear_bitmap(struct cma *cma, unsigned long pfn, spin_unlock_irqrestore(&cma->lock, flags); } +/* Free whole pageblock and set its migration type to MIGRATE_CMA. */ +static void init_cma_reserved_pageblock(struct page *page) +{ + unsigned i = pageblock_nr_pages; + struct page *p = page; + + do { + __ClearPageReserved(p); + set_page_count(p, 0); + } while (++p, --i); + + set_pageblock_migratetype(page, MIGRATE_CMA); + set_page_refcounted(page); + __free_pages(page, pageblock_order); + + adjust_managed_page_count(page, pageblock_nr_pages); + page_zone(page)->cma_pages += pageblock_nr_pages; +} + static void __init cma_activate_area(struct cma *cma) { unsigned long base_pfn = cma->base_pfn, pfn; diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 87d760236dba..22e3da842e3f 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -2280,27 +2280,6 @@ void __init page_alloc_init_late(void) set_zone_contiguous(zone); } -#ifdef CONFIG_CMA -/* Free whole pageblock and set its migration type to MIGRATE_CMA. */ -void __init init_cma_reserved_pageblock(struct page *page) -{ - unsigned i = pageblock_nr_pages; - struct page *p = page; - - do { - __ClearPageReserved(p); - set_page_count(p, 0); - } while (++p, --i); - - set_pageblock_migratetype(page, MIGRATE_CMA); - set_page_refcounted(page); - __free_pages(page, pageblock_order); - - adjust_managed_page_count(page, pageblock_nr_pages); - page_zone(page)->cma_pages += pageblock_nr_pages; -} -#endif - /* * The order of subdivision here is critical for the IO subsystem. * Please do not alter this order without good reasons and regression