[RESEND][PATCH 0/6] Constify struct page arguments

[RESEND][PATCH 0/6] Constify struct page arguments

[Matthew Wilcox (Oracle)]

[I'm told that patches 2-6 did not make it to the list; resending and
cc'ing lkml this time]

While working on various solutions to the 32-bit struct page size
regression, one of the problems I found was the networking stack expects
to be able to pass const struct page pointers around, and the mm doesn't
provide a lot of const-friendly functions to call.  The root tangle of
problems is that a lot of functions call VM_BUG_ON_PAGE(), which calls
dump_page(), which calls a lot of functions which don't take a const
struct page (but could be const).

I have other things I need to work on, but I offer these patches as a few
steps towards being able to make dump_page() take a const page pointer.

Matthew Wilcox (Oracle) (6):
  mm: Make __dump_page static
  mm/debug: Factor PagePoisoned out of __dump_page
  mm/page_owner: Constify dump_page_owner
  mm: Make compound_head const-preserving
  mm: Constify get_pfnblock_flags_mask and get_pfnblock_migratetype
  mm: Constify page_count and page_ref_count

 include/linux/mmdebug.h         |  3 +--
 include/linux/page-flags.h      | 10 +++++-----
 include/linux/page_owner.h      |  6 +++---
 include/linux/page_ref.h        |  4 ++--
 include/linux/pageblock-flags.h |  2 +-
 mm/debug.c                      | 25 +++++++------------------
 mm/page_alloc.c                 | 16 ++++++++--------
 mm/page_owner.c                 |  2 +-
 8 files changed, 28 insertions(+), 40 deletions(-)

-- 
2.30.2

[PATCH 1/6] mm: Make __dump_page static

[Matthew Wilcox (Oracle)]

The only caller of __dump_page() now opencodes dump_page(), so
remove it as an externally visible symbol.

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

diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
index 5d0767cb424a..1935d4c72d10 100644
--- a/include/linux/mmdebug.h
+++ b/include/linux/mmdebug.h
@@ -9,8 +9,7 @@ struct page;
 struct vm_area_struct;
 struct mm_struct;
 
-extern void dump_page(struct page *page, const char *reason);
-extern void __dump_page(struct page *page, const char *reason);
+void dump_page(struct page *page, const char *reason);
 void dump_vma(const struct vm_area_struct *vma);
 void dump_mm(const struct mm_struct *mm);
 
diff --git a/mm/debug.c b/mm/debug.c
index 0bdda8407f71..84cdcd0f7bd3 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -42,7 +42,7 @@ const struct trace_print_flags vmaflag_names[] = {
 	{0, NULL}
 };
 
-void __dump_page(struct page *page, const char *reason)
+static void __dump_page(struct page *page, const char *reason)
 {
 	struct page *head = compound_head(page);
 	struct address_space *mapping;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5a35f21b57c6..0152670c6f04 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -658,8 +658,7 @@ 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);
-	dump_page_owner(page);
+	dump_page(page, reason);
 
 	print_modules();
 	dump_stack();
-- 
2.30.2

[PATCH 2/6] mm/debug: Factor PagePoisoned out of __dump_page

[Matthew Wilcox (Oracle)]

Move the PagePoisoned test into dump_page().  Skip the hex print
for poisoned pages -- we know they're full of ffffffff.  Move the
reason printing from __dump_page() to dump_page().

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

diff --git a/mm/debug.c b/mm/debug.c
index 84cdcd0f7bd3..e73fe0a8ec3d 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -42,11 +42,10 @@ const struct trace_print_flags vmaflag_names[] = {
 	{0, NULL}
 };
 
-static void __dump_page(struct page *page, const char *reason)
+static void __dump_page(struct page *page)
 {
 	struct page *head = compound_head(page);
 	struct address_space *mapping;
-	bool page_poisoned = PagePoisoned(page);
 	bool compound = PageCompound(page);
 	/*
 	 * Accessing the pageblock without the zone lock. It could change to
@@ -58,16 +57,6 @@ static void __dump_page(struct page *page, const char *reason)
 	int mapcount;
 	char *type = "";
 
-	/*
-	 * If struct page is poisoned don't access Page*() functions as that
-	 * leads to recursive loop. Page*() check for poisoned pages, and calls
-	 * dump_page() when detected.
-	 */
-	if (page_poisoned) {
-		pr_warn("page:%px is uninitialized and poisoned", page);
-		goto hex_only;
-	}
-
 	if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) {
 		/*
 		 * Corrupt page, so we cannot call page_mapping. Instead, do a
@@ -173,8 +162,6 @@ static void __dump_page(struct page *page, const char *reason)
 
 	pr_warn("%sflags: %#lx(%pGp)%s\n", type, head->flags, &head->flags,
 		page_cma ? " CMA" : "");
-
-hex_only:
 	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
 			sizeof(unsigned long), page,
 			sizeof(struct page), false);
@@ -182,14 +169,16 @@ static void __dump_page(struct page *page, const char *reason)
 		print_hex_dump(KERN_WARNING, "head: ", DUMP_PREFIX_NONE, 32,
 			sizeof(unsigned long), head,
 			sizeof(struct page), false);
-
-	if (reason)
-		pr_warn("page dumped because: %s\n", reason);
 }
 
 void dump_page(struct page *page, const char *reason)
 {
-	__dump_page(page, reason);
+	if (PagePoisoned(page))
+		pr_warn("page:%p is uninitialized and poisoned", page);
+	else
+		__dump_page(page);
+	if (reason)
+		pr_warn("page dumped because: %s\n", reason);
 	dump_page_owner(page);
 }
 EXPORT_SYMBOL(dump_page);
-- 
2.30.2

[PATCH 3/6] mm/page_owner: Constify dump_page_owner

[Matthew Wilcox (Oracle)]

dump_page_owner() only uses struct page to find the page_ext, and
lookup_page_ext() already takes a const argument.

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

diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h
index 3468794f83d2..719bfe5108c5 100644
--- a/include/linux/page_owner.h
+++ b/include/linux/page_owner.h
@@ -14,7 +14,7 @@ extern void __set_page_owner(struct page *page,
 extern void __split_page_owner(struct page *page, unsigned int nr);
 extern void __copy_page_owner(struct page *oldpage, struct page *newpage);
 extern void __set_page_owner_migrate_reason(struct page *page, int reason);
-extern void __dump_page_owner(struct page *page);
+extern void __dump_page_owner(const struct page *page);
 extern void pagetypeinfo_showmixedcount_print(struct seq_file *m,
 					pg_data_t *pgdat, struct zone *zone);
 
@@ -46,7 +46,7 @@ static inline void set_page_owner_migrate_reason(struct page *page, int reason)
 	if (static_branch_unlikely(&page_owner_inited))
 		__set_page_owner_migrate_reason(page, reason);
 }
-static inline void dump_page_owner(struct page *page)
+static inline void dump_page_owner(const struct page *page)
 {
 	if (static_branch_unlikely(&page_owner_inited))
 		__dump_page_owner(page);
@@ -69,7 +69,7 @@ static inline void copy_page_owner(struct page *oldpage, struct page *newpage)
 static inline void set_page_owner_migrate_reason(struct page *page, int reason)
 {
 }
-static inline void dump_page_owner(struct page *page)
+static inline void dump_page_owner(const struct page *page)
 {
 }
 #endif /* CONFIG_PAGE_OWNER */
diff --git a/mm/page_owner.c b/mm/page_owner.c
index adfabb560eb9..f51a57e92aa3 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -392,7 +392,7 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
 	return -ENOMEM;
 }
 
-void __dump_page_owner(struct page *page)
+void __dump_page_owner(const struct page *page)
 {
 	struct page_ext *page_ext = lookup_page_ext(page);
 	struct page_owner *page_owner;
-- 
2.30.2

[PATCH 4/6] mm: Make compound_head const-preserving

[Matthew Wilcox (Oracle)]

If you pass a const pointer to compound_head(), you get a const pointer
back; if you pass a mutable pointer, you get a mutable pointer back.
Also remove an unnecessary forward definition of struct page; we're about
to dereference page->compound_head, so it must already have been defined.

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

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 04a34c08e0a6..d8e26243db25 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -177,17 +177,17 @@ enum pageflags {
 
 #ifndef __GENERATING_BOUNDS_H
 
-struct page;	/* forward declaration */
-
-static inline struct page *compound_head(struct page *page)
+static inline unsigned long _compound_head(const struct page *page)
 {
 	unsigned long head = READ_ONCE(page->compound_head);
 
 	if (unlikely(head & 1))
-		return (struct page *) (head - 1);
-	return page;
+		return head - 1;
+	return (unsigned long)page;
 }
 
+#define compound_head(page)	((typeof(page))_compound_head(page))
+
 static __always_inline int PageTail(struct page *page)
 {
 	return READ_ONCE(page->compound_head) & 1;
-- 
2.30.2

[PATCH 5/6] mm: Constify get_pfnblock_flags_mask and get_pfnblock_migratetype

[Matthew Wilcox (Oracle)]

The struct page is not modified by these routines, so it can be marked
const.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/pageblock-flags.h |  2 +-
 mm/page_alloc.c                 | 13 +++++++------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
index fff52ad370c1..973fd731a520 100644
--- a/include/linux/pageblock-flags.h
+++ b/include/linux/pageblock-flags.h
@@ -54,7 +54,7 @@ extern unsigned int pageblock_order;
 /* Forward declaration */
 struct page;
 
-unsigned long get_pfnblock_flags_mask(struct page *page,
+unsigned long get_pfnblock_flags_mask(const struct page *page,
 				unsigned long pfn,
 				unsigned long mask);
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0152670c6f04..4be2179eedd5 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -474,7 +474,7 @@ static inline bool defer_init(int nid, unsigned long pfn, unsigned long end_pfn)
 #endif
 
 /* Return a pointer to the bitmap storing bits affecting a block of pages */
-static inline unsigned long *get_pageblock_bitmap(struct page *page,
+static inline unsigned long *get_pageblock_bitmap(const struct page *page,
 							unsigned long pfn)
 {
 #ifdef CONFIG_SPARSEMEM
@@ -484,7 +484,7 @@ static inline unsigned long *get_pageblock_bitmap(struct page *page,
 #endif /* CONFIG_SPARSEMEM */
 }
 
-static inline int pfn_to_bitidx(struct page *page, unsigned long pfn)
+static inline int pfn_to_bitidx(const struct page *page, unsigned long pfn)
 {
 #ifdef CONFIG_SPARSEMEM
 	pfn &= (PAGES_PER_SECTION-1);
@@ -495,7 +495,7 @@ static inline int pfn_to_bitidx(struct page *page, unsigned long pfn)
 }
 
 static __always_inline
-unsigned long __get_pfnblock_flags_mask(struct page *page,
+unsigned long __get_pfnblock_flags_mask(const struct page *page,
 					unsigned long pfn,
 					unsigned long mask)
 {
@@ -520,13 +520,14 @@ unsigned long __get_pfnblock_flags_mask(struct page *page,
  *
  * Return: pageblock_bits flags
  */
-unsigned long get_pfnblock_flags_mask(struct page *page, unsigned long pfn,
-					unsigned long mask)
+unsigned long get_pfnblock_flags_mask(const struct page *page,
+					unsigned long pfn, unsigned long mask)
 {
 	return __get_pfnblock_flags_mask(page, pfn, mask);
 }
 
-static __always_inline int get_pfnblock_migratetype(struct page *page, unsigned long pfn)
+static __always_inline int get_pfnblock_migratetype(const struct page *page,
+					unsigned long pfn)
 {
 	return __get_pfnblock_flags_mask(page, pfn, MIGRATETYPE_MASK);
 }
-- 
2.30.2

[PATCH 6/6] mm: Constify page_count and page_ref_count

[Matthew Wilcox (Oracle)]

Now that compound_head() accepts a const struct page pointer, these two
functions can be marked as not modifying the page pointer they are passed.

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

diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
index f3318f34fc54..7ad46f45df39 100644
--- a/include/linux/page_ref.h
+++ b/include/linux/page_ref.h
@@ -62,12 +62,12 @@ static inline void __page_ref_unfreeze(struct page *page, int v)
 
 #endif
 
-static inline int page_ref_count(struct page *page)
+static inline int page_ref_count(const struct page *page)
 {
 	return atomic_read(&page->_refcount);
 }
 
-static inline int page_count(struct page *page)
+static inline int page_count(const struct page *page)
 {
 	return atomic_read(&compound_head(page)->_refcount);
 }
-- 
2.30.2

Re: [RESEND][PATCH 0/6] Constify struct page arguments

[William Kucharski]

Looks good to me and I like the cleanup.

For the series:

Reviewed-by: William Kucharski <william.kucharski@oracle.com>

> On Apr 16, 2021, at 5:15 PM, Matthew Wilcox (Oracle) <willy@infradead.org> wrote:
> 
> [I'm told that patches 2-6 did not make it to the list; resending and
> cc'ing lkml this time]
> 
> While working on various solutions to the 32-bit struct page size
> regression, one of the problems I found was the networking stack expects
> to be able to pass const struct page pointers around, and the mm doesn't
> provide a lot of const-friendly functions to call.  The root tangle of
> problems is that a lot of functions call VM_BUG_ON_PAGE(), which calls
> dump_page(), which calls a lot of functions which don't take a const
> struct page (but could be const).
> 
> I have other things I need to work on, but I offer these patches as a few
> steps towards being able to make dump_page() take a const page pointer.
> 
> Matthew Wilcox (Oracle) (6):
>  mm: Make __dump_page static
>  mm/debug: Factor PagePoisoned out of __dump_page
>  mm/page_owner: Constify dump_page_owner
>  mm: Make compound_head const-preserving
>  mm: Constify get_pfnblock_flags_mask and get_pfnblock_migratetype
>  mm: Constify page_count and page_ref_count
> 
> include/linux/mmdebug.h         |  3 +--
> include/linux/page-flags.h      | 10 +++++-----
> include/linux/page_owner.h      |  6 +++---
> include/linux/page_ref.h        |  4 ++--
> include/linux/pageblock-flags.h |  2 +-
> mm/debug.c                      | 25 +++++++------------------
> mm/page_alloc.c                 | 16 ++++++++--------
> mm/page_owner.c                 |  2 +-
> 8 files changed, 28 insertions(+), 40 deletions(-)
> 
> -- 
> 2.30.2
> 
> 

Re: [PATCH 1/6] mm: Make __dump_page static

[Vlastimil Babka]

On 4/17/21 1:15 AM, Matthew Wilcox (Oracle) wrote:
> The only caller of __dump_page() now opencodes dump_page(), so
> remove it as an externally visible symbol.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  include/linux/mmdebug.h | 3 +--
>  mm/debug.c              | 2 +-
>  mm/page_alloc.c         | 3 +--
>  3 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
> index 5d0767cb424a..1935d4c72d10 100644
> --- a/include/linux/mmdebug.h
> +++ b/include/linux/mmdebug.h
> @@ -9,8 +9,7 @@ struct page;
>  struct vm_area_struct;
>  struct mm_struct;
>  
> -extern void dump_page(struct page *page, const char *reason);
> -extern void __dump_page(struct page *page, const char *reason);
> +void dump_page(struct page *page, const char *reason);
>  void dump_vma(const struct vm_area_struct *vma);
>  void dump_mm(const struct mm_struct *mm);
>  
> diff --git a/mm/debug.c b/mm/debug.c
> index 0bdda8407f71..84cdcd0f7bd3 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -42,7 +42,7 @@ const struct trace_print_flags vmaflag_names[] = {
>  	{0, NULL}
>  };
>  
> -void __dump_page(struct page *page, const char *reason)
> +static void __dump_page(struct page *page, const char *reason)
>  {
>  	struct page *head = compound_head(page);
>  	struct address_space *mapping;
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5a35f21b57c6..0152670c6f04 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -658,8 +658,7 @@ 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);
> -	dump_page_owner(page);
> +	dump_page(page, reason);
>  
>  	print_modules();
>  	dump_stack();
> 

Re: [PATCH 2/6] mm/debug: Factor PagePoisoned out of __dump_page

[Vlastimil Babka]

On 4/17/21 1:15 AM, Matthew Wilcox (Oracle) wrote:
> Move the PagePoisoned test into dump_page().  Skip the hex print
> for poisoned pages -- we know they're full of ffffffff.  Move the
> reason printing from __dump_page() to dump_page().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

Re: [PATCH 3/6] mm/page_owner: Constify dump_page_owner

[Vlastimil Babka]

On 4/17/21 1:15 AM, Matthew Wilcox (Oracle) wrote:
> dump_page_owner() only uses struct page to find the page_ext, and
> lookup_page_ext() already takes a const argument.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  include/linux/page_owner.h | 6 +++---
>  mm/page_owner.c            | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h
> index 3468794f83d2..719bfe5108c5 100644
> --- a/include/linux/page_owner.h
> +++ b/include/linux/page_owner.h
> @@ -14,7 +14,7 @@ extern void __set_page_owner(struct page *page,
>  extern void __split_page_owner(struct page *page, unsigned int nr);
>  extern void __copy_page_owner(struct page *oldpage, struct page *newpage);
>  extern void __set_page_owner_migrate_reason(struct page *page, int reason);
> -extern void __dump_page_owner(struct page *page);
> +extern void __dump_page_owner(const struct page *page);
>  extern void pagetypeinfo_showmixedcount_print(struct seq_file *m,
>  					pg_data_t *pgdat, struct zone *zone);
>  
> @@ -46,7 +46,7 @@ static inline void set_page_owner_migrate_reason(struct page *page, int reason)
>  	if (static_branch_unlikely(&page_owner_inited))
>  		__set_page_owner_migrate_reason(page, reason);
>  }
> -static inline void dump_page_owner(struct page *page)
> +static inline void dump_page_owner(const struct page *page)
>  {
>  	if (static_branch_unlikely(&page_owner_inited))
>  		__dump_page_owner(page);
> @@ -69,7 +69,7 @@ static inline void copy_page_owner(struct page *oldpage, struct page *newpage)
>  static inline void set_page_owner_migrate_reason(struct page *page, int reason)
>  {
>  }
> -static inline void dump_page_owner(struct page *page)
> +static inline void dump_page_owner(const struct page *page)
>  {
>  }
>  #endif /* CONFIG_PAGE_OWNER */
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index adfabb560eb9..f51a57e92aa3 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -392,7 +392,7 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
>  	return -ENOMEM;
>  }
>  
> -void __dump_page_owner(struct page *page)
> +void __dump_page_owner(const struct page *page)
>  {
>  	struct page_ext *page_ext = lookup_page_ext(page);
>  	struct page_owner *page_owner;
> 

Re: [PATCH 4/6] mm: Make compound_head const-preserving

[Vlastimil Babka]

On 4/17/21 1:15 AM, Matthew Wilcox (Oracle) wrote:
> If you pass a const pointer to compound_head(), you get a const pointer
> back; if you pass a mutable pointer, you get a mutable pointer back.

Hmm, nice trick.

> Also remove an unnecessary forward definition of struct page; we're about
> to dereference page->compound_head, so it must already have been defined.

Right.

> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  include/linux/page-flags.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 04a34c08e0a6..d8e26243db25 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -177,17 +177,17 @@ enum pageflags {
>  
>  #ifndef __GENERATING_BOUNDS_H
>  
> -struct page;	/* forward declaration */
> -
> -static inline struct page *compound_head(struct page *page)
> +static inline unsigned long _compound_head(const struct page *page)
>  {
>  	unsigned long head = READ_ONCE(page->compound_head);
>  
>  	if (unlikely(head & 1))
> -		return (struct page *) (head - 1);
> -	return page;
> +		return head - 1;
> +	return (unsigned long)page;
>  }
>  
> +#define compound_head(page)	((typeof(page))_compound_head(page))
> +
>  static __always_inline int PageTail(struct page *page)
>  {
>  	return READ_ONCE(page->compound_head) & 1;
> 

Re: [PATCH 5/6] mm: Constify get_pfnblock_flags_mask and get_pfnblock_migratetype

[Vlastimil Babka]

On 4/17/21 1:15 AM, Matthew Wilcox (Oracle) wrote:
> The struct page is not modified by these routines, so it can be marked
> const.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  include/linux/pageblock-flags.h |  2 +-
>  mm/page_alloc.c                 | 13 +++++++------
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
> index fff52ad370c1..973fd731a520 100644
> --- a/include/linux/pageblock-flags.h
> +++ b/include/linux/pageblock-flags.h
> @@ -54,7 +54,7 @@ extern unsigned int pageblock_order;
>  /* Forward declaration */
>  struct page;
>  
> -unsigned long get_pfnblock_flags_mask(struct page *page,
> +unsigned long get_pfnblock_flags_mask(const struct page *page,
>  				unsigned long pfn,
>  				unsigned long mask);
>  
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0152670c6f04..4be2179eedd5 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -474,7 +474,7 @@ static inline bool defer_init(int nid, unsigned long pfn, unsigned long end_pfn)
>  #endif
>  
>  /* Return a pointer to the bitmap storing bits affecting a block of pages */
> -static inline unsigned long *get_pageblock_bitmap(struct page *page,
> +static inline unsigned long *get_pageblock_bitmap(const struct page *page,
>  							unsigned long pfn)
>  {
>  #ifdef CONFIG_SPARSEMEM
> @@ -484,7 +484,7 @@ static inline unsigned long *get_pageblock_bitmap(struct page *page,
>  #endif /* CONFIG_SPARSEMEM */
>  }
>  
> -static inline int pfn_to_bitidx(struct page *page, unsigned long pfn)
> +static inline int pfn_to_bitidx(const struct page *page, unsigned long pfn)
>  {
>  #ifdef CONFIG_SPARSEMEM
>  	pfn &= (PAGES_PER_SECTION-1);
> @@ -495,7 +495,7 @@ static inline int pfn_to_bitidx(struct page *page, unsigned long pfn)
>  }
>  
>  static __always_inline
> -unsigned long __get_pfnblock_flags_mask(struct page *page,
> +unsigned long __get_pfnblock_flags_mask(const struct page *page,
>  					unsigned long pfn,
>  					unsigned long mask)
>  {
> @@ -520,13 +520,14 @@ unsigned long __get_pfnblock_flags_mask(struct page *page,
>   *
>   * Return: pageblock_bits flags
>   */
> -unsigned long get_pfnblock_flags_mask(struct page *page, unsigned long pfn,
> -					unsigned long mask)
> +unsigned long get_pfnblock_flags_mask(const struct page *page,
> +					unsigned long pfn, unsigned long mask)
>  {
>  	return __get_pfnblock_flags_mask(page, pfn, mask);
>  }
>  
> -static __always_inline int get_pfnblock_migratetype(struct page *page, unsigned long pfn)
> +static __always_inline int get_pfnblock_migratetype(const struct page *page,
> +					unsigned long pfn)
>  {
>  	return __get_pfnblock_flags_mask(page, pfn, MIGRATETYPE_MASK);
>  }
> 

Re: [PATCH 6/6] mm: Constify page_count and page_ref_count

[Vlastimil Babka]

On 4/17/21 1:15 AM, Matthew Wilcox (Oracle) wrote:
> Now that compound_head() accepts a const struct page pointer, these two
> functions can be marked as not modifying the page pointer they are passed.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  include/linux/page_ref.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
> index f3318f34fc54..7ad46f45df39 100644
> --- a/include/linux/page_ref.h
> +++ b/include/linux/page_ref.h
> @@ -62,12 +62,12 @@ static inline void __page_ref_unfreeze(struct page *page, int v)
>  
>  #endif
>  
> -static inline int page_ref_count(struct page *page)
> +static inline int page_ref_count(const struct page *page)
>  {
>  	return atomic_read(&page->_refcount);
>  }
>  
> -static inline int page_count(struct page *page)
> +static inline int page_count(const struct page *page)
>  {
>  	return atomic_read(&compound_head(page)->_refcount);
>  }
> 

Re: [PATCH 1/6] mm: Make __dump_page static

[Anshuman Khandual]

On 4/17/21 4:45 AM, Matthew Wilcox (Oracle) wrote:
> The only caller of __dump_page() now opencodes dump_page(), so
> remove it as an externally visible symbol.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  include/linux/mmdebug.h | 3 +--
>  mm/debug.c              | 2 +-
>  mm/page_alloc.c         | 3 +--
>  3 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h
> index 5d0767cb424a..1935d4c72d10 100644
> --- a/include/linux/mmdebug.h
> +++ b/include/linux/mmdebug.h
> @@ -9,8 +9,7 @@ struct page;
>  struct vm_area_struct;
>  struct mm_struct;
>  
> -extern void dump_page(struct page *page, const char *reason);
> -extern void __dump_page(struct page *page, const char *reason);
> +void dump_page(struct page *page, const char *reason);
>  void dump_vma(const struct vm_area_struct *vma);
>  void dump_mm(const struct mm_struct *mm);
>  
> diff --git a/mm/debug.c b/mm/debug.c
> index 0bdda8407f71..84cdcd0f7bd3 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -42,7 +42,7 @@ const struct trace_print_flags vmaflag_names[] = {
>  	{0, NULL}
>  };
>  
> -void __dump_page(struct page *page, const char *reason)
> +static void __dump_page(struct page *page, const char *reason)
>  {
>  	struct page *head = compound_head(page);
>  	struct address_space *mapping;
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5a35f21b57c6..0152670c6f04 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -658,8 +658,7 @@ 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);
> -	dump_page_owner(page);
> +	dump_page(page, reason);
>  
>  	print_modules();
>  	dump_stack();
> 

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

Re: [PATCH 2/6] mm/debug: Factor PagePoisoned out of __dump_page

[Anshuman Khandual]

On 4/17/21 4:45 AM, Matthew Wilcox (Oracle) wrote:
> Move the PagePoisoned test into dump_page().  Skip the hex print
> for poisoned pages -- we know they're full of ffffffff.  Move the
> reason printing from __dump_page() to dump_page().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/debug.c | 25 +++++++------------------
>  1 file changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/mm/debug.c b/mm/debug.c
> index 84cdcd0f7bd3..e73fe0a8ec3d 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -42,11 +42,10 @@ const struct trace_print_flags vmaflag_names[] = {
>  	{0, NULL}
>  };
>  
> -static void __dump_page(struct page *page, const char *reason)
> +static void __dump_page(struct page *page)
>  {
>  	struct page *head = compound_head(page);
>  	struct address_space *mapping;
> -	bool page_poisoned = PagePoisoned(page);
>  	bool compound = PageCompound(page);
>  	/*
>  	 * Accessing the pageblock without the zone lock. It could change to
> @@ -58,16 +57,6 @@ static void __dump_page(struct page *page, const char *reason)
>  	int mapcount;
>  	char *type = "";
>  
> -	/*
> -	 * If struct page is poisoned don't access Page*() functions as that
> -	 * leads to recursive loop. Page*() check for poisoned pages, and calls
> -	 * dump_page() when detected.
> -	 */
> -	if (page_poisoned) {
> -		pr_warn("page:%px is uninitialized and poisoned", page);
> -		goto hex_only;
> -	}
> -
>  	if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) {
>  		/*
>  		 * Corrupt page, so we cannot call page_mapping. Instead, do a
> @@ -173,8 +162,6 @@ static void __dump_page(struct page *page, const char *reason)
>  
>  	pr_warn("%sflags: %#lx(%pGp)%s\n", type, head->flags, &head->flags,
>  		page_cma ? " CMA" : "");
> -
> -hex_only:
>  	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
>  			sizeof(unsigned long), page,
>  			sizeof(struct page), false);
> @@ -182,14 +169,16 @@ static void __dump_page(struct page *page, const char *reason)
>  		print_hex_dump(KERN_WARNING, "head: ", DUMP_PREFIX_NONE, 32,
>  			sizeof(unsigned long), head,
>  			sizeof(struct page), false);
> -
> -	if (reason)
> -		pr_warn("page dumped because: %s\n", reason);
>  }
>  
>  void dump_page(struct page *page, const char *reason)
>  {
> -	__dump_page(page, reason);
> +	if (PagePoisoned(page))
> +		pr_warn("page:%p is uninitialized and poisoned", page);
> +	else
> +		__dump_page(page);
> +	if (reason)
> +		pr_warn("page dumped because: %s\n", reason);
>  	dump_page_owner(page);
>  }
>  EXPORT_SYMBOL(dump_page);
> 

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

Re: [PATCH 3/6] mm/page_owner: Constify dump_page_owner

[Anshuman Khandual]

On 4/17/21 4:45 AM, Matthew Wilcox (Oracle) wrote:
> dump_page_owner() only uses struct page to find the page_ext, and
> lookup_page_ext() already takes a const argument.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  include/linux/page_owner.h | 6 +++---
>  mm/page_owner.c            | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h
> index 3468794f83d2..719bfe5108c5 100644
> --- a/include/linux/page_owner.h
> +++ b/include/linux/page_owner.h
> @@ -14,7 +14,7 @@ extern void __set_page_owner(struct page *page,
>  extern void __split_page_owner(struct page *page, unsigned int nr);
>  extern void __copy_page_owner(struct page *oldpage, struct page *newpage);
>  extern void __set_page_owner_migrate_reason(struct page *page, int reason);
> -extern void __dump_page_owner(struct page *page);
> +extern void __dump_page_owner(const struct page *page);
>  extern void pagetypeinfo_showmixedcount_print(struct seq_file *m,
>  					pg_data_t *pgdat, struct zone *zone);
>  
> @@ -46,7 +46,7 @@ static inline void set_page_owner_migrate_reason(struct page *page, int reason)
>  	if (static_branch_unlikely(&page_owner_inited))
>  		__set_page_owner_migrate_reason(page, reason);
>  }
> -static inline void dump_page_owner(struct page *page)
> +static inline void dump_page_owner(const struct page *page)
>  {
>  	if (static_branch_unlikely(&page_owner_inited))
>  		__dump_page_owner(page);
> @@ -69,7 +69,7 @@ static inline void copy_page_owner(struct page *oldpage, struct page *newpage)
>  static inline void set_page_owner_migrate_reason(struct page *page, int reason)
>  {
>  }
> -static inline void dump_page_owner(struct page *page)
> +static inline void dump_page_owner(const struct page *page)
>  {
>  }
>  #endif /* CONFIG_PAGE_OWNER */
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index adfabb560eb9..f51a57e92aa3 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -392,7 +392,7 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
>  	return -ENOMEM;
>  }
>  
> -void __dump_page_owner(struct page *page)
> +void __dump_page_owner(const struct page *page)
>  {
>  	struct page_ext *page_ext = lookup_page_ext(page);
>  	struct page_owner *page_owner;
> 

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

Re: [PATCH 4/6] mm: Make compound_head const-preserving

[Anshuman Khandual]

On 4/17/21 4:45 AM, Matthew Wilcox (Oracle) wrote:
> If you pass a const pointer to compound_head(), you get a const pointer
> back; if you pass a mutable pointer, you get a mutable pointer back.
> Also remove an unnecessary forward definition of struct page; we're about
> to dereference page->compound_head, so it must already have been defined.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  include/linux/page-flags.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 04a34c08e0a6..d8e26243db25 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -177,17 +177,17 @@ enum pageflags {
>  
>  #ifndef __GENERATING_BOUNDS_H
>  
> -struct page;	/* forward declaration */
> -
> -static inline struct page *compound_head(struct page *page)
> +static inline unsigned long _compound_head(const struct page *page)
>  {
>  	unsigned long head = READ_ONCE(page->compound_head);
>  
>  	if (unlikely(head & 1))
> -		return (struct page *) (head - 1);
> -	return page;
> +		return head - 1;
> +	return (unsigned long)page;
>  }
>  
> +#define compound_head(page)	((typeof(page))_compound_head(page))
> +
>  static __always_inline int PageTail(struct page *page)
>  {
>  	return READ_ONCE(page->compound_head) & 1;
> 

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

Re: [PATCH 5/6] mm: Constify get_pfnblock_flags_mask and get_pfnblock_migratetype

[Anshuman Khandual]

On 4/17/21 4:45 AM, Matthew Wilcox (Oracle) wrote:
> The struct page is not modified by these routines, so it can be marked
> const.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  include/linux/pageblock-flags.h |  2 +-
>  mm/page_alloc.c                 | 13 +++++++------
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
> index fff52ad370c1..973fd731a520 100644
> --- a/include/linux/pageblock-flags.h
> +++ b/include/linux/pageblock-flags.h
> @@ -54,7 +54,7 @@ extern unsigned int pageblock_order;
>  /* Forward declaration */
>  struct page;
>  
> -unsigned long get_pfnblock_flags_mask(struct page *page,
> +unsigned long get_pfnblock_flags_mask(const struct page *page,
>  				unsigned long pfn,
>  				unsigned long mask);
>  
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0152670c6f04..4be2179eedd5 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -474,7 +474,7 @@ static inline bool defer_init(int nid, unsigned long pfn, unsigned long end_pfn)
>  #endif
>  
>  /* Return a pointer to the bitmap storing bits affecting a block of pages */
> -static inline unsigned long *get_pageblock_bitmap(struct page *page,
> +static inline unsigned long *get_pageblock_bitmap(const struct page *page,
>  							unsigned long pfn)
>  {
>  #ifdef CONFIG_SPARSEMEM
> @@ -484,7 +484,7 @@ static inline unsigned long *get_pageblock_bitmap(struct page *page,
>  #endif /* CONFIG_SPARSEMEM */
>  }
>  
> -static inline int pfn_to_bitidx(struct page *page, unsigned long pfn)
> +static inline int pfn_to_bitidx(const struct page *page, unsigned long pfn)
>  {
>  #ifdef CONFIG_SPARSEMEM
>  	pfn &= (PAGES_PER_SECTION-1);
> @@ -495,7 +495,7 @@ static inline int pfn_to_bitidx(struct page *page, unsigned long pfn)
>  }
>  
>  static __always_inline
> -unsigned long __get_pfnblock_flags_mask(struct page *page,
> +unsigned long __get_pfnblock_flags_mask(const struct page *page,
>  					unsigned long pfn,
>  					unsigned long mask)
>  {
> @@ -520,13 +520,14 @@ unsigned long __get_pfnblock_flags_mask(struct page *page,
>   *
>   * Return: pageblock_bits flags
>   */
> -unsigned long get_pfnblock_flags_mask(struct page *page, unsigned long pfn,
> -					unsigned long mask)
> +unsigned long get_pfnblock_flags_mask(const struct page *page,
> +					unsigned long pfn, unsigned long mask)
>  {
>  	return __get_pfnblock_flags_mask(page, pfn, mask);
>  }
>  
> -static __always_inline int get_pfnblock_migratetype(struct page *page, unsigned long pfn)
> +static __always_inline int get_pfnblock_migratetype(const struct page *page,
> +					unsigned long pfn)
>  {
>  	return __get_pfnblock_flags_mask(page, pfn, MIGRATETYPE_MASK);
>  }
> 

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

Re: [PATCH 6/6] mm: Constify page_count and page_ref_count

[Anshuman Khandual]

On 4/17/21 4:45 AM, Matthew Wilcox (Oracle) wrote:
> Now that compound_head() accepts a const struct page pointer, these two
> functions can be marked as not modifying the page pointer they are passed.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  include/linux/page_ref.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
> index f3318f34fc54..7ad46f45df39 100644
> --- a/include/linux/page_ref.h
> +++ b/include/linux/page_ref.h
> @@ -62,12 +62,12 @@ static inline void __page_ref_unfreeze(struct page *page, int v)
>  
>  #endif
>  
> -static inline int page_ref_count(struct page *page)
> +static inline int page_ref_count(const struct page *page)
>  {
>  	return atomic_read(&page->_refcount);
>  }
>  
> -static inline int page_count(struct page *page)
> +static inline int page_count(const struct page *page)
>  {
>  	return atomic_read(&compound_head(page)->_refcount);
>  }
> 

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>