Re: [PATCH 4/8] mm: Rename accept_page() to accept_page_memory()

[David Hildenbrand <david@xxxxxxxxxx>]

On 05.08.24 16:59, Kirill A. Shutemov wrote:
> Rename the helper. The accept_page() name is going to be used for
> different function.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> ---
>   mm/page_alloc.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a35efb114496..34718852d576 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -286,7 +286,7 @@ EXPORT_SYMBOL(nr_online_nodes);
>   #endif
>   
>   static bool page_contains_unaccepted(struct page *page, unsigned int order);
> -static void accept_page(struct page *page, unsigned int order);
> +static void accept_page_memory(struct page *page, unsigned int order);
>   static bool cond_accept_memory(struct zone *zone, unsigned int order);
>   static inline bool has_unaccepted_memory(void);
>   static bool __free_unaccepted(struct page *page);
> @@ -1263,7 +1263,7 @@ void __meminit __free_pages_core(struct page *page, unsigned int order,
>   		if (order == MAX_PAGE_ORDER && __free_unaccepted(page))
>   			return;
>   
> -		accept_page(page, order);
> +		accept_page_memory(page, order);

I wonder if we can do better at naming.

Naming them accept_page1 and accept_page2 might be just as confusing as what you use here.

And I think we better just don't have this "page" wrapper here at all and just move to
memory ranges.

After all, the page contains no information we need here besides the PFN.


What about:

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 7a8bdfa742e1..5dc4066f35b3 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -286,7 +286,6 @@ EXPORT_SYMBOL(nr_online_nodes);
 #endif
 
 static bool page_contains_unaccepted(struct page *page, unsigned int order);
-static void accept_page(struct page *page, unsigned int order);
 static inline bool has_unaccepted_memory(void);
 static bool __free_unaccepted(struct page *page);
 
@@ -1262,7 +1261,7 @@ void __meminit __free_pages_core(struct page *page, unsigned int order,
                if (order == MAX_PAGE_ORDER && __free_unaccepted(page))
                        return;
 
-               accept_page(page, order);
+               __accept_memory(page_to_phys(page), PAGE_SIZE << order);
        }
 
        /*
@@ -6975,11 +6974,9 @@ static bool page_contains_unaccepted(struct page *page, unsigned int order)
        return range_contains_unaccepted_memory(start, end);
 }
 
-static void accept_page(struct page *page, unsigned int order)
+static void __accept_memory(phys_addr_t start, phys_addr_t size)
 {
-       phys_addr_t start = page_to_phys(page);
-
-       accept_memory(start, start + (PAGE_SIZE << order));
+       accept_memory(start, start + size);
 }
 
 static bool try_to_accept_memory_one(struct zone *zone)
@@ -7006,7 +7003,7 @@ static bool try_to_accept_memory_one(struct zone *zone)
        __mod_zone_page_state(zone, NR_UNACCEPTED, -MAX_ORDER_NR_PAGES);
        spin_unlock_irqrestore(&zone->lock, flags);
 
-       accept_page(page, MAX_PAGE_ORDER);
+       __accept_memory(page_to_phys(page), PAGE_SIZE << MAX_PAGE_ORDER);
 
        __free_pages_ok(page, MAX_PAGE_ORDER, FPI_TO_TAIL);
 
@@ -7071,7 +7068,7 @@ static bool page_contains_unaccepted(struct page *page, unsigned int order)
        return false;
 }
 
-static void accept_page(struct page *page, unsigned int order)
+static void __accept_memory(phys_addr_t start, phys_addr_t size)
 {
 }
 

It would be even easier if accept_memory() would just accept start+size.

--
Cheers,

David / dhildenb