[PATCH 0/8] PageFlags cleanups

[PATCH 0/8] PageFlags cleanups

[Matthew Wilcox (Oracle)]

We have now successfully removed all of the uses of some of the PageFlags
from the kernel, but there's nothing to stop somebody reintroducing them.
By splitting out FOLIO_FLAGS from PAGEFLAGS, we can stop defining the
old flags; and we do that in some of the later patches.

After doing this, I realised that dump_page() was living dangerously;
we could end up calling folio_test_foo() on a pointer which no longer
pointed to a folio (as dump_page() is not necessarily called when the
caller has a reference to the page).  So I fixed that up.

And then I realised that this was the key to making dump_page() take
a const argument, which means we can constify the page flags testing,
which means we can remove more cast-away-the-const bad code.

And here's where I ended up.

Matthew Wilcox (Oracle) (8):
  mm: Separate out FOLIO_FLAGS from PAGEFLAGS
  mm: Remove PageWaiters, PageSetWaiters and PageClearWaiters
  mm: Remove PageYoung and PageIdle definitions
  mm: Add __dump_folio()
  mm: Make dump_page() take a const argument
  mm: Constify testing page/folio flags
  mm: Constify more page/folio tests
  mm: Remove cast from page_to_nid()

 include/linux/mm.h         |   6 +-
 include/linux/mmdebug.h    |   2 +-
 include/linux/page-flags.h | 153 +++++++++++++++++++++----------------
 mm/debug.c                 | 122 ++++++++++++++++-------------
 mm/hugetlb.c               |   4 +-
 5 files changed, 161 insertions(+), 126 deletions(-)

-- 
2.43.0

[PATCH 1/8] mm: Separate out FOLIO_FLAGS from PAGEFLAGS

[Matthew Wilcox (Oracle)]

We've progressed far enough with the folio transition that some flags
are now no longer checked on pages, but only on folios.  To prevent
new users appearing, prepare to only define the folio versions of the
flag test/set/clear.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/page-flags.h | 63 ++++++++++++++++++++++++++------------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 735cddc13d20..95ab75d0b39c 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -367,54 +367,77 @@ static unsigned long *folio_flags(struct folio *folio, unsigned n)
 #define FOLIO_PF_NO_COMPOUND	0
 #define FOLIO_PF_SECOND		1
 
+#define FOLIO_HEAD_PAGE		0
+#define FOLIO_SECOND_PAGE	1
+
 /*
  * Macros to create function definitions for page flags
  */
