diff mbox series

[v2] mm/page_alloc: simplify page_is_buddy() for better code readability

Message ID 1583807664-10343-1-git-send-email-qiwuchen55@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v2] mm/page_alloc: simplify page_is_buddy() for better code readability | expand

Commit Message

chenqiwu March 10, 2020, 2:34 a.m. UTC 
From: chenqiwu <chenqiwu@xiaomi.com>

Simplify page_is_buddy() to reduce the redundant code for better code
readability.

Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>
---
changes in v2:
 - split the conditional statement into four separate tests
   for better code readability.
---
 mm/page_alloc.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

Comments

Vlastimil Babka March 10, 2020, 8:28 a.m. UTC | #1 
On 3/10/20 3:34 AM, qiwuchen55@gmail.com wrote:
> From: chenqiwu <chenqiwu@xiaomi.com>
> 
> Simplify page_is_buddy() to reduce the redundant code for better code
> readability.
> 
> Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>

I would personally keep testing the page type first, and even switch it so that
PageBuddy is tested before page_is_guard (but it only matters on debugging kernels).

I can only speculate which will have better performance (if any), but it's more
logical to test the type first. Both orders should be safe though as we have the
zone lock.
chenqiwu March 10, 2020, 9:09 a.m. UTC | #2 
On Tue, Mar 10, 2020 at 09:28:46AM +0100, Vlastimil Babka wrote:
> On 3/10/20 3:34 AM, qiwuchen55@gmail.com wrote:
> > From: chenqiwu <chenqiwu@xiaomi.com>
> > 
> > Simplify page_is_buddy() to reduce the redundant code for better code
> > readability.
> > 
> > Signed-off-by: chenqiwu <chenqiwu@xiaomi.com>
> 
> I would personally keep testing the page type first, and even switch it so that
> PageBuddy is tested before page_is_guard (but it only matters on debugging kernels).
> 
> I can only speculate which will have better performance (if any), but it's more
> logical to test the type first. Both orders should be safe though as we have the
> zone lock.

Yes, I have ignored the iegality of the page must be checked first.
Thanks for your review!
diff mbox series

Patch

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3c4eb75..b221f6d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -794,29 +794,23 @@  static inline void set_page_order(struct page *page, unsigned int order)
 static inline int page_is_buddy(struct page *page, struct page *buddy,
 							unsigned int order)
 {
-	if (page_is_guard(buddy) && page_order(buddy) == order) {
-		if (page_zone_id(page) != page_zone_id(buddy))
-			return 0;
-
-		VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy);
+	if (page_order(buddy) != order)
+		return 0;
 
-		return 1;
-	}
+	if (!page_is_guard(buddy) && !PageBuddy(buddy))
+		return 0;
 
-	if (PageBuddy(buddy) && page_order(buddy) == order) {
-		/*
-		 * zone check is done late to avoid uselessly
-		 * calculating zone/node ids for pages that could
-		 * never merge.
-		 */
-		if (page_zone_id(page) != page_zone_id(buddy))
-			return 0;
+	/*
+	 * zone check is done late to avoid uselessly
+	 * calculating zone/node ids for pages that could
+	 * never merge.
+	 */
+	if (page_zone_id(page) != page_zone_id(buddy))
+		return 0;
 
-		VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy);
+	VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy);
 
-		return 1;
-	}
-	return 0;
+	return 1;
 }
 
 #ifdef CONFIG_COMPACTION