linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [Patch v3 0/5] mm/page_alloc.c: cleanup on check page
@ 2020-04-11 22:03 Wei Yang
  2020-04-11 22:03 ` [Patch v3 1/5] mm/page_alloc.c: bad_[reason|flags] is not necessary when PageHWPoison Wei Yang
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Wei Yang @ 2020-04-11 22:03 UTC (permalink / raw)
  To: akpm; +Cc: linux-mm, linux-kernel, rientjes, anshuman.khandual, david, Wei Yang

The patch set does some cleanup related to check page.

1. Remove unnecessary bad_reason assignment
2. Remove bad_flags to bad_page()
3. Rename function for naming convention
4. Extract common part to check page

Thanks suggestions from David Rientjes and Anshuman Khandual.

Wei Yang (5):
  mm/page_alloc.c: bad_[reason|flags] is not necessary when PageHWPoison
  mm/page_alloc.c: bad_flags is not necessary for bad_page()
  mm/page_alloc.c: rename free_pages_check_bad() to
    check_free_page_bad()
  mm/page_alloc.c: rename free_pages_check() to check_free_page()
  mm/page_alloc.c: extract check_[new|free]_page_bad() common part to
    page_bad_reason()

 mm/page_alloc.c | 74 +++++++++++++++++++------------------------------
 1 file changed, 28 insertions(+), 46 deletions(-)

-- 
2.23.0



^ permalink raw reply	[flat|nested] 7+ messages in thread
* [Patch v3 1/5] mm/page_alloc.c: bad_[reason|flags] is not necessary when PageHWPoison
  2020-04-11 22:03 [Patch v3 0/5] mm/page_alloc.c: cleanup on check page Wei Yang
@ 2020-04-11 22:03 ` Wei Yang
  2020-04-11 22:03 ` [Patch v3 2/5] mm/page_alloc.c: bad_flags is not necessary for bad_page() Wei Yang
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Wei Yang @ 2020-04-11 22:03 UTC (permalink / raw)
  To: akpm; +Cc: linux-mm, linux-kernel, rientjes, anshuman.khandual, david, Wei Yang

Since function returns directly, bad_[reason|flags] is not used any
where. And move this to the first.

This is a following cleanup for commit e570f56cccd21 ("mm:
check_new_page_bad() directly returns in __PG_HWPOISON case")

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Reviewed-by: David Hildenbrand <david@redhat.com>

---
v3:
  * move this handling to the first case
---
 mm/page_alloc.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f49f28f1220b..7327dc039069 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2037,19 +2037,17 @@ static void check_new_page_bad(struct page *page)
 	const char *bad_reason = NULL;
 	unsigned long bad_flags = 0;
 
+	if (unlikely(page->flags & __PG_HWPOISON)) {
+		/* Don't complain about hwpoisoned pages */
+		page_mapcount_reset(page); /* remove PageBuddy */
+		return;
+	}
 	if (unlikely(atomic_read(&page->_mapcount) != -1))
 		bad_reason = "nonzero mapcount";
 	if (unlikely(page->mapping != NULL))
 		bad_reason = "non-NULL mapping";
 	if (unlikely(page_ref_count(page) != 0))
 		bad_reason = "nonzero _refcount";
-	if (unlikely(page->flags & __PG_HWPOISON)) {
-		bad_reason = "HWPoisoned (hardware-corrupted)";
-		bad_flags = __PG_HWPOISON;
-		/* Don't complain about hwpoisoned pages */
-		page_mapcount_reset(page); /* remove PageBuddy */
-		return;
-	}
 	if (unlikely(page->flags & PAGE_FLAGS_CHECK_AT_PREP)) {
 		bad_reason = "PAGE_FLAGS_CHECK_AT_PREP flag set";
 		bad_flags = PAGE_FLAGS_CHECK_AT_PREP;
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [Patch v3 2/5] mm/page_alloc.c: bad_flags is not necessary for bad_page()
  2020-04-11 22:03 [Patch v3 0/5] mm/page_alloc.c: cleanup on check page Wei Yang
  2020-04-11 22:03 ` [Patch v3 1/5] mm/page_alloc.c: bad_[reason|flags] is not necessary when PageHWPoison Wei Yang
@ 2020-04-11 22:03 ` Wei Yang
  2020-04-11 22:03 ` [Patch v3 3/5] mm/page_alloc.c: rename free_pages_check_bad() to check_free_page_bad() Wei Yang
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Wei Yang @ 2020-04-11 22:03 UTC (permalink / raw)
  To: akpm; +Cc: linux-mm, linux-kernel, rientjes, anshuman.khandual, david, Wei Yang

After commit 5b57b8f22709 ("mm/debug.c: always print flags in
dump_page()"), page->flags is always printed for a bad page. It is not
necessary to have bad_flags any more.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Suggested-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 mm/page_alloc.c | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 7327dc039069..0648127af872 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -602,8 +602,7 @@ static inline int __maybe_unused bad_range(struct zone *zone, struct page *page)
 }
 #endif
 
-static void bad_page(struct page *page, const char *reason,
-		unsigned long bad_flags)
+static void bad_page(struct page *page, const char *reason)
 {
 	static unsigned long resume;
 	static unsigned long nr_shown;
@@ -632,10 +631,6 @@ static void bad_page(struct page *page, const char *reason,
 	pr_alert("BUG: Bad page state in process %s  pfn:%05lx\n",
 		current->comm, page_to_pfn(page));
 	__dump_page(page, reason);
-	bad_flags &= page->flags;
-	if (bad_flags)
-		pr_alert("bad because of flags: %#lx(%pGp)\n",
-						bad_flags, &bad_flags);
 	dump_page_owner(page);
 
 	print_modules();
@@ -1020,11 +1015,7 @@ static inline bool page_expected_state(struct page *page,
 
 static void free_pages_check_bad(struct page *page)
 {
-	const char *bad_reason;
-	unsigned long bad_flags;
-
-	bad_reason = NULL;
-	bad_flags = 0;
+	const char *bad_reason = NULL;
 
 	if (unlikely(atomic_read(&page->_mapcount) != -1))
 		bad_reason = "nonzero mapcount";
@@ -1032,15 +1023,13 @@ static void free_pages_check_bad(struct page *page)
 		bad_reason = "non-NULL mapping";
 	if (unlikely(page_ref_count(page) != 0))
 		bad_reason = "nonzero _refcount";
-	if (unlikely(page->flags & PAGE_FLAGS_CHECK_AT_FREE)) {
+	if (unlikely(page->flags & PAGE_FLAGS_CHECK_AT_FREE))
 		bad_reason = "PAGE_FLAGS_CHECK_AT_FREE flag(s) set";
-		bad_flags = PAGE_FLAGS_CHECK_AT_FREE;
-	}
 #ifdef CONFIG_MEMCG
 	if (unlikely(page->mem_cgroup))
 		bad_reason = "page still charged to cgroup";
 #endif
-	bad_page(page, bad_reason, bad_flags);
+	bad_page(page, bad_reason);
 }
 
 static inline int free_pages_check(struct page *page)
@@ -1071,7 +1060,7 @@ static int free_tail_pages_check(struct page *head_page, struct page *page)
 	case 1:
 		/* the first tail page: ->mapping may be compound_mapcount() */
 		if (unlikely(compound_mapcount(page))) {
-			bad_page(page, "nonzero compound_mapcount", 0);
+			bad_page(page, "nonzero compound_mapcount");
 			goto out;
 		}
 		break;
@@ -1083,17 +1072,17 @@ static int free_tail_pages_check(struct page *head_page, struct page *page)
 		break;
 	default:
 		if (page->mapping != TAIL_MAPPING) {
-			bad_page(page, "corrupted mapping in tail page", 0);
+			bad_page(page, "corrupted mapping in tail page");
 			goto out;
 		}
 		break;
 	}
 	if (unlikely(!PageTail(page))) {
-		bad_page(page, "PageTail not set", 0);
+		bad_page(page, "PageTail not set");
 		goto out;
 	}
 	if (unlikely(compound_head(page) != head_page)) {
-		bad_page(page, "compound_head not consistent", 0);
+		bad_page(page, "compound_head not consistent");
 		goto out;
 	}
 	ret = 0;
@@ -2035,7 +2024,6 @@ static inline void expand(struct zone *zone, struct page *page,
 static void check_new_page_bad(struct page *page)
 {
 	const char *bad_reason = NULL;
-	unsigned long bad_flags = 0;
 
 	if (unlikely(page->flags & __PG_HWPOISON)) {
 		/* Don't complain about hwpoisoned pages */
@@ -2048,15 +2036,13 @@ static void check_new_page_bad(struct page *page)
 		bad_reason = "non-NULL mapping";
 	if (unlikely(page_ref_count(page) != 0))
 		bad_reason = "nonzero _refcount";
-	if (unlikely(page->flags & PAGE_FLAGS_CHECK_AT_PREP)) {
+	if (unlikely(page->flags & PAGE_FLAGS_CHECK_AT_PREP))
 		bad_reason = "PAGE_FLAGS_CHECK_AT_PREP flag set";
-		bad_flags = PAGE_FLAGS_CHECK_AT_PREP;
-	}
 #ifdef CONFIG_MEMCG
 	if (unlikely(page->mem_cgroup))
 		bad_reason = "page still charged to cgroup";
 #endif
-	bad_page(page, bad_reason, bad_flags);
+	bad_page(page, bad_reason);
 }
 
 /*
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [Patch v3 3/5] mm/page_alloc.c: rename free_pages_check_bad() to check_free_page_bad()
  2020-04-11 22:03 [Patch v3 0/5] mm/page_alloc.c: cleanup on check page Wei Yang
  2020-04-11 22:03 ` [Patch v3 1/5] mm/page_alloc.c: bad_[reason|flags] is not necessary when PageHWPoison Wei Yang
  2020-04-11 22:03 ` [Patch v3 2/5] mm/page_alloc.c: bad_flags is not necessary for bad_page() Wei Yang
@ 2020-04-11 22:03 ` Wei Yang
  2020-04-11 22:03 ` [Patch v3 4/5] mm/page_alloc.c: rename free_pages_check() to check_free_page() Wei Yang
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Wei Yang @ 2020-04-11 22:03 UTC (permalink / raw)
  To: akpm; +Cc: linux-mm, linux-kernel, rientjes, anshuman.khandual, david, Wei Yang

Function free_pages_check_bad() is the counterpart of
check_new_page_bad(). Rename it to use the same name convention.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/page_alloc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0648127af872..85d7aec5fb45 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1013,7 +1013,7 @@ static inline bool page_expected_state(struct page *page,
 	return true;
 }
 
-static void free_pages_check_bad(struct page *page)
+static void check_free_page_bad(struct page *page)
 {
 	const char *bad_reason = NULL;
 
@@ -1038,7 +1038,7 @@ static inline int free_pages_check(struct page *page)
 		return 0;
 
 	/* Something has gone sideways, find it */
-	free_pages_check_bad(page);
+	check_free_page_bad(page);
 	return 1;
 }
 
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [Patch v3 4/5] mm/page_alloc.c: rename free_pages_check() to check_free_page()
  2020-04-11 22:03 [Patch v3 0/5] mm/page_alloc.c: cleanup on check page Wei Yang
                   ` (2 preceding siblings ...)
  2020-04-11 22:03 ` [Patch v3 3/5] mm/page_alloc.c: rename free_pages_check_bad() to check_free_page_bad() Wei Yang
@ 2020-04-11 22:03 ` Wei Yang
  2020-04-11 22:03 ` [Patch v3 5/5] mm/page_alloc.c: extract check_[new|free]_page_bad() common part to page_bad_reason() Wei Yang
  2020-04-11 22:06 ` [Patch v3 0/5] mm/page_alloc.c: cleanup on check page Wei Yang
  5 siblings, 0 replies; 7+ messages in thread
From: Wei Yang @ 2020-04-11 22:03 UTC (permalink / raw)
  To: akpm; +Cc: linux-mm, linux-kernel, rientjes, anshuman.khandual, david, Wei Yang

Function free_pages_check() is the counterpart of check_new_page().
Rename it to use the same name convention.

Signed-off-by: Wei Yang <richard.weiyang@gmail.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 85d7aec5fb45..dfcf2682ed40 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1032,7 +1032,7 @@ static void check_free_page_bad(struct page *page)
 	bad_page(page, bad_reason);
 }
 
-static inline int free_pages_check(struct page *page)
+static inline int check_free_page(struct page *page)
 {
 	if (likely(page_expected_state(page, PAGE_FLAGS_CHECK_AT_FREE)))
 		return 0;
@@ -1124,7 +1124,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
 		for (i = 1; i < (1 << order); i++) {
 			if (compound)
 				bad += free_tail_pages_check(page, page + i);
-			if (unlikely(free_pages_check(page + i))) {
+			if (unlikely(check_free_page(page + i))) {
 				bad++;
 				continue;
 			}
@@ -1136,7 +1136,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
 	if (memcg_kmem_enabled() && PageKmemcg(page))
 		__memcg_kmem_uncharge(page, order);
 	if (check_free)
-		bad += free_pages_check(page);
+		bad += check_free_page(page);
 	if (bad)
 		return false;
 
@@ -1183,7 +1183,7 @@ static bool free_pcp_prepare(struct page *page)
 static bool bulkfree_pcp_prepare(struct page *page)
 {
 	if (debug_pagealloc_enabled_static())
-		return free_pages_check(page);
+		return check_free_page(page);
 	else
 		return false;
 }
@@ -1204,7 +1204,7 @@ static bool free_pcp_prepare(struct page *page)
 
 static bool bulkfree_pcp_prepare(struct page *page)
 {
-	return free_pages_check(page);
+	return check_free_page(page);
 }
 #endif /* CONFIG_DEBUG_VM */
 
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [Patch v3 5/5] mm/page_alloc.c: extract check_[new|free]_page_bad() common part to page_bad_reason()
  2020-04-11 22:03 [Patch v3 0/5] mm/page_alloc.c: cleanup on check page Wei Yang
                   ` (3 preceding siblings ...)
  2020-04-11 22:03 ` [Patch v3 4/5] mm/page_alloc.c: rename free_pages_check() to check_free_page() Wei Yang
@ 2020-04-11 22:03 ` Wei Yang
  2020-04-11 22:06 ` [Patch v3 0/5] mm/page_alloc.c: cleanup on check page Wei Yang
  5 siblings, 0 replies; 7+ messages in thread
From: Wei Yang @ 2020-04-11 22:03 UTC (permalink / raw)
  To: akpm; +Cc: linux-mm, linux-kernel, rientjes, anshuman.khandual, david, Wei Yang

We share the similar code in check_[new|free]_page_bad() to get the
page's bad reason.

Let's extract it and reduce code duplication.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 mm/page_alloc.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index dfcf2682ed40..c12a5a9b79c8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1013,7 +1013,7 @@ static inline bool page_expected_state(struct page *page,
 	return true;
 }
 
-static void check_free_page_bad(struct page *page)
+static const char *page_bad_reason(struct page *page, unsigned long flags)
 {
 	const char *bad_reason = NULL;
 
@@ -1023,13 +1023,23 @@ static void check_free_page_bad(struct page *page)
 		bad_reason = "non-NULL mapping";
 	if (unlikely(page_ref_count(page) != 0))
 		bad_reason = "nonzero _refcount";
-	if (unlikely(page->flags & PAGE_FLAGS_CHECK_AT_FREE))
-		bad_reason = "PAGE_FLAGS_CHECK_AT_FREE flag(s) set";
+	if (unlikely(page->flags & flags)) {
+		if (flags == PAGE_FLAGS_CHECK_AT_PREP)
+			bad_reason = "PAGE_FLAGS_CHECK_AT_PREP flag(s) set";
+		else
+			bad_reason = "PAGE_FLAGS_CHECK_AT_FREE flag(s) set";
+	}
 #ifdef CONFIG_MEMCG
 	if (unlikely(page->mem_cgroup))
 		bad_reason = "page still charged to cgroup";
 #endif
-	bad_page(page, bad_reason);
+	return bad_reason;
+}
+
+static void check_free_page_bad(struct page *page)
+{
+	bad_page(page,
+		 page_bad_reason(page, PAGE_FLAGS_CHECK_AT_FREE));
 }
 
 static inline int check_free_page(struct page *page)
@@ -2023,26 +2033,14 @@ static inline void expand(struct zone *zone, struct page *page,
 
 static void check_new_page_bad(struct page *page)
 {
-	const char *bad_reason = NULL;
-
 	if (unlikely(page->flags & __PG_HWPOISON)) {
 		/* Don't complain about hwpoisoned pages */
 		page_mapcount_reset(page); /* remove PageBuddy */
 		return;
 	}
-	if (unlikely(atomic_read(&page->_mapcount) != -1))
-		bad_reason = "nonzero mapcount";
-	if (unlikely(page->mapping != NULL))
-		bad_reason = "non-NULL mapping";
-	if (unlikely(page_ref_count(page) != 0))
-		bad_reason = "nonzero _refcount";
-	if (unlikely(page->flags & PAGE_FLAGS_CHECK_AT_PREP))
-		bad_reason = "PAGE_FLAGS_CHECK_AT_PREP flag set";
-#ifdef CONFIG_MEMCG
-	if (unlikely(page->mem_cgroup))
-		bad_reason = "page still charged to cgroup";
-#endif
-	bad_page(page, bad_reason);
+
+	bad_page(page,
+		 page_bad_reason(page, PAGE_FLAGS_CHECK_AT_PREP));
 }
 
 /*
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [Patch v3 0/5] mm/page_alloc.c: cleanup on check page
  2020-04-11 22:03 [Patch v3 0/5] mm/page_alloc.c: cleanup on check page Wei Yang
                   ` (4 preceding siblings ...)
  2020-04-11 22:03 ` [Patch v3 5/5] mm/page_alloc.c: extract check_[new|free]_page_bad() common part to page_bad_reason() Wei Yang
@ 2020-04-11 22:06 ` Wei Yang
  5 siblings, 0 replies; 7+ messages in thread
From: Wei Yang @ 2020-04-11 22:06 UTC (permalink / raw)
  To: Wei Yang; +Cc: akpm, linux-mm, linux-kernel, rientjes, anshuman.khandual, david

On Sat, Apr 11, 2020 at 10:03:52PM +0000, Wei Yang wrote:
>The patch set does some cleanup related to check page.
>
>1. Remove unnecessary bad_reason assignment
>2. Remove bad_flags to bad_page()
>3. Rename function for naming convention
>4. Extract common part to check page
>
>Thanks suggestions from David Rientjes and Anshuman Khandual.

Oops, miss the history.

v3:
  * still just print the highest priority bad reason
  * remove the bad_flags to bad_page()

v2:
  * merge two rename patches into extract patch
  * enable dump several reasons for __dump_page()

>
>Wei Yang (5):
>  mm/page_alloc.c: bad_[reason|flags] is not necessary when PageHWPoison
>  mm/page_alloc.c: bad_flags is not necessary for bad_page()
>  mm/page_alloc.c: rename free_pages_check_bad() to
>    check_free_page_bad()
>  mm/page_alloc.c: rename free_pages_check() to check_free_page()
>  mm/page_alloc.c: extract check_[new|free]_page_bad() common part to
>    page_bad_reason()
>
> mm/page_alloc.c | 74 +++++++++++++++++++------------------------------
> 1 file changed, 28 insertions(+), 46 deletions(-)
>
>-- 
>2.23.0

-- 
Wei Yang
Help you, Help me


^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-04-11 22:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-11 22:03 [Patch v3 0/5] mm/page_alloc.c: cleanup on check page Wei Yang
2020-04-11 22:03 ` [Patch v3 1/5] mm/page_alloc.c: bad_[reason|flags] is not necessary when PageHWPoison Wei Yang
2020-04-11 22:03 ` [Patch v3 2/5] mm/page_alloc.c: bad_flags is not necessary for bad_page() Wei Yang
2020-04-11 22:03 ` [Patch v3 3/5] mm/page_alloc.c: rename free_pages_check_bad() to check_free_page_bad() Wei Yang
2020-04-11 22:03 ` [Patch v3 4/5] mm/page_alloc.c: rename free_pages_check() to check_free_page() Wei Yang
2020-04-11 22:03 ` [Patch v3 5/5] mm/page_alloc.c: extract check_[new|free]_page_bad() common part to page_bad_reason() Wei Yang
2020-04-11 22:06 ` [Patch v3 0/5] mm/page_alloc.c: cleanup on check page Wei Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).