linux-kernel - Re: [PATCH 4/8] mm: Rename accept_page() to accept_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4220779f-1037-444b-b835-1732d497b9a2@redhat.com>
Date: Tue, 6 Aug 2024 14:18:24 +0200
From: David Hildenbrand <david@...hat.com>
To: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
 Andrew Morton <akpm@...ux-foundation.org>,
 "Borislav Petkov (AMD)" <bp@...en8.de>, Mel Gorman <mgorman@...e.de>,
 Vlastimil Babka <vbabka@...e.cz>
Cc: Tom Lendacky <thomas.lendacky@....com>, Mike Rapoport <rppt@...nel.org>,
 "Matthew Wilcox (Oracle)" <willy@...radead.org>,
 Johannes Weiner <hannes@...xchg.org>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/8] mm: Rename accept_page() to accept_page_memory()

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@...ux.intel.com>
> ---
>   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