LinuxLists.cc - [RESEND][PATCH 0/6] Constify struct page arguments

[permalink] [raw]

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

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) <[email protected]>
---
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

2021-04-16 23:18:03

[permalink] [raw]

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

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) <[email protected]>
---
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

2021-04-16 23:19:12

[permalink] [raw]

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

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) <[email protected]>
---
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

2021-04-16 23:19:32

[permalink] [raw]

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

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

Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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

2021-04-16 23:19:36

[permalink] [raw]

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

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

Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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

2021-04-16 23:20:13

[permalink] [raw]

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

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) <[email protected]>
---
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

2021-04-17 07:16:58

by William Kucharski

[permalink] [raw]

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

Looks good to me and I like the cleanup.

For the series:

Reviewed-by: William Kucharski <[email protected]>

> On Apr 16, 2021, at 5:15 PM, Matthew Wilcox (Oracle) <[email protected]> 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
>
>

2021-04-23 14:51:23

[permalink] [raw]

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

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) <[email protected]>

Reviewed-by: Vlastimil Babka <[email protected]>

> ---
> 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();
>

2021-04-23 14:54:50

[permalink] [raw]

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

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) <[email protected]>

Reviewed-by: Vlastimil Babka <[email protected]>

2021-04-23 14:54:55

[permalink] [raw]

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

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) <[email protected]>

Reviewed-by: Vlastimil Babka <[email protected]>

> ---
> 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;
>

2021-04-23 14:59:38

[permalink] [raw]

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

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) <[email protected]>

Reviewed-by: Vlastimil Babka <[email protected]>

> ---
> 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;
>

2021-04-23 15:03:20

[permalink] [raw]

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

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) <[email protected]>

Reviewed-by: Vlastimil Babka <[email protected]>

> ---
> 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);
> }
>

2021-04-23 15:04:09

[permalink] [raw]

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

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) <[email protected]>

Reviewed-by: Vlastimil Babka <[email protected]>

> ---
> 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);
> }
>

2021-04-27 03:30:47

[permalink] [raw]

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

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) <[email protected]>
> ---
> 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 <[email protected]>

2021-04-27 03:31:18

[permalink] [raw]

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

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) <[email protected]>
> ---
> 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 <[email protected]>

2021-04-27 03:31:30

[permalink] [raw]

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

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) <[email protected]>
> ---
> 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 <[email protected]>

2021-04-27 03:32:33

[permalink] [raw]

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

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) <[email protected]>
> ---
> 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 <[email protected]>

2021-04-27 03:32:58

[permalink] [raw]

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

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) <[email protected]>
> ---
> 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 <[email protected]>

2021-04-27 03:33:04