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

PATCH Applied [email protected] 5 participants 5 messages Mar 10, 2020

Subsystems (2)

Memory management Page allocator

lore

Share & tools

lore

[email protected]

author

2020-03-10 15:22 UTC #0 lore raw

PATCH v4 1/1 mm/page_alloc: simplify page_is_buddy() for better code readability

From: chenqiwu <[email protected]>

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

Signed-off-by chenqiwu <[email protected]>

---
changes in v4:
- change the return type of page_is_buddy() from int to bool.
- reflow the comment into fewer lines.

---
 mm/page_alloc.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

mm/page_alloc.c +13 ~~-20~~

index 3c4eb75..229fc8c 100644
@@ -791,32 +791,25 @@ static inline void set_page_order(struct page *page, unsigned int order)
  *
  * For recording page's order, we use page_private(page).
  */
-static inline int page_is_buddy(struct page *page, struct page *buddy,
+static inline bool 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_is_guard(buddy) && !PageBuddy(buddy))
+		return false;
 
-		return 1;
-	}
+	if (page_order(buddy) != order)
+		return false;
 
-	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 false;
 
-		VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy);
+	VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy);
 
-		return 1;
-	}
-	return 0;
+	return true;
 }
 
 #ifdef CONFIG_COMPACTION

Signature · 1 line

1.9.1

Alexander Duyck

Reviewed-by on v4 1/1

2020-03-10 15:56 UTC #1 lore raw

Quoted 6 lines · On Tue, Mar 10, 2020 at 8:22 AM <[email protected]> wrote:

From: chenqiwu <[email protected]>
Simplify page_is_buddy() to reduce the redundant code for better code readability.

Quoted 6 lines

Signed-off-by chenqiwu <[email protected]>
---
changes in v4:
- change the return type of page_is_buddy() from int to bool.
- reflow the comment into fewer lines.
---

Looks good to me.

Reviewed-by Alexander Duyck <[email protected]>

Quoted 56 lines

 mm/page_alloc.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

mm/page_alloc.c +13 ~~-21~~

index 3c4eb75..229fc8c 100644
@@ -791,32 +791,25 @@ static inline void set_page_order(struct page *page, unsigned int order)
  *
  * For recording page's order, we use page_private(page).
  */
-static inline int page_is_buddy(struct page *page, struct page *buddy,
+static inline bool 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_is_guard(buddy) && !PageBuddy(buddy))
+               return false;
 
-               return 1;
-       }
+       if (page_order(buddy) != order)
+               return false;
 
-       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 false;
 
-               VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy);
+       VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy);
 
-               return 1;
-       }
-       return 0;
+       return true;
 }
 
 #ifdef CONFIG_COMPACTION
--

1.9.1

Matthew Wilcox

Reviewed-by on v4 1/1

2020-03-10 18:09 UTC #2 lore raw

In reply to [email protected]

Quoted 6 lines · On Tue, Mar 10, 2020 at 11:22:31PM +0800, [email protected] wrote:

From: chenqiwu <[email protected]>
Simplify page_is_buddy() to reduce the redundant code for better code readability.
Signed-off-by chenqiwu <[email protected]>

Reviewed-by Matthew Wilcox (Oracle) <[email protected]>

Vlastimil Babka

Reviewed-by on v4 1/1

2020-03-10 18:23 UTC #3 lore raw

In reply to [email protected]

Quoted 6 lines · On 3/10/20 4:22 PM, [email protected] wrote:

From: chenqiwu <[email protected]>
Simplify page_is_buddy() to reduce the redundant code for better code readability.
Signed-off-by chenqiwu <[email protected]>

Reviewed-by Vlastimil Babka <[email protected]>

Thanks.

Pankaj Gupta

Acked-by on v4 1/1

2020-03-10 19:09 UTC #4 lore raw

In reply to [email protected]

Quoted 62 lines

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

Signed-off-by chenqiwu <[email protected]>

---
changes in v4:
- change the return type of page_is_buddy() from int to bool.
- reflow the comment into fewer lines.

---
 mm/page_alloc.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

mm/page_alloc.c +13 ~~-20~~

index 3c4eb75..229fc8c 100644
@@ -791,32 +791,25 @@ static inline void set_page_order(struct page *page,
 unsigned int order)
  *
  * For recording page's order, we use page_private(page).
  */
-static inline int page_is_buddy(struct page *page, struct page *buddy,
+static inline bool 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_is_guard(buddy) && !PageBuddy(buddy))
+               return false;
 
-               return 1;
-       }
+       if (page_order(buddy) != order)
+               return false;
 
-       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 false;
 
-               VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy);
+       VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy);
 
-               return 1;
-       }
-       return 0;
+       return true;
 }

Quoted 3 lines

#ifdef CONFIG_COMPACTION
--

Acked-by Pankaj Gupta <[email protected]>

Quoted 3 lines

1.9.1