diff mbox series

[02/15] mm/cma: move init_cma_reserved_pages() to cma.c and make it static

Message ID 20230319220008.2138576-3-rppt@kernel.org
State New
Headers
Series mm: move core MM initialization to mm/mm_init.c |

Commit Message

Mike Rapoport March 19, 2023, 9:59 p.m. UTC 
  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.

Signed-off-by: Mike Rapoport (IBM) <rppt@kernel.org>
---
 include/linux/gfp.h |  5 -----
 mm/cma.c            | 21 +++++++++++++++++++++
 mm/page_alloc.c     | 21 ---------------------
 3 files changed, 21 insertions(+), 26 deletions(-)

Comments

David Hildenbrand March 20, 2023, 10:30 a.m. UTC | #1 
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?
Mike Rapoport March 20, 2023, 11:11 a.m. UTC | #2 
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
>
David Hildenbrand March 20, 2023, 11:14 a.m. UTC | #3 
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 :)
Doug Berger March 20, 2023, 7:12 p.m. UTC | #4 
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 mbox series

Patch

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