+#define FOLIO_TEST_FLAG(name, page)					\
+static __always_inline bool folio_test_##name(struct folio *folio)	\
+{ return test_bit(PG_##name, folio_flags(folio, page)); }
+
+#define FOLIO_SET_FLAG(name, page)					\
+static __always_inline void folio_set_##name(struct folio *folio)	\
+{ set_bit(PG_##name, folio_flags(folio, page)); }
+
+#define FOLIO_CLEAR_FLAG(name, page)					\
+static __always_inline void folio_clear_##name(struct folio *folio)	\
+{ clear_bit(PG_##name, folio_flags(folio, page)); }
+
+#define __FOLIO_SET_FLAG(name, page)					\
+static __always_inline void __folio_set_##name(struct folio *folio)	\
+{ __set_bit(PG_##name, folio_flags(folio, page)); }
+
+#define __FOLIO_CLEAR_FLAG(name, page)					\
+static __always_inline void __folio_clear_##name(struct folio *folio)	\
+{ __clear_bit(PG_##name, folio_flags(folio, page)); }
+
+#define FOLIO_TEST_SET_FLAG(name, page)					\
+static __always_inline bool folio_test_set_##name(struct folio *folio)	\
+{ return test_and_set_bit(PG_##name, folio_flags(folio, page)); }
+
+#define FOLIO_TEST_CLEAR_FLAG(name, page)				\
+static __always_inline bool folio_test_clear_##name(struct folio *folio) \
+{ return test_and_clear_bit(PG_##name, folio_flags(folio, page)); }
+
+#define FOLIO_FLAG(name, page)						\
+FOLIO_TEST_FLAG(name, page)						\
+FOLIO_SET_FLAG(name, page)						\
+FOLIO_CLEAR_FLAG(name, page)
+
 #define TESTPAGEFLAG(uname, lname, policy)				\
-static __always_inline bool folio_test_##lname(struct folio *folio)	\
-{ return test_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); }	\
+FOLIO_TEST_FLAG(lname, FOLIO_##policy)					\
 static __always_inline int Page##uname(struct page *page)		\
 { return test_bit(PG_##lname, &policy(page, 0)->flags); }
 
 #define SETPAGEFLAG(uname, lname, policy)				\
-static __always_inline							\
-void folio_set_##lname(struct folio *folio)				\
-{ set_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); }		\
+FOLIO_SET_FLAG(lname, FOLIO_##policy)					\
 static __always_inline void SetPage##uname(struct page *page)		\
 { set_bit(PG_##lname, &policy(page, 1)->flags); }
 
 #define CLEARPAGEFLAG(uname, lname, policy)				\
-static __always_inline							\
-void folio_clear_##lname(struct folio *folio)				\
-{ clear_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); }		\
+FOLIO_CLEAR_FLAG(lname, FOLIO_##policy)					\
 static __always_inline void ClearPage##uname(struct page *page)		\
 { clear_bit(PG_##lname, &policy(page, 1)->flags); }
 
 #define __SETPAGEFLAG(uname, lname, policy)				\
-static __always_inline							\
-void __folio_set_##lname(struct folio *folio)				\
-{ __set_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); }		\
+__FOLIO_SET_FLAG(lname, FOLIO_##policy)					\
 static __always_inline void __SetPage##uname(struct page *page)		\
 { __set_bit(PG_##lname, &policy(page, 1)->flags); }
 
 #define __CLEARPAGEFLAG(uname, lname, policy)				\
-static __always_inline							\
-void __folio_clear_##lname(struct folio *folio)				\
-{ __clear_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); }	\
+__FOLIO_CLEAR_FLAG(lname, FOLIO_##policy)				\
 static __always_inline void __ClearPage##uname(struct page *page)	\
 { __clear_bit(PG_##lname, &policy(page, 1)->flags); }
 
 #define TESTSETFLAG(uname, lname, policy)				\
-static __always_inline							\
-bool folio_test_set_##lname(struct folio *folio)			\
-{ return test_and_set_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \
+FOLIO_TEST_SET_FLAG(lname, FOLIO_##policy)				\
 static __always_inline int TestSetPage##uname(struct page *page)	\
 { return test_and_set_bit(PG_##lname, &policy(page, 1)->flags); }
 
 #define TESTCLEARFLAG(uname, lname, policy)				\
-static __always_inline							\
-bool folio_test_clear_##lname(struct folio *folio)			\
-{ return test_and_clear_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \
+FOLIO_TEST_CLEAR_FLAG(lname, FOLIO_##policy)				\
 static __always_inline int TestClearPage##uname(struct page *page)	\
 { return test_and_clear_bit(PG_##lname, &policy(page, 1)->flags); }
 
-- 
2.43.0

[PATCH 2/8] mm: Remove PageWaiters, PageSetWaiters and PageClearWaiters

[Matthew Wilcox (Oracle)]

All callers have been converted to use folios.  This was the only user
of PF_ONLY_HEAD, so remove that too.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/page-flags.h | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 95ab75d0b39c..d8f5127ae72e 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -328,9 +328,6 @@ static unsigned long *folio_flags(struct folio *folio, unsigned n)
  *     for compound page all operations related to the page flag applied to
  *     head page.
  *
- * PF_ONLY_HEAD:
- *     for compound page, callers only ever operate on the head page.
- *
  * PF_NO_TAIL:
  *     modifications of the page flag must be done on small or head pages,
  *     checks can be done on tail pages too.
@@ -346,9 +343,6 @@ static unsigned long *folio_flags(struct folio *folio, unsigned n)
 		page; })
 #define PF_ANY(page, enforce)	PF_POISONED_CHECK(page)
 #define PF_HEAD(page, enforce)	PF_POISONED_CHECK(compound_head(page))
-#define PF_ONLY_HEAD(page, enforce) ({					\
-		VM_BUG_ON_PGFLAGS(PageTail(page), page);		\
-		PF_POISONED_CHECK(page); })
 #define PF_NO_TAIL(page, enforce) ({					\
 		VM_BUG_ON_PGFLAGS(enforce && PageTail(page), page);	\
 		PF_POISONED_CHECK(compound_head(page)); })
@@ -362,7 +356,6 @@ static unsigned long *folio_flags(struct folio *folio, unsigned n)
 /* Which page is the flag stored in */
 #define FOLIO_PF_ANY		0
 #define FOLIO_PF_HEAD		0
-#define FOLIO_PF_ONLY_HEAD	0
 #define FOLIO_PF_NO_TAIL	0
 #define FOLIO_PF_NO_COMPOUND	0
 #define FOLIO_PF_SECOND		1
@@ -488,7 +481,7 @@ static inline int TestClearPage##uname(struct page *page) { return 0; }
 	TESTSETFLAG_FALSE(uname, lname) TESTCLEARFLAG_FALSE(uname, lname)
 
 __PAGEFLAG(Locked, locked, PF_NO_TAIL)
-PAGEFLAG(Waiters, waiters, PF_ONLY_HEAD)
+FOLIO_FLAG(waiters, FOLIO_HEAD_PAGE)
 PAGEFLAG(Error, error, PF_NO_TAIL) TESTCLEARFLAG(Error, error, PF_NO_TAIL)
 PAGEFLAG(Referenced, referenced, PF_HEAD)
 	TESTCLEARFLAG(Referenced, referenced, PF_HEAD)
@@ -1138,7 +1131,6 @@ static inline bool folio_has_private(struct folio *folio)
 
 #undef PF_ANY
 #undef PF_HEAD
-#undef PF_ONLY_HEAD
 #undef PF_NO_TAIL
 #undef PF_NO_COMPOUND
 #undef PF_SECOND
-- 
2.43.0

[PATCH 3/8] mm: Remove PageYoung and PageIdle definitions

[Matthew Wilcox (Oracle)]

All callers have been converted to use folios, so remove the various
set/clear/test functions defined on pages.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/page-flags.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index d8f5127ae72e..582ca7400eca 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -599,10 +599,10 @@ PAGEFLAG_FALSE(HWPoison, hwpoison)
 #endif
 
 #if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT)
-TESTPAGEFLAG(Young, young, PF_ANY)
-SETPAGEFLAG(Young, young, PF_ANY)
-TESTCLEARFLAG(Young, young, PF_ANY)
-PAGEFLAG(Idle, idle, PF_ANY)
+FOLIO_TEST_FLAG(young, FOLIO_HEAD_PAGE)
+FOLIO_SET_FLAG(young, FOLIO_HEAD_PAGE)
+FOLIO_TEST_CLEAR_FLAG(young, FOLIO_HEAD_PAGE)
+FOLIO_FLAG(idle, FOLIO_HEAD_PAGE)
 #endif
 
 /*
-- 
2.43.0

[PATCH 4/8] mm: Add __dump_folio()

[Matthew Wilcox (Oracle)]

Turn __dump_page() into a wrapper around __dump_folio().  Snapshot the
page & folio into a stack variable so we don't hit BUG_ON() if an
allocation is freed under us and what was a folio pointer becomes a
pointer to a tail page.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/debug.c | 120 +++++++++++++++++++++++++++++------------------------
 1 file changed, 66 insertions(+), 54 deletions(-)

diff --git a/mm/debug.c b/mm/debug.c
index ee533a5ceb79..96555fc78f1a 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -51,84 +51,96 @@ const struct trace_print_flags vmaflag_names[] = {
 	{0, NULL}
 };
 
-static void __dump_page(struct page *page)
+static void __dump_folio(struct folio *folio, struct page *page,
+		unsigned long pfn, unsigned long idx)
 {
-	struct folio *folio = page_folio(page);
-	struct page *head = &folio->page;
-	struct address_space *mapping;
-	bool compound = PageCompound(page);
-	/*
-	 * Accessing the pageblock without the zone lock. It could change to
-	 * "isolate" again in the meantime, but since we are just dumping the
-	 * state for debugging, it should be fine to accept a bit of
-	 * inaccuracy here due to racing.
-	 */
-	bool page_cma = is_migrate_cma_page(page);
-	int mapcount;
+	struct address_space *mapping = folio_mapping(folio);
+	bool page_cma;
+	int mapcount = 0;
 	char *type = "";
 
-	if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) {
-		/*
-		 * Corrupt page, so we cannot call page_mapping. Instead, do a
-		 * safe subset of the steps that page_mapping() does. Caution:
-		 * this will be misleading for tail pages, PageSwapCache pages,
-		 * and potentially other situations. (See the page_mapping()
-		 * implementation for what's missing here.)
-		 */
-		unsigned long tmp = (unsigned long)page->mapping;
-
-		if (tmp & PAGE_MAPPING_ANON)
-			mapping = NULL;
-		else
-			mapping = (void *)(tmp & ~PAGE_MAPPING_FLAGS);
-		head = page;
-		folio = (struct folio *)page;
-		compound = false;
-	} else {
-		mapping = page_mapping(page);
-	}
-
 	/*
-	 * Avoid VM_BUG_ON() in page_mapcount().
-	 * page->_mapcount space in struct page is used by sl[aou]b pages to
-	 * encode own info.
+	 * page->_mapcount space in struct page is used by slab pages to
+	 * encode own info, and we must avoid calling page_folio() again.
 	 */
-	mapcount = PageSlab(head) ? 0 : page_mapcount(page);
-
-	pr_warn("page:%p refcount:%d mapcount:%d mapping:%p index:%#lx pfn:%#lx\n",
-			page, page_ref_count(head), mapcount, mapping,
-			page_to_pgoff(page), page_to_pfn(page));
-	if (compound) {
-		pr_warn("head:%p order:%u entire_mapcount:%d nr_pages_mapped:%d pincount:%d\n",
-				head, compound_order(head),
+	if (!folio_test_slab(folio)) {
+		mapcount = atomic_read(&page->_mapcount) + 1;
+		if (folio_test_large(folio))
+			mapcount += folio_entire_mapcount(folio);
+	}
+
+	pr_warn("page: refcount:%d mapcount:%d mapping:%p index:%#lx pfn:%#lx\n",
+			folio_ref_count(folio), mapcount, mapping,
+			folio->index + idx, pfn);
+	if (folio_test_large(folio)) {
+		pr_warn("head: order:%u entire_mapcount:%d nr_pages_mapped:%d pincount:%d\n",
+				folio_order(folio),
 				folio_entire_mapcount(folio),
 				folio_nr_pages_mapped(folio),
 				atomic_read(&folio->_pincount));
 	}
 
 #ifdef CONFIG_MEMCG
-	if (head->memcg_data)
-		pr_warn("memcg:%lx\n", head->memcg_data);
+	if (folio->memcg_data)
+		pr_warn("memcg:%lx\n", folio->memcg_data);
 #endif
-	if (PageKsm(page))
+	if (folio_test_ksm(folio))
 		type = "ksm ";
-	else if (PageAnon(page))
+	else if (folio_test_anon(folio))
 		type = "anon ";
 	else if (mapping)
 		dump_mapping(mapping);
 	BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1);
 
-	pr_warn("%sflags: %pGp%s\n", type, &head->flags,
+	/*
+	 * Accessing the pageblock without the zone lock. It could change to
+	 * "isolate" again in the meantime, but since we are just dumping the
+	 * state for debugging, it should be fine to accept a bit of
+	 * inaccuracy here due to racing.
+	 */
+	page_cma = is_migrate_cma_page(page);
+	pr_warn("%sflags: %pGp%s\n", type, &folio->flags,
 		page_cma ? " CMA" : "");
-	pr_warn("page_type: %pGt\n", &head->page_type);
+	pr_warn("page_type: %pGt\n", &folio->page.page_type);
 
 	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
 			sizeof(unsigned long), page,
 			sizeof(struct page), false);
-	if (head != page)
+	if (folio_test_large(folio))
 		print_hex_dump(KERN_WARNING, "head: ", DUMP_PREFIX_NONE, 32,
-			sizeof(unsigned long), head,
-			sizeof(struct page), false);
+			sizeof(unsigned long), folio,
+			2 * sizeof(struct page), false);
+}
+
+static void __dump_page(const struct page *page)
+{
+	struct folio *foliop, folio;
+	struct page precise;
+	unsigned long pfn = page_to_pfn(page);
+	unsigned long idx, nr_pages = 1;
+	int loops = 5;
+
+again:
+	memcpy(&precise, page, sizeof(*page));
+	foliop = page_folio(&precise);
+	idx = folio_page_idx(foliop, page);
+	if (idx != 0) {
+		if (idx < (1UL << PUD_ORDER)) {
+			memcpy(&folio, foliop, 2 * sizeof(struct page));
+			nr_pages = folio_nr_pages(&folio);
+		}
+
+		if (idx > nr_pages) {
+			if (loops-- > 0)
+				goto again;
+			printk("page does not match folio\n");
+			precise.compound_head &= ~1UL;
+			foliop = (struct folio *)&precise;
+			idx = 0;
+		}
+	}
+
+	__dump_folio(foliop, &precise, pfn, idx);
 }
 
 void dump_page(struct page *page, const char *reason)
-- 
2.43.0

[PATCH 5/8] mm: Make dump_page() take a const argument

[Matthew Wilcox (Oracle)]

Now that __dump_page() takes a const argument, we can make dump_page()
take a const struct page too.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/mmdebug.h | 2 +-
 mm/debug.c              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
index 7c3e7b0b0e8f..39a7714605a7 100644
--- a/include/linux/mmdebug.h
+++ b/include/linux/mmdebug.h
@@ -10,7 +10,7 @@ struct vm_area_struct;
 struct mm_struct;
 struct vma_iterator;
 
-void dump_page(struct page *page, const char *reason);
+void dump_page(const struct page *page, const char *reason);
 void dump_vma(const struct vm_area_struct *vma);
 void dump_mm(const struct mm_struct *mm);
 void vma_iter_dump_tree(const struct vma_iterator *vmi);
diff --git a/mm/debug.c b/mm/debug.c
index 96555fc78f1a..6149944016a7 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -143,7 +143,7 @@ static void __dump_page(const struct page *page)
 	__dump_folio(foliop, &precise, pfn, idx);
 }
 
-void dump_page(struct page *page, const char *reason)
+void dump_page(const struct page *page, const char *reason)
 {
 	if (PagePoisoned(page))
 		pr_warn("page:%p is uninitialized and poisoned", page);
-- 
2.43.0

[PATCH 6/8] mm: Constify testing page/folio flags

[Matthew Wilcox (Oracle)]

Now that dump_page() takes a const argument, we can constify all the
page flag tests.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/page-flags.h | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 582ca7400eca..3463cd1baebf 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -237,7 +237,7 @@ static inline const struct page *page_fixed_fake_head(const struct page *page)
 }
 #endif
 
-static __always_inline int page_is_fake_head(struct page *page)
+static __always_inline int page_is_fake_head(const struct page *page)
 {
 	return page_fixed_fake_head(page) != page;
 }
@@ -281,12 +281,12 @@ static inline unsigned long _compound_head(const struct page *page)
  */
 #define folio_page(folio, n)	nth_page(&(folio)->page, n)
 
-static __always_inline int PageTail(struct page *page)
+static __always_inline int PageTail(const struct page *page)
 {
 	return READ_ONCE(page->compound_head) & 1 || page_is_fake_head(page);
 }
 
-static __always_inline int PageCompound(struct page *page)
+static __always_inline int PageCompound(const struct page *page)
 {
 	return test_bit(PG_head, &page->flags) ||
 	       READ_ONCE(page->compound_head) & 1;
@@ -306,6 +306,16 @@ static inline void page_init_poison(struct page *page, size_t size)
 }
 #endif
 
+static const unsigned long *const_folio_flags(const struct folio *folio,
+		unsigned n)
+{
+	const struct page *page = &folio->page;
+
+	VM_BUG_ON_PGFLAGS(PageTail(page), page);
+	VM_BUG_ON_PGFLAGS(n > 0 && !test_bit(PG_head, &page->flags), page);
+	return &page[n].flags;
+}
+
 static unsigned long *folio_flags(struct folio *folio, unsigned n)
 {
 	struct page *page = &folio->page;
@@ -367,8 +377,8 @@ static unsigned long *folio_flags(struct folio *folio, unsigned n)
  * Macros to create function definitions for page flags
  */
 #define FOLIO_TEST_FLAG(name, page)					\
-static __always_inline bool folio_test_##name(struct folio *folio)	\
-{ return test_bit(PG_##name, folio_flags(folio, page)); }
+static __always_inline bool folio_test_##name(const struct folio *folio) \
+{ return test_bit(PG_##name, const_folio_flags(folio, page)); }
 
 #define FOLIO_SET_FLAG(name, page)					\
 static __always_inline void folio_set_##name(struct folio *folio)	\
@@ -401,7 +411,7 @@ FOLIO_CLEAR_FLAG(name, page)
 
 #define TESTPAGEFLAG(uname, lname, policy)				\
 FOLIO_TEST_FLAG(lname, FOLIO_##policy)					\
-static __always_inline int Page##uname(struct page *page)		\
+static __always_inline int Page##uname(const struct page *page)		\
 { return test_bit(PG_##lname, &policy(page, 0)->flags); }
 
 #define SETPAGEFLAG(uname, lname, policy)				\
@@ -801,7 +811,7 @@ static __always_inline bool folio_test_head(struct folio *folio)
 	return test_bit(PG_head, folio_flags(folio, FOLIO_PF_ANY));
 }
 
-static __always_inline int PageHead(struct page *page)
+static __always_inline int PageHead(const struct page *page)
 {
 	PF_POISONED_CHECK(page);
 	return test_bit(PG_head, &page->flags) && !page_is_fake_head(page);
-- 
2.43.0

[PATCH 7/8] mm: Constify more page/folio tests

[Matthew Wilcox (Oracle)]

Constify the flag tests that aren't automatically generated and the
tests that look like flag tests but are more complicated.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/page-flags.h | 52 +++++++++++++++++++-------------------
 mm/hugetlb.c               |  4 +--
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 3463cd1baebf..652d77805e99 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -558,13 +558,13 @@ PAGEFLAG_FALSE(HighMem, highmem)
 #endif
 
 #ifdef CONFIG_SWAP
-static __always_inline bool folio_test_swapcache(struct folio *folio)
+static __always_inline bool folio_test_swapcache(const struct folio *folio)
 {
 	return folio_test_swapbacked(folio) &&
-			test_bit(PG_swapcache, folio_flags(folio, 0));
+			test_bit(PG_swapcache, const_folio_flags(folio, 0));
 }
 
-static __always_inline bool PageSwapCache(struct page *page)
+static __always_inline bool PageSwapCache(const struct page *page)
 {
 	return folio_test_swapcache(page_folio(page));
 }
@@ -663,22 +663,22 @@ PAGEFLAG_FALSE(VmemmapSelfHosted, vmemmap_self_hosted)
  */
 #define PAGE_MAPPING_DAX_SHARED	((void *)0x1)
 
-static __always_inline bool folio_mapping_flags(struct folio *folio)
+static __always_inline bool folio_mapping_flags(const struct folio *folio)
 {
 	return ((unsigned long)folio->mapping & PAGE_MAPPING_FLAGS) != 0;
 }
 
-static __always_inline int PageMappingFlags(struct page *page)
+static __always_inline int PageMappingFlags(const struct page *page)
 {
 	return ((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) != 0;
 }
 
-static __always_inline bool folio_test_anon(struct folio *folio)
+static __always_inline bool folio_test_anon(const struct folio *folio)
 {
 	return ((unsigned long)folio->mapping & PAGE_MAPPING_ANON) != 0;
 }
 
-static __always_inline bool PageAnon(struct page *page)
+static __always_inline bool PageAnon(const struct page *page)
 {
 	return folio_test_anon(page_folio(page));
 }
@@ -689,7 +689,7 @@ static __always_inline bool __folio_test_movable(const struct folio *folio)
 			PAGE_MAPPING_MOVABLE;
 }
 
-static __always_inline int __PageMovable(struct page *page)
+static __always_inline int __PageMovable(const struct page *page)
 {
 	return ((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) ==
 				PAGE_MAPPING_MOVABLE;
@@ -702,13 +702,13 @@ static __always_inline int __PageMovable(struct page *page)
  * is found in VM_MERGEABLE vmas.  It's a PageAnon page, pointing not to any
  * anon_vma, but to that page's node of the stable tree.
  */
-static __always_inline bool folio_test_ksm(struct folio *folio)
+static __always_inline bool folio_test_ksm(const struct folio *folio)
 {
 	return ((unsigned long)folio->mapping & PAGE_MAPPING_FLAGS) ==
 				PAGE_MAPPING_KSM;
 }
 
-static __always_inline bool PageKsm(struct page *page)
+static __always_inline bool PageKsm(const struct page *page)
 {
 	return folio_test_ksm(page_folio(page));
 }
@@ -747,9 +747,9 @@ static inline bool folio_xor_flags_has_waiters(struct folio *folio,
  * some of the bytes in it may be; see the is_partially_uptodate()
  * address_space operation.
  */
-static inline bool folio_test_uptodate(struct folio *folio)
+static inline bool folio_test_uptodate(const struct folio *folio)
 {
-	bool ret = test_bit(PG_uptodate, folio_flags(folio, 0));
+	bool ret = test_bit(PG_uptodate, const_folio_flags(folio, 0));
 	/*
 	 * Must ensure that the data we read out of the folio is loaded
 	 * _after_ we've loaded folio->flags to check the uptodate bit.
@@ -764,7 +764,7 @@ static inline bool folio_test_uptodate(struct folio *folio)
 	return ret;
 }
 
-static inline int PageUptodate(struct page *page)
+static inline int PageUptodate(const struct page *page)
 {
 	return folio_test_uptodate(page_folio(page));
 }
@@ -806,9 +806,9 @@ void set_page_writeback(struct page *page);
 #define folio_start_writeback_keepwrite(folio)	\
 	__folio_start_writeback(folio, true)
 
-static __always_inline bool folio_test_head(struct folio *folio)
+static __always_inline bool folio_test_head(const struct folio *folio)
 {
-	return test_bit(PG_head, folio_flags(folio, FOLIO_PF_ANY));
+	return test_bit(PG_head, const_folio_flags(folio, FOLIO_PF_ANY));
 }
 
 static __always_inline int PageHead(const struct page *page)
@@ -827,7 +827,7 @@ CLEARPAGEFLAG(Head, head, PF_ANY)
  *
  * Return: True if the folio is larger than one page.
  */
-static inline bool folio_test_large(struct folio *folio)
+static inline bool folio_test_large(const struct folio *folio)
 {
 	return folio_test_head(folio);
 }
@@ -856,7 +856,7 @@ TESTPAGEFLAG_FALSE(LargeRmappable, large_rmappable)
 #define PG_head_mask ((1UL << PG_head))
 
 #ifdef CONFIG_HUGETLB_PAGE
-int PageHuge(struct page *page);
+int PageHuge(const struct page *page);
 SETPAGEFLAG(HugeTLB, hugetlb, PF_SECOND)
 CLEARPAGEFLAG(HugeTLB, hugetlb, PF_SECOND)
 
@@ -869,10 +869,10 @@ CLEARPAGEFLAG(HugeTLB, hugetlb, PF_SECOND)
  * Return: True for hugetlbfs folios, false for anon folios or folios
  * belonging to other filesystems.
  */
-static inline bool folio_test_hugetlb(struct folio *folio)
+static inline bool folio_test_hugetlb(const struct folio *folio)
 {
 	return folio_test_large(folio) &&
-		test_bit(PG_hugetlb, folio_flags(folio, 1));
+		test_bit(PG_hugetlb, const_folio_flags(folio, 1));
 }
 #else
 TESTPAGEFLAG_FALSE(Huge, hugetlb)
@@ -887,7 +887,7 @@ TESTPAGEFLAG_FALSE(Huge, hugetlb)
  * hugetlbfs pages, but not normal pages. PageTransHuge() can only be
  * called only in the core VM paths where hugetlbfs pages can't exist.
  */
-static inline int PageTransHuge(struct page *page)
+static inline int PageTransHuge(const struct page *page)
 {
 	VM_BUG_ON_PAGE(PageTail(page), page);
 	return PageHead(page);
@@ -898,7 +898,7 @@ static inline int PageTransHuge(struct page *page)
  * and hugetlbfs pages, so it should only be called when it's known
  * that hugetlbfs pages aren't involved.
  */
-static inline int PageTransCompound(struct page *page)
+static inline int PageTransCompound(const struct page *page)
 {
 	return PageCompound(page);
 }
@@ -908,7 +908,7 @@ static inline int PageTransCompound(struct page *page)
  * and hugetlbfs pages, so it should only be called when it's known
  * that hugetlbfs pages aren't involved.
  */
-static inline int PageTransTail(struct page *page)
+static inline int PageTransTail(const struct page *page)
 {
 	return PageTail(page);
 }
@@ -972,7 +972,7 @@ static inline int page_type_has_type(unsigned int page_type)
 	return (int)page_type < PAGE_MAPCOUNT_RESERVE;
 }
 
-static inline int page_has_type(struct page *page)
+static inline int page_has_type(const struct page *page)
 {
 	return page_type_has_type(page->page_type);
 }
@@ -1056,7 +1056,7 @@ extern bool is_free_buddy_page(struct page *page);
 
 PAGEFLAG(Isolated, isolated, PF_ANY);
 
-static __always_inline int PageAnonExclusive(struct page *page)
+static __always_inline int PageAnonExclusive(const struct page *page)
 {
 	VM_BUG_ON_PGFLAGS(!PageAnon(page), page);
 	VM_BUG_ON_PGFLAGS(PageHuge(page) && !PageHead(page), page);
@@ -1129,12 +1129,12 @@ static __always_inline void __ClearPageAnonExclusive(struct page *page)
  * Determine if a page has private stuff, indicating that release routines
  * should be invoked upon it.
  */
-static inline int page_has_private(struct page *page)
+static inline int page_has_private(const struct page *page)
 {
 	return !!(page->flags & PAGE_FLAGS_PRIVATE);
 }
 
-static inline bool folio_has_private(struct folio *folio)
+static inline bool folio_has_private(const struct folio *folio)
 {
 	return page_has_private(&folio->page);
 }
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 418d66953224..bb17e5c22759 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2164,9 +2164,9 @@ static bool prep_compound_gigantic_folio_for_demote(struct folio *folio,
  * transparent huge pages.  See the PageTransHuge() documentation for more
  * details.
  */
-int PageHuge(struct page *page)
+int PageHuge(const struct page *page)
 {
-	struct folio *folio;
+	const struct folio *folio;
 
 	if (!PageCompound(page))
 		return 0;
-- 
2.43.0

[PATCH 8/8] mm: Remove cast from page_to_nid()

[Matthew Wilcox (Oracle)]

Now that PF_POISONED_CHECK() can take a const argument, we can drop
the cast.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/mm.h | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index c4a76520c967..21a8cfbe75aa 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1662,13 +1662,11 @@ static inline int page_zone_id(struct page *page)
 }
 
 #ifdef NODE_NOT_IN_PAGE_FLAGS
-extern int page_to_nid(const struct page *page);
+int page_to_nid(const struct page *page);
 #else
 static inline int page_to_nid(const struct page *page)
 {
-	struct page *p = (struct page *)page;
-
-	return (PF_POISONED_CHECK(p)->flags >> NODES_PGSHIFT) & NODES_MASK;
+	return (PF_POISONED_CHECK(page)->flags >> NODES_PGSHIFT) & NODES_MASK;
 }
 #endif
 
-- 
2.43.0

Re: [PATCH 4/8] mm: Add __dump_folio()

[SeongJae Park]

Hi,

On Tue, 27 Feb 2024 19:23:31 +0000 "Matthew Wilcox (Oracle)" <willy@infradead.org> wrote:

> Turn __dump_page() into a wrapper around __dump_folio().  Snapshot the
> page & folio into a stack variable so we don't hit BUG_ON() if an
> allocation is freed under us and what was a folio pointer becomes a
> pointer to a tail page.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/debug.c | 120 +++++++++++++++++++++++++++++------------------------
>  1 file changed, 66 insertions(+), 54 deletions(-)
> 
> diff --git a/mm/debug.c b/mm/debug.c
> index ee533a5ceb79..96555fc78f1a 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
[...]
> +static void __dump_page(const struct page *page)
> +{
> +	struct folio *foliop, folio;
> +	struct page precise;
> +	unsigned long pfn = page_to_pfn(page);
> +	unsigned long idx, nr_pages = 1;
> +	int loops = 5;
> +
> +again:
> +	memcpy(&precise, page, sizeof(*page));
> +	foliop = page_folio(&precise);
> +	idx = folio_page_idx(foliop, page);
> +	if (idx != 0) {
> +		if (idx < (1UL << PUD_ORDER)) {
> +			memcpy(&folio, foliop, 2 * sizeof(struct page));
> +			nr_pages = folio_nr_pages(&folio);
> +		}
> +
> +		if (idx > nr_pages) {
> +			if (loops-- > 0)
> +				goto again;
> +			printk("page does not match folio\n");
> +			precise.compound_head &= ~1UL;
> +			foliop = (struct folio *)&precise;
> +			idx = 0;
> +		}
> +	}
> +
> +	__dump_folio(foliop, &precise, pfn, idx);
>  }

I just found one of my build tests that runs with a CONFIG_MMU disabled config
fails with below build error on mm-unstable starting from this patch.  After
enabling CONFIG_MMU, the build didn't fail. I haven't had a time to look into
the code, but reporting first.


      CC      mm/debug.o
    In file included from ...linux/include/asm-generic/pgtable-nopud.h:7,
                     from ...linux/arch/m68k/include/asm/pgtable_no.h:5,
                     from ...linux/arch/m68k/include/asm/pgtable.h:6,
                     from ...linux/include/linux/pgtable.h:6,
                     from ...linux/include/linux/mm.h:29,
                     from ...linux/mm/debug.c:10:
    ...linux/mm/debug.c: In function '__dump_page':
    ...linux/include/asm-generic/pgtable-nop4d.h:11:33: error: 'PGDIR_SHIFT' undeclared (first use in this function); did you mean 'PUD_SHIFT'?
       11 | #define P4D_SHIFT               PGDIR_SHIFT
          |                                 ^~~~~~~~~~~
    ...linux/include/asm-generic/pgtable-nopud.h:18:25: note: in expansion of macro 'P4D_SHIFT'
       18 | #define PUD_SHIFT       P4D_SHIFT
          |                         ^~~~~~~~~
    ...linux/include/linux/pgtable.h:9:26: note: in expansion of macro 'PUD_SHIFT'
        9 | #define PUD_ORDER       (PUD_SHIFT - PAGE_SHIFT)
          |                          ^~~~~~~~~
    ...linux/mm/debug.c:128:35: note: in expansion of macro 'PUD_ORDER'
      128 |                 if (idx < (1UL << PUD_ORDER)) {
          |                                   ^~~~~~~~~
    ...linux/include/asm-generic/pgtable-nop4d.h:11:33: note: each undeclared identifier is reported only once for each function it appears in
       11 | #define P4D_SHIFT               PGDIR_SHIFT
          |                                 ^~~~~~~~~~~
    ...linux/include/asm-generic/pgtable-nopud.h:18:25: note: in expansion of macro 'P4D_SHIFT'
       18 | #define PUD_SHIFT       P4D_SHIFT
          |                         ^~~~~~~~~
    ...linux/include/linux/pgtable.h:9:26: note: in expansion of macro 'PUD_SHIFT'
        9 | #define PUD_ORDER       (PUD_SHIFT - PAGE_SHIFT)
          |                          ^~~~~~~~~
    ...linux/mm/debug.c:128:35: note: in expansion of macro 'PUD_ORDER'
      128 |                 if (idx < (1UL << PUD_ORDER)) {
          |                                   ^~~~~~~~~

Thanks,
SJ

>  
>  void dump_page(struct page *page, const char *reason)
> -- 
> 2.43.0

Re: [PATCH 4/8] mm: Add __dump_folio()

[Matthew Wilcox]

On Wed, Feb 28, 2024 at 01:34:58PM -0800, SeongJae Park wrote:
>     ...linux/mm/debug.c: In function '__dump_page':
>     ...linux/include/asm-generic/pgtable-nop4d.h:11:33: error: 'PGDIR_SHIFT' undeclared (first use in this function); did you mean 'PUD_SHIFT'?
>        11 | #define P4D_SHIFT               PGDIR_SHIFT
>           |                                 ^~~~~~~~~~~
>     ...linux/include/asm-generic/pgtable-nopud.h:18:25: note: in expansion of macro 'P4D_SHIFT'
>        18 | #define PUD_SHIFT       P4D_SHIFT
>           |                         ^~~~~~~~~
>     ...linux/include/linux/pgtable.h:9:26: note: in expansion of macro 'PUD_SHIFT'
>         9 | #define PUD_ORDER       (PUD_SHIFT - PAGE_SHIFT)
>           |                          ^~~~~~~~~
>     ...linux/mm/debug.c:128:35: note: in expansion of macro 'PUD_ORDER'
>       128 |                 if (idx < (1UL << PUD_ORDER)) {

Thanks.  Can you try this?

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 49d87a4d29b9..e25e86b755be 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2078,6 +2078,13 @@ static inline long folio_nr_pages(struct folio *folio)
 #endif
 }
 
+/* Only hugetlbfs can allocate folios larger than MAX_ORDER */
+#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
+#define MAX_FOLIO_NR_PAGES	(1UL << PUD_ORDER)
+#else
+#define MAX_FOLIO_NR_PAGES	MAX_ORDER_NR_PAGES
+#endif
+
 /*
  * compound_nr() returns the number of pages in this potentially compound
  * page.  compound_nr() can be called on a tail page, and is defined to
diff --git a/mm/debug.c b/mm/debug.c
index 6149944016a7..32ac7d79fd04 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -125,7 +125,7 @@ static void __dump_page(const struct page *page)
 	foliop = page_folio(&precise);
 	idx = folio_page_idx(foliop, page);
 	if (idx != 0) {
-		if (idx < (1UL << PUD_ORDER)) {
+		if (idx < MAX_FOLIO_NR_PAGES) {
 			memcpy(&folio, foliop, 2 * sizeof(struct page));
 			nr_pages = folio_nr_pages(&folio);
 		}

Re: [PATCH 4/8] mm: Add __dump_folio()

[SeongJae Park]

On Thu, 29 Feb 2024 04:37:31 +0000 Matthew Wilcox <willy@infradead.org> wrote:

> On Wed, Feb 28, 2024 at 01:34:58PM -0800, SeongJae Park wrote:
> >     ...linux/mm/debug.c: In function '__dump_page':
> >     ...linux/include/asm-generic/pgtable-nop4d.h:11:33: error: 'PGDIR_SHIFT' undeclared (first use in this function); did you mean 'PUD_SHIFT'?
> >        11 | #define P4D_SHIFT               PGDIR_SHIFT
> >           |                                 ^~~~~~~~~~~
> >     ...linux/include/asm-generic/pgtable-nopud.h:18:25: note: in expansion of macro 'P4D_SHIFT'
> >        18 | #define PUD_SHIFT       P4D_SHIFT
> >           |                         ^~~~~~~~~
> >     ...linux/include/linux/pgtable.h:9:26: note: in expansion of macro 'PUD_SHIFT'
> >         9 | #define PUD_ORDER       (PUD_SHIFT - PAGE_SHIFT)
> >           |                          ^~~~~~~~~
> >     ...linux/mm/debug.c:128:35: note: in expansion of macro 'PUD_ORDER'
> >       128 |                 if (idx < (1UL << PUD_ORDER)) {
> 
> Thanks.  Can you try this?
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 49d87a4d29b9..e25e86b755be 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2078,6 +2078,13 @@ static inline long folio_nr_pages(struct folio *folio)
>  #endif
>  }
>  
> +/* Only hugetlbfs can allocate folios larger than MAX_ORDER */
> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
> +#define MAX_FOLIO_NR_PAGES	(1UL << PUD_ORDER)
> +#else
> +#define MAX_FOLIO_NR_PAGES	MAX_ORDER_NR_PAGES
> +#endif
> +
>  /*
>   * compound_nr() returns the number of pages in this potentially compound
>   * page.  compound_nr() can be called on a tail page, and is defined to
> diff --git a/mm/debug.c b/mm/debug.c
> index 6149944016a7..32ac7d79fd04 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -125,7 +125,7 @@ static void __dump_page(const struct page *page)
>  	foliop = page_folio(&precise);
>  	idx = folio_page_idx(foliop, page);
>  	if (idx != 0) {
> -		if (idx < (1UL << PUD_ORDER)) {
> +		if (idx < MAX_FOLIO_NR_PAGES) {
>  			memcpy(&folio, foliop, 2 * sizeof(struct page));
>  			nr_pages = folio_nr_pages(&folio);
>  		}

Thank you for this fast and kind reply.  I confirmed this fixes my issue :)

Tested-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

Re: [PATCH 4/8] mm: Add __dump_folio()

[Ryan Roberts]

Hi Matthew,

On 27/02/2024 19:23, Matthew Wilcox (Oracle) wrote:
> Turn __dump_page() into a wrapper around __dump_folio().  Snapshot the
> page & folio into a stack variable so we don't hit BUG_ON() if an
> allocation is freed under us and what was a folio pointer becomes a
> pointer to a tail page.

I'm seeing a couple of panics caused by this patch. I already raised the first one at [1], and it looks like there is a bug in Ard's patch (which he now has a proposed fix for) which provokes the bug in this.

[1] https://lore.kernel.org/linux-arm-kernel/fc691e8d-1a50-4be6-a3b2-d60d6f2e2487@arm.com/

The other way to trigger it is to run the mm kselftest:

  gup_test -ct -F 0x1 0 19 0x1000

This calls into the kernel and deliberately calls dump_page() via dump_pages_test() in gup_test.c.

Panic is as follows (root cause identified below):

[   22.994800] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
[   22.995428] Mem abort info:
[   22.995617]   ESR = 0x0000000096000005
[   22.995867]   EC = 0x25: DABT (current EL), IL = 32 bits
[   22.996215]   SET = 0, FnV = 0
[   22.996419]   EA = 0, S1PTW = 0
[   22.996628]   FSC = 0x05: level 1 translation fault
[   22.996951] Data abort info:
[   22.997145]   ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
[   22.997541]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   22.998025]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   22.998438] user pgtable: 4k pages, 39-bit VAs, pgdp=000000019f2d6000
[   22.998937] [0000000000000008] pgd=0000000000000000, p4d=0000000000000000, pud=0000000000000000
[   22.999608] Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
[   23.000083] Modules linked in:
[   23.000319] CPU: 6 PID: 1222 Comm: gup_test Not tainted 6.8.0-rc6-00915-g7f43e0f76e47 #2
[   23.000883] Hardware name: linux,dummy-virt (DT)
[   23.001209] pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   23.001621] pc : get_pfnblock_flags_mask+0x3c/0x68
[   23.001929] lr : __dump_page+0x188/0x400
[   23.002168] sp : ffffffc0885ebb40
[   23.002370] x29: ffffffc0885ebb40 x28: 0000000000ffffc0 x27: 0000000000000000
[   23.002839] x26: 0000000000000000 x25: ffffffc0885ebba0 x24: 00000000ffffffff
[   23.003335] x23: ffffffeabbbc1000 x22: 0000000000000030 x21: ffffffeabb9f5e98
[   23.003869] x20: ffffffc0885ebba0 x19: ffffffc0885ebba0 x18: ffffffffffffffff
[   23.004489] x17: 34383833383a6465 x16: 7070616d5f736567 x15: ffffffeabd058782
[   23.004989] x14: 0000000000000000 x13: 3030303730303039 x12: 3138666666666666
[   23.005501] x11: fffffffffffe0000 x10: ffffffeabca9c018 x9 : ffffffeab9f0c798
[   23.005980] x8 : 00000000ffffefff x7 : ffffffeabca9c018 x6 : 0000000000000000
[   23.006448] x5 : 000003fffffffc28 x4 : 0001fffffffe144a x3 : 0000000000000000
[   23.006931] x2 : 0000000000000007 x1 : ffffffff0a257aee x0 : 00000000001fffff
[   23.007436] Call trace:
[   23.007623]  get_pfnblock_flags_mask+0x3c/0x68
[   23.007928]  dump_page+0x2c/0x70
[   23.008156]  gup_test_ioctl+0xb34/0xc40
[   23.008416]  __arm64_sys_ioctl+0xb0/0x100
[   23.008694]  invoke_syscall+0x50/0x128
[   23.008944]  el0_svc_common.constprop.0+0x48/0xf8
[   23.009259]  do_el0_svc+0x28/0x40
[   23.009499]  el0_svc+0x34/0xb8
[   23.009720]  el0t_64_sync_handler+0x13c/0x158
[   23.010029]  el0t_64_sync+0x190/0x198
[   23.010293] Code: d37b1884 f100007f 8b040064 9a831083 (f9400460) 
[   23.010714] ---[ end trace 0000000000000000 ]---


> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/debug.c | 120 +++++++++++++++++++++++++++++------------------------
>  1 file changed, 66 insertions(+), 54 deletions(-)
> 
> diff --git a/mm/debug.c b/mm/debug.c
> index ee533a5ceb79..96555fc78f1a 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -51,84 +51,96 @@ const struct trace_print_flags vmaflag_names[] = {
>  	{0, NULL}
>  };
>  
> -static void __dump_page(struct page *page)
> +static void __dump_folio(struct folio *folio, struct page *page,
> +		unsigned long pfn, unsigned long idx)
>  {
> -	struct folio *folio = page_folio(page);
> -	struct page *head = &folio->page;
> -	struct address_space *mapping;
> -	bool compound = PageCompound(page);
> -	/*
> -	 * Accessing the pageblock without the zone lock. It could change to
> -	 * "isolate" again in the meantime, but since we are just dumping the
> -	 * state for debugging, it should be fine to accept a bit of
> -	 * inaccuracy here due to racing.
> -	 */
> -	bool page_cma = is_migrate_cma_page(page);
> -	int mapcount;
> +	struct address_space *mapping = folio_mapping(folio);
> +	bool page_cma;
> +	int mapcount = 0;
>  	char *type = "";
>  
> -	if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) {
> -		/*
> -		 * Corrupt page, so we cannot call page_mapping. Instead, do a
> -		 * safe subset of the steps that page_mapping() does. Caution:
> -		 * this will be misleading for tail pages, PageSwapCache pages,
> -		 * and potentially other situations. (See the page_mapping()
> -		 * implementation for what's missing here.)
> -		 */
> -		unsigned long tmp = (unsigned long)page->mapping;
> -
> -		if (tmp & PAGE_MAPPING_ANON)
> -			mapping = NULL;
> -		else
> -			mapping = (void *)(tmp & ~PAGE_MAPPING_FLAGS);
> -		head = page;
> -		folio = (struct folio *)page;
> -		compound = false;
> -	} else {
> -		mapping = page_mapping(page);
> -	}
> -
>  	/*
> -	 * Avoid VM_BUG_ON() in page_mapcount().
> -	 * page->_mapcount space in struct page is used by sl[aou]b pages to
> -	 * encode own info.
> +	 * page->_mapcount space in struct page is used by slab pages to
> +	 * encode own info, and we must avoid calling page_folio() again.
>  	 */
> -	mapcount = PageSlab(head) ? 0 : page_mapcount(page);
> -
> -	pr_warn("page:%p refcount:%d mapcount:%d mapping:%p index:%#lx pfn:%#lx\n",
> -			page, page_ref_count(head), mapcount, mapping,
> -			page_to_pgoff(page), page_to_pfn(page));
> -	if (compound) {
> -		pr_warn("head:%p order:%u entire_mapcount:%d nr_pages_mapped:%d pincount:%d\n",
> -				head, compound_order(head),
> +	if (!folio_test_slab(folio)) {
> +		mapcount = atomic_read(&page->_mapcount) + 1;
> +		if (folio_test_large(folio))
> +			mapcount += folio_entire_mapcount(folio);
> +	}
> +
> +	pr_warn("page: refcount:%d mapcount:%d mapping:%p index:%#lx pfn:%#lx\n",
> +			folio_ref_count(folio), mapcount, mapping,
> +			folio->index + idx, pfn);
> +	if (folio_test_large(folio)) {
> +		pr_warn("head: order:%u entire_mapcount:%d nr_pages_mapped:%d pincount:%d\n",
> +				folio_order(folio),
>  				folio_entire_mapcount(folio),
>  				folio_nr_pages_mapped(folio),
>  				atomic_read(&folio->_pincount));
>  	}
>  
>  #ifdef CONFIG_MEMCG
> -	if (head->memcg_data)
> -		pr_warn("memcg:%lx\n", head->memcg_data);
> +	if (folio->memcg_data)
> +		pr_warn("memcg:%lx\n", folio->memcg_data);
>  #endif
> -	if (PageKsm(page))
> +	if (folio_test_ksm(folio))
>  		type = "ksm ";
> -	else if (PageAnon(page))
> +	else if (folio_test_anon(folio))
>  		type = "anon ";
>  	else if (mapping)
>  		dump_mapping(mapping);
>  	BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1);
>  
> -	pr_warn("%sflags: %pGp%s\n", type, &head->flags,
> +	/*
> +	 * Accessing the pageblock without the zone lock. It could change to
> +	 * "isolate" again in the meantime, but since we are just dumping the
> +	 * state for debugging, it should be fine to accept a bit of
> +	 * inaccuracy here due to racing.
> +	 */
> +	page_cma = is_migrate_cma_page(page);

Problem is here: is_migrate_cma_page() is a macro that resolves to this:

page_cma = get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK) == MIGRATE_CMA;

And since page is on the stack, page_to_pfn() gives a very wrong answer.

I confirmed that the problem goes away for both cases above, when changing the line to:

page_cma = get_pfnblock_flags_mask(page, pfn, MIGRATETYPE_MASK) == MIGRATE_CMA;

Thanks,
Ryan

> +	pr_warn("%sflags: %pGp%s\n", type, &folio->flags,
>  		page_cma ? " CMA" : "");
> -	pr_warn("page_type: %pGt\n", &head->page_type);
> +	pr_warn("page_type: %pGt\n", &folio->page.page_type);
>  
>  	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
>  			sizeof(unsigned long), page,
>  			sizeof(struct page), false);
> -	if (head != page)
> +	if (folio_test_large(folio))
>  		print_hex_dump(KERN_WARNING, "head: ", DUMP_PREFIX_NONE, 32,
> -			sizeof(unsigned long), head,
> -			sizeof(struct page), false);
> +			sizeof(unsigned long), folio,
> +			2 * sizeof(struct page), false);
> +}
> +
> +static void __dump_page(const struct page *page)
> +{
> +	struct folio *foliop, folio;
> +	struct page precise;
> +	unsigned long pfn = page_to_pfn(page);
> +	unsigned long idx, nr_pages = 1;
> +	int loops = 5;
> +
> +again:
> +	memcpy(&precise, page, sizeof(*page));
> +	foliop = page_folio(&precise);
> +	idx = folio_page_idx(foliop, page);
> +	if (idx != 0) {
> +		if (idx < (1UL << PUD_ORDER)) {
> +			memcpy(&folio, foliop, 2 * sizeof(struct page));
> +			nr_pages = folio_nr_pages(&folio);
> +		}
> +
> +		if (idx > nr_pages) {
> +			if (loops-- > 0)
> +				goto again;
> +			printk("page does not match folio\n");
> +			precise.compound_head &= ~1UL;
> +			foliop = (struct folio *)&precise;
> +			idx = 0;
> +		}
> +	}
> +
> +	__dump_folio(foliop, &precise, pfn, idx);
>  }
>  
>  void dump_page(struct page *page, const char *reason)

Re: [PATCH 1/8] mm: Separate out FOLIO_FLAGS from PAGEFLAGS

[David Hildenbrand]

On 27.02.24 20:23, Matthew Wilcox (Oracle) wrote:
> We've progressed far enough with the folio transition that some flags
> are now no longer checked on pages, but only on folios.  To prevent
> new users appearing, prepare to only define the folio versions of the
> flag test/set/clear.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   include/linux/page-flags.h | 63 ++++++++++++++++++++++++++------------
>   1 file changed, 43 insertions(+), 20 deletions(-)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 735cddc13d20..95ab75d0b39c 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -367,54 +367,77 @@ static unsigned long *folio_flags(struct folio *folio, unsigned n)
>   #define FOLIO_PF_NO_COMPOUND	0
>   #define FOLIO_PF_SECOND		1
>   
> +#define FOLIO_HEAD_PAGE		0
> +#define FOLIO_SECOND_PAGE	1
> +
>   /*
>    * Macros to create function definitions for page flags
>    */
> +#define FOLIO_TEST_FLAG(name, page)					\
> +static __always_inline bool folio_test_##name(struct folio *folio)	\
> +{ return test_bit(PG_##name, folio_flags(folio, page)); }
> +
> +#define FOLIO_SET_FLAG(name, page)					\
> +static __always_inline void folio_set_##name(struct folio *folio)	\
> +{ set_bit(PG_##name, folio_flags(folio, page)); }
> +
> +#define FOLIO_CLEAR_FLAG(name, page)					\
> +static __always_inline void folio_clear_##name(struct folio *folio)	\
> +{ clear_bit(PG_##name, folio_flags(folio, page)); }
> +
> +#define __FOLIO_SET_FLAG(name, page)					\
> +static __always_inline void __folio_set_##name(struct folio *folio)	\
> +{ __set_bit(PG_##name, folio_flags(folio, page)); }
> +
> +#define __FOLIO_CLEAR_FLAG(name, page)					\
> +static __always_inline void __folio_clear_##name(struct folio *folio)	\
> +{ __clear_bit(PG_##name, folio_flags(folio, page)); }
> +
> +#define FOLIO_TEST_SET_FLAG(name, page)					\
> +static __always_inline bool folio_test_set_##name(struct folio *folio)	\
> +{ return test_and_set_bit(PG_##name, folio_flags(folio, page)); }
> +
> +#define FOLIO_TEST_CLEAR_FLAG(name, page)				\
> +static __always_inline bool folio_test_clear_##name(struct folio *folio) \
> +{ return test_and_clear_bit(PG_##name, folio_flags(folio, page)); }
> +
> +#define FOLIO_FLAG(name, page)						\
> +FOLIO_TEST_FLAG(name, page)						\
> +FOLIO_SET_FLAG(name, page)						\
> +FOLIO_CLEAR_FLAG(name, page)
> +
>   #define TESTPAGEFLAG(uname, lname, policy)				\
> -static __always_inline bool folio_test_##lname(struct folio *folio)	\
> -{ return test_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); }	\
> +FOLIO_TEST_FLAG(lname, FOLIO_##policy)					\
>   static __always_inline int Page##uname(struct page *page)		\
>   { return test_bit(PG_##lname, &policy(page, 0)->flags); }
>   
>   #define SETPAGEFLAG(uname, lname, policy)				\
> -static __always_inline							\
> -void folio_set_##lname(struct folio *folio)				\
> -{ set_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); }		\
> +FOLIO_SET_FLAG(lname, FOLIO_##policy)					\
>   static __always_inline void SetPage##uname(struct page *page)		\
>   { set_bit(PG_##lname, &policy(page, 1)->flags); }
>   
>   #define CLEARPAGEFLAG(uname, lname, policy)				\
> -static __always_inline							\
> -void folio_clear_##lname(struct folio *folio)				\
> -{ clear_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); }		\
> +FOLIO_CLEAR_FLAG(lname, FOLIO_##policy)					\
>   static __always_inline void ClearPage##uname(struct page *page)		\
>   { clear_bit(PG_##lname, &policy(page, 1)->flags); }
>   
>   #define __SETPAGEFLAG(uname, lname, policy)				\
> -static __always_inline							\
> -void __folio_set_##lname(struct folio *folio)				\
> -{ __set_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); }		\
> +__FOLIO_SET_FLAG(lname, FOLIO_##policy)					\
>   static __always_inline void __SetPage##uname(struct page *page)		\
>   { __set_bit(PG_##lname, &policy(page, 1)->flags); }
>   
>   #define __CLEARPAGEFLAG(uname, lname, policy)				\
> -static __always_inline							\
> -void __folio_clear_##lname(struct folio *folio)				\
> -{ __clear_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); }	\
> +__FOLIO_CLEAR_FLAG(lname, FOLIO_##policy)				\
>   static __always_inline void __ClearPage##uname(struct page *page)	\
>   { __clear_bit(PG_##lname, &policy(page, 1)->flags); }
>   
>   #define TESTSETFLAG(uname, lname, policy)				\
> -static __always_inline							\
> -bool folio_test_set_##lname(struct folio *folio)			\
> -{ return test_and_set_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \
> +FOLIO_TEST_SET_FLAG(lname, FOLIO_##policy)				\
>   static __always_inline int TestSetPage##uname(struct page *page)	\
>   { return test_and_set_bit(PG_##lname, &policy(page, 1)->flags); }
>   
>   #define TESTCLEARFLAG(uname, lname, policy)				\
> -static __always_inline							\
> -bool folio_test_clear_##lname(struct folio *folio)			\
> -{ return test_and_clear_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \
> +FOLIO_TEST_CLEAR_FLAG(lname, FOLIO_##policy)				\
>   static __always_inline int TestClearPage##uname(struct page *page)	\
>   { return test_and_clear_bit(PG_##lname, &policy(page, 1)->flags); }
>   

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH 2/8] mm: Remove PageWaiters, PageSetWaiters and PageClearWaiters

[David Hildenbrand]

On 27.02.24 20:23, Matthew Wilcox (Oracle) wrote:
> All callers have been converted to use folios.  This was the only user
> of PF_ONLY_HEAD, so remove that too.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   include/linux/page-flags.h | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 95ab75d0b39c..d8f5127ae72e 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -328,9 +328,6 @@ static unsigned long *folio_flags(struct folio *folio, unsigned n)
>    *     for compound page all operations related to the page flag applied to
>    *     head page.
>    *
> - * PF_ONLY_HEAD:
> - *     for compound page, callers only ever operate on the head page.
> - *
>    * PF_NO_TAIL:
>    *     modifications of the page flag must be done on small or head pages,
>    *     checks can be done on tail pages too.
> @@ -346,9 +343,6 @@ static unsigned long *folio_flags(struct folio *folio, unsigned n)
>   		page; })
>   #define PF_ANY(page, enforce)	PF_POISONED_CHECK(page)
>   #define PF_HEAD(page, enforce)	PF_POISONED_CHECK(compound_head(page))
> -#define PF_ONLY_HEAD(page, enforce) ({					\
> -		VM_BUG_ON_PGFLAGS(PageTail(page), page);		\
> -		PF_POISONED_CHECK(page); })
>   #define PF_NO_TAIL(page, enforce) ({					\
>   		VM_BUG_ON_PGFLAGS(enforce && PageTail(page), page);	\
>   		PF_POISONED_CHECK(compound_head(page)); })
> @@ -362,7 +356,6 @@ static unsigned long *folio_flags(struct folio *folio, unsigned n)
>   /* Which page is the flag stored in */
>   #define FOLIO_PF_ANY		0
>   #define FOLIO_PF_HEAD		0
> -#define FOLIO_PF_ONLY_HEAD	0
>   #define FOLIO_PF_NO_TAIL	0
>   #define FOLIO_PF_NO_COMPOUND	0
>   #define FOLIO_PF_SECOND		1
> @@ -488,7 +481,7 @@ static inline int TestClearPage##uname(struct page *page) { return 0; }
>   	TESTSETFLAG_FALSE(uname, lname) TESTCLEARFLAG_FALSE(uname, lname)
>   
>   __PAGEFLAG(Locked, locked, PF_NO_TAIL)
> -PAGEFLAG(Waiters, waiters, PF_ONLY_HEAD)
> +FOLIO_FLAG(waiters, FOLIO_HEAD_PAGE)
>   PAGEFLAG(Error, error, PF_NO_TAIL) TESTCLEARFLAG(Error, error, PF_NO_TAIL)
>   PAGEFLAG(Referenced, referenced, PF_HEAD)
>   	TESTCLEARFLAG(Referenced, referenced, PF_HEAD)
> @@ -1138,7 +1131,6 @@ static inline bool folio_has_private(struct folio *folio)
>   
>   #undef PF_ANY
>   #undef PF_HEAD
> -#undef PF_ONLY_HEAD
>   #undef PF_NO_TAIL
>   #undef PF_NO_COMPOUND
>   #undef PF_SECOND

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH 3/8] mm: Remove PageYoung and PageIdle definitions

[David Hildenbrand]

On 27.02.24 20:23, Matthew Wilcox (Oracle) wrote:
> All callers have been converted to use folios, so remove the various
> set/clear/test functions defined on pages.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   include/linux/page-flags.h | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index d8f5127ae72e..582ca7400eca 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -599,10 +599,10 @@ PAGEFLAG_FALSE(HWPoison, hwpoison)
>   #endif
>   
>   #if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT)
> -TESTPAGEFLAG(Young, young, PF_ANY)
> -SETPAGEFLAG(Young, young, PF_ANY)
> -TESTCLEARFLAG(Young, young, PF_ANY)
> -PAGEFLAG(Idle, idle, PF_ANY)
> +FOLIO_TEST_FLAG(young, FOLIO_HEAD_PAGE)
> +FOLIO_SET_FLAG(young, FOLIO_HEAD_PAGE)
> +FOLIO_TEST_CLEAR_FLAG(young, FOLIO_HEAD_PAGE)
> +FOLIO_FLAG(idle, FOLIO_HEAD_PAGE)
>   #endif
>   
>   /*

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH 5/8] mm: Make dump_page() take a const argument

[David Hildenbrand]

On 27.02.24 20:23, Matthew Wilcox (Oracle) wrote:
> Now that __dump_page() takes a const argument, we can make dump_page()
> take a const struct page too.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   include/linux/mmdebug.h | 2 +-
>   mm/debug.c              | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
> index 7c3e7b0b0e8f..39a7714605a7 100644
> --- a/include/linux/mmdebug.h
> +++ b/include/linux/mmdebug.h
> @@ -10,7 +10,7 @@ struct vm_area_struct;
>   struct mm_struct;
>   struct vma_iterator;
>   
> -void dump_page(struct page *page, const char *reason);
> +void dump_page(const struct page *page, const char *reason);
>   void dump_vma(const struct vm_area_struct *vma);
>   void dump_mm(const struct mm_struct *mm);
>   void vma_iter_dump_tree(const struct vma_iterator *vmi);
> diff --git a/mm/debug.c b/mm/debug.c
> index 96555fc78f1a..6149944016a7 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -143,7 +143,7 @@ static void __dump_page(const struct page *page)
>   	__dump_folio(foliop, &precise, pfn, idx);
>   }
>   
> -void dump_page(struct page *page, const char *reason)
> +void dump_page(const struct page *page, const char *reason)
>   {
>   	if (PagePoisoned(page))
>   		pr_warn("page:%p is uninitialized and poisoned", page);

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH 8/8] mm: Remove cast from page_to_nid()

[David Hildenbrand]

On 27.02.24 20:23, Matthew Wilcox (Oracle) wrote:
> Now that PF_POISONED_CHECK() can take a const argument, we can drop
> the cast.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   include/linux/mm.h | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index c4a76520c967..21a8cfbe75aa 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1662,13 +1662,11 @@ static inline int page_zone_id(struct page *page)
>   }
>   
>   #ifdef NODE_NOT_IN_PAGE_FLAGS
> -extern int page_to_nid(const struct page *page);
> +int page_to_nid(const struct page *page);
>   #else
>   static inline int page_to_nid(const struct page *page)
>   {
> -	struct page *p = (struct page *)page;
> -
> -	return (PF_POISONED_CHECK(p)->flags >> NODES_PGSHIFT) & NODES_MASK;
> +	return (PF_POISONED_CHECK(page)->flags >> NODES_PGSHIFT) & NODES_MASK;
>   }
>   #endif
>   

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH 7/8] mm: Constify more page/folio tests

[David Hildenbrand]

On 27.02.24 20:23, Matthew Wilcox (Oracle) wrote:
> Constify the flag tests that aren't automatically generated and the
> tests that look like flag tests but are more complicated.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH 6/8] mm: Constify testing page/folio flags

[David Hildenbrand]

On 27.02.24 20:23, Matthew Wilcox (Oracle) wrote:
> Now that dump_page() takes a const argument, we can constify all the
> page flag tests.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH 4/8] mm: Add __dump_folio()

[Matthew Wilcox]

On Fri, Mar 01, 2024 at 10:21:10AM +0000, Ryan Roberts wrote:
> > +	page_cma = is_migrate_cma_page(page);
> 
> Problem is here: is_migrate_cma_page() is a macro that resolves to this:

Ah, yeah, maybe somebody should be testing with CONFIG_CMA enabled.
Ahem.

> page_cma = get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK) == MIGRATE_CMA;
> 
> And since page is on the stack, page_to_pfn() gives a very wrong answer.
> 
> I confirmed that the problem goes away for both cases above, when changing the line to:
> 
> page_cma = get_pfnblock_flags_mask(page, pfn, MIGRATETYPE_MASK) == MIGRATE_CMA;

Thanks!  I think what we end up wanting is ...

From f005189ad418d05d168c0ff00daecc2a444733ef Mon Sep 17 00:00:00 2001
From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Date: Fri, 1 Mar 2024 16:11:20 -0500
Subject: [PATCH] mm: Fix __dump_folio

Ryan Roberts reports that (if you have CONFIG_CMA enabled), calling
__dump_folio() will panic as we call is_migrate_cma_page() with a
stack copy of struct page, which gets passed to page_to_pfn().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/mmzone.h | 3 +++
 mm/debug.c             | 4 +---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 633812a1d220..c11b7cde81ef 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -76,9 +76,12 @@ extern const char * const migratetype_names[MIGRATE_TYPES];
 #ifdef CONFIG_CMA
 #  define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)
 #  define is_migrate_cma_page(_page) (get_pageblock_migratetype(_page) == MIGRATE_CMA)
+#  define is_migrate_cma_folio(folio, pfn)	(MIGRATE_CMA ==		\
+	get_pfnblock_flags_mask(&folio->page, pfn, MIGRATETYPE_MASK))
 #else
 #  define is_migrate_cma(migratetype) false
 #  define is_migrate_cma_page(_page) false
+#  define is_migrate_cma_folio(folio, pfn) false
 #endif
 
 static inline bool is_migrate_movable(int mt)
diff --git a/mm/debug.c b/mm/debug.c
index 32ac7d79fd04..e7aa8a9d5d86 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -55,7 +55,6 @@ static void __dump_folio(struct folio *folio, struct page *page,
 		unsigned long pfn, unsigned long idx)
 {
 	struct address_space *mapping = folio_mapping(folio);
-	bool page_cma;
 	int mapcount = 0;
 	char *type = "";
 
@@ -98,9 +97,8 @@ static void __dump_folio(struct folio *folio, struct page *page,
 	 * state for debugging, it should be fine to accept a bit of
 	 * inaccuracy here due to racing.
 	 */
-	page_cma = is_migrate_cma_page(page);
 	pr_warn("%sflags: %pGp%s\n", type, &folio->flags,
-		page_cma ? " CMA" : "");
+		is_migrate_cma_folio(folio, pfn) ? " CMA" : "");
 	pr_warn("page_type: %pGt\n", &folio->page.page_type);
 
 	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
-- 
2.43.0

Re: [PATCH 4/8] mm: Add __dump_folio()

[Matthew Wilcox]

Further testing revealed some more problems.  We were getting confused
between various different pointers leading to spurious messages about
the page not matching the folio and passing the wrong pointer to
__dump_folio().  Here's a fix-3 patch which I tested like so:

+static int __init page_dump(void)
+{
+       struct page *page;
+
+       printk("testing page dump\n");
+
+       page = alloc_page(GFP_KERNEL);
+       dump_page(page, "single");
+       put_page(page);
+       page = alloc_pages(GFP_KERNEL | __GFP_COMP, 2);
+       dump_page(page, "head");
+       dump_page(page + 1, "tail 1");
+       dump_page(page + 2, "tail 2");
+       dump_page(page + 3, "tail 3");
+       put_page(page);
+
+       return 0;
+}
+
+module_init(page_dump);

(needed some extra debug to check the values of the pointers being
passed to __dump_folio() which we wouldn't want to include)

---
 mm/debug.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/mm/debug.c b/mm/debug.c
index e7aa8a9d5d86..4dae73bc4530 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -121,23 +121,31 @@ static void __dump_page(const struct page *page)
 again:
 	memcpy(&precise, page, sizeof(*page));
 	foliop = page_folio(&precise);
-	idx = folio_page_idx(foliop, page);
-	if (idx != 0) {
-		if (idx < MAX_FOLIO_NR_PAGES) {
-			memcpy(&folio, foliop, 2 * sizeof(struct page));
-			nr_pages = folio_nr_pages(&folio);
-		}
+	if (foliop == (struct folio *)&precise) {
+		idx = 0;
+		if (!folio_test_large(foliop))
+			goto dump;
+		foliop = (struct folio *)page;
+	} else {
+		idx = folio_page_idx(foliop, page);
+	}
 
-		if (idx > nr_pages) {
-			if (loops-- > 0)
-				goto again;
-			printk("page does not match folio\n");
-			precise.compound_head &= ~1UL;
-			foliop = (struct folio *)&precise;
-			idx = 0;
-		}
+	if (idx < MAX_FOLIO_NR_PAGES) {
+		memcpy(&folio, foliop, 2 * sizeof(struct page));
+		nr_pages = folio_nr_pages(&folio);
+		foliop = &folio;
+	}
+
+	if (idx > nr_pages) {
+		if (loops-- > 0)
+			goto again;
+		printk("page does not match folio\n");
+		precise.compound_head &= ~1UL;
+		foliop = (struct folio *)&precise;
+		idx = 0;
 	}
 
+dump:
 	__dump_folio(foliop, &precise, pfn, idx);
 }
 
-- 
2.43.0

Re: [PATCH 4/8] mm: Add __dump_folio()

[Kees Cook]

Hi!

While working on testing an improved -Warray-bounds in GCC, I encountered
this, which seems to be reasonable:

In file included from ./arch/x86/include/generated/asm/rwonce.h:1,
                 from ../include/linux/compiler.h:299,
                 from ../include/linux/array_size.h:5,
                 from ../include/linux/kernel.h:16,
                 from ../mm/debug.c:9:
In function 'page_fixed_fake_head',
    inlined from '_compound_head' at ../include/linux/page-flags.h:251:24,
    inlined from '__dump_page' at ../mm/debug.c:123:11:
../include/asm-generic/rwonce.h:44:26: warning: array subscript 9 is outside array bounds of 'struct page[1]' [-Warray-bounds=]
   44 | #define __READ_ONCE(x)  (*(const volatile __unqual_scalar_typeof(x) *)&(x))
      |                         ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/asm-generic/rwonce.h:50:9: note: in expansion of macro '__READ_ONCE'
   50 |         __READ_ONCE(x);                                                 \
      |         ^~~~~~~~~~~
../include/linux/page-flags.h:226:38: note: in expansion of macro 'READ_ONCE'
  226 |                 unsigned long head = READ_ONCE(page[1].compound_head);
      |                                      ^~~~~~~~~
../mm/debug.c: In function '__dump_page':
../mm/debug.c:116:21: note: at offset 72 into object 'precise' of size 64
  116 |         struct page precise;
      |                     ^~~~~~~

(Not noted in this warning is that the code passes through page_folio()
_Generic macro.)

It doesn't like that it can see that "precise" is exactly one page, so
looking at page[1] later is going to freak out. I suspect this may be
"impossible" at run-time, but I'm not 100% sure. Regardless, the compiler
can't tell.

I suspect just making precise be a 2 page array would make this happy,
but it wasn't clear to me how such a page should be initialized.

-Kees

--
Kees Cook

Re: [PATCH 4/8] mm: Add __dump_folio()

[Matthew Wilcox]

On Mon, May 13, 2024 at 09:33:57PM -0700, Kees Cook wrote:
> Hi!
> 
> While working on testing an improved -Warray-bounds in GCC, I encountered
> this, which seems to be reasonable:

Eek.  I think you're right.  This is a bad interaction between the page
dumping code and the fixed fake head code.  I will need to think about
this (and LSFMM is happening right now, so I don't necessarily have a
lot of time to think).  I'll get back to you as soon as I can.

> In file included from ./arch/x86/include/generated/asm/rwonce.h:1,
>                  from ../include/linux/compiler.h:299,
>                  from ../include/linux/array_size.h:5,
>                  from ../include/linux/kernel.h:16,
>                  from ../mm/debug.c:9:
> In function 'page_fixed_fake_head',
>     inlined from '_compound_head' at ../include/linux/page-flags.h:251:24,
>     inlined from '__dump_page' at ../mm/debug.c:123:11:
> ../include/asm-generic/rwonce.h:44:26: warning: array subscript 9 is outside array bounds of 'struct page[1]' [-Warray-bounds=]
>    44 | #define __READ_ONCE(x)  (*(const volatile __unqual_scalar_typeof(x) *)&(x))
>       |                         ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../include/asm-generic/rwonce.h:50:9: note: in expansion of macro '__READ_ONCE'
>    50 |         __READ_ONCE(x);                                                 \
>       |         ^~~~~~~~~~~
> ../include/linux/page-flags.h:226:38: note: in expansion of macro 'READ_ONCE'
>   226 |                 unsigned long head = READ_ONCE(page[1].compound_head);
>       |                                      ^~~~~~~~~
> ../mm/debug.c: In function '__dump_page':
> ../mm/debug.c:116:21: note: at offset 72 into object 'precise' of size 64
>   116 |         struct page precise;
>       |                     ^~~~~~~
> 
> (Not noted in this warning is that the code passes through page_folio()
> _Generic macro.)
> 
> It doesn't like that it can see that "precise" is exactly one page, so
> looking at page[1] later is going to freak out. I suspect this may be
> "impossible" at run-time, but I'm not 100% sure. Regardless, the compiler
> can't tell.
> 
> I suspect just making precise be a 2 page array would make this happy,
> but it wasn't clear to me how such a page should be initialized.
> 
> -Kees
> 
> --
> Kees Cook

Re: [PATCH 4/8] mm: Add __dump_folio()

[Matthew Wilcox]

On Mon, May 13, 2024 at 09:33:57PM -0700, Kees Cook wrote:
> In function 'page_fixed_fake_head',
>     inlined from '_compound_head' at ../include/linux/page-flags.h:251:24,
>     inlined from '__dump_page' at ../mm/debug.c:123:11:
> ../include/asm-generic/rwonce.h:44:26: warning: array subscript 9 is outside array bounds of 'struct page[1]' [-Warray-bounds=]
> 
> (Not noted in this warning is that the code passes through page_folio()
> _Generic macro.)
> 
> It doesn't like that it can see that "precise" is exactly one page, so
> looking at page[1] later is going to freak out. I suspect this may be
> "impossible" at run-time, but I'm not 100% sure. Regardless, the compiler
> can't tell.

Actually, I'm not sure that I can tell that it's impossible.

I think we just need to open-code page_folio() here so that we don't
get into the fixed fake head palaver.  Something like this, although
it's only compile-tested.


diff --git a/mm/debug.c b/mm/debug.c
index e3ff3ac19fa1..47ba8b0a4872 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -110,19 +110,22 @@ static void __dump_page(const struct page *page)
 {
 	struct folio *foliop, folio;
 	struct page precise;
+	unsigned long head;
 	unsigned long pfn = page_to_pfn(page);
 	unsigned long idx, nr_pages = 1;
 	int loops = 5;
 
 again:
 	memcpy(&precise, page, sizeof(*page));
-	foliop = page_folio(&precise);
-	if (foliop == (struct folio *)&precise) {
+	head = precise.compound_head;
+	if ((head & 1) == 0) {
+		foliop = (struct folio *)&precise;
 		idx = 0;
 		if (!folio_test_large(foliop))
 			goto dump;
 		foliop = (struct folio *)page;
 	} else {
+		foliop = (struct folio *)(head - 1);
 		idx = folio_page_idx(foliop, page);
 	}