diff mbox series

[v4] mm/page_alloc: Consolidate unlikely handling in page_expected_state

Message ID 20250328014757.1212737-1-ye.liu@linux.dev (mailing list archive)
State New
Headers show
Series [v4] mm/page_alloc: Consolidate unlikely handling in page_expected_state | expand

Commit Message

Ye Liu March 28, 2025, 1:47 a.m. UTC 
From: Ye Liu <liuye@kylinos.cn>

Consolidate the handling of unlikely conditions in the 
page_expected_state() function to reduce code duplication and improve 
readability.

Move the logic for handling __PG_HWPOISON flags from the 
check_new_page_bad() function to the page_expected_state() function, 
and remove check_new_page_bad().

Call bad_page() directly from the check_new_page() function if the page has
unexpected flags.

Simplify the code by reducing the number of functions and centralizing the
handling of unlikely conditions.

Signed-off-by: Ye Liu <liuye@kylinos.cn>
Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

---
V4: Use "imperative mood" to modify the commit and append parentheses 
to all function names.
V3: Delete 'This patch'.
V2: return true instead of false in the PageHWPoison branch.
---
 mm/page_alloc.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

Comments

Markus Elfring March 28, 2025, 9:44 a.m. UTC | #1 
…
> ---
> V4: … and append parentheses to all function names.

Collateral evolution for summary phrases?


> V3: Delete 'This patch'.
> V2: return true instead of false in the PageHWPoison branch.
> ---
>  mm/page_alloc.c | 24 ++++++++----------------
…

How do you think about to replace repeated marker lines by line breaks?

Regards,
Markus
Matthew Wilcox March 28, 2025, 2:29 p.m. UTC | #2 
On Fri, Mar 28, 2025 at 09:47:57AM +0800, Ye Liu wrote:
> Consolidate the handling of unlikely conditions in the 
> page_expected_state() function to reduce code duplication and improve 
> readability.

I don't think this is an equivalent transformation.

Please, stop with these tweaky patches to incredibly sensitive core code.
Fix a problem, or leave it alone.  We are primarily short of reviewer
bandwidth.  You could help with that by reviewing other people's patches.
Sending patches of your own just adds to other people's workload.
diff mbox series

Patch

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2842da893eea..e8b95c6a96c2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -903,6 +903,12 @@  static inline bool page_expected_state(struct page *page,
 			(page->flags & check_flags)))
 		return false;
 
+	if (unlikely(PageHWPoison(page))) {
+		/* Don't complain about hwpoisoned pages */
+		if (PageBuddy(page))
+			__ClearPageBuddy(page);
+	}
+
 	return true;
 }
 
@@ -1586,29 +1592,15 @@  static __always_inline void page_del_and_expand(struct zone *zone,
 	account_freepages(zone, -nr_pages, migratetype);
 }
 
-static void check_new_page_bad(struct page *page)
-{
-	if (unlikely(PageHWPoison(page))) {
-		/* Don't complain about hwpoisoned pages */
-		if (PageBuddy(page))
-			__ClearPageBuddy(page);
-		return;
-	}
-
-	bad_page(page,
-		 page_bad_reason(page, PAGE_FLAGS_CHECK_AT_PREP));
-}
-
 /*
  * This page is about to be returned from the page allocator
  */
 static bool check_new_page(struct page *page)
 {
-	if (likely(page_expected_state(page,
-				PAGE_FLAGS_CHECK_AT_PREP|__PG_HWPOISON)))
+	if (likely(page_expected_state(page, PAGE_FLAGS_CHECK_AT_PREP)))
 		return false;
 
-	check_new_page_bad(page);
+	bad_page(page, page_bad_reason(page, PAGE_FLAGS_CHECK_AT_PREP));
 	return true;
 }