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
Function free_pages_check() is the counterpart of check_new_page().
Rename it to use the same name convention.
Signed-off-by: Wei Yang <[email protected]>
---
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
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 <[email protected]>
Acked-by: Michal Hocko <[email protected]>
Reviewed-by: David Hildenbrand <[email protected]>
---
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
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 <[email protected]>
Suggested-by: Anshuman Khandual <[email protected]>
---
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
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 <[email protected]>
---
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
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