[PATCH v2 0/5] Improvements to %pGp

[PATCH v2 0/5] Improvements to %pGp

[Matthew Wilcox (Oracle)]

The first four patches are improvements to page_flags_test().
The fifth is the patch I originally sent out which exposed some of
the weaknesses in page_flags_test().

v2:
 - use scnprintf (Rasmus)
 - remove page_flags variable (Kirill)

Matthew Wilcox (Oracle) (5):
  test_printf: Make pft array const
  test_printf: Remove separate page_flags variable
  test_printf: Remove custom appending of '|'
  test_printf: Append strings more efficiently
  vsprintf: Make %pGp print the hex value

 lib/test_printf.c   | 61 +++++++++++++++++++--------------------------
 lib/vsprintf.c      |  8 ++++++
 mm/debug.c          |  2 +-
 mm/memory-failure.c |  8 +++---
 mm/page_owner.c     |  4 +--
 mm/slub.c           |  4 +--
 6 files changed, 42 insertions(+), 45 deletions(-)

-- 
2.32.0

[PATCH v2 1/5] test_printf: Make pft array const

[Matthew Wilcox (Oracle)]

Instead of assigning ptf[i].value, leave the values in the on-stack
array and then we can make the array const.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 lib/test_printf.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 55082432f37e..a52c1c3a55ba 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -586,22 +586,21 @@ struct page_flags_test {
 	int width;
 	int shift;
 	int mask;
-	unsigned long value;
 	const char *fmt;
 	const char *name;
 };
 
-static struct page_flags_test pft[] = {
+static const struct page_flags_test pft[] = {
 	{SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK,
-	 0, "%d", "section"},
+	 "%d", "section"},
 	{NODES_WIDTH, NODES_PGSHIFT, NODES_MASK,
-	 0, "%d", "node"},
+	 "%d", "node"},
 	{ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK,
-	 0, "%d", "zone"},
+	 "%d", "zone"},
 	{LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK,
-	 0, "%#x", "lastcpupid"},
+	 "%#x", "lastcpupid"},
 	{KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK,
-	 0, "%#x", "kasantag"},
+	 "%#x", "kasantag"},
 };
 
 static void __init
@@ -627,10 +626,6 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
 #endif
 	}
 
-	/* Set the test value */
-	for (i = 0; i < ARRAY_SIZE(pft); i++)
-		pft[i].value = values[i];
-
 	for (i = 0; i < ARRAY_SIZE(pft); i++) {
 		if (!pft[i].width)
 			continue;
@@ -640,11 +635,11 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
 			size = strlen(cmp_buf);
 		}
 
-		page_flags |= (pft[i].value & pft[i].mask) << pft[i].shift;
+		page_flags |= (values[i] & pft[i].mask) << pft[i].shift;
 		snprintf(cmp_buf + size, BUF_SIZE - size, "%s=", pft[i].name);
 		size = strlen(cmp_buf);
 		snprintf(cmp_buf + size, BUF_SIZE - size, pft[i].fmt,
-			 pft[i].value & pft[i].mask);
+			 values[i] & pft[i].mask);
 		size = strlen(cmp_buf);
 		append = true;
 	}
-- 
2.32.0

[PATCH v2 2/5] test_printf: Remove separate page_flags variable

[Matthew Wilcox (Oracle)]

Keep flags intact so that we also test what happens when unknown flags
are passed to %pGp.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 lib/test_printf.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index a52c1c3a55ba..4531063afd45 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -605,17 +605,15 @@ static const struct page_flags_test pft[] = {
 
 static void __init
 page_flags_test(int section, int node, int zone, int last_cpupid,
-		int kasan_tag, int flags, const char *name, char *cmp_buf)
+		int kasan_tag, unsigned long flags, const char *name,
+		char *cmp_buf)
 {
 	unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag};
-	unsigned long page_flags = 0;
 	unsigned long size = 0;
 	bool append = false;
 	int i;
 
-	flags &= PAGEFLAGS_MASK;
-	if (flags) {
-		page_flags |= flags;
+	if (flags & PAGEFLAGS_MASK) {
 		snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name);
 		size = strlen(cmp_buf);
 #if SECTIONS_WIDTH || NODES_WIDTH || ZONES_WIDTH || \
@@ -635,7 +633,7 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
 			size = strlen(cmp_buf);
 		}
 
-		page_flags |= (values[i] & pft[i].mask) << pft[i].shift;
+		flags |= (values[i] & pft[i].mask) << pft[i].shift;
 		snprintf(cmp_buf + size, BUF_SIZE - size, "%s=", pft[i].name);
 		size = strlen(cmp_buf);
 		snprintf(cmp_buf + size, BUF_SIZE - size, pft[i].fmt,
@@ -644,7 +642,7 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
 		append = true;
 	}
 
-	test(cmp_buf, "%pGp", &page_flags);
+	test(cmp_buf, "%pGp", &flags);
 }
 
 static void __init
-- 
2.32.0

Re: [PATCH v2 2/5] test_printf: Remove separate page_flags variable

[Yafang Shao]

On Tue, Oct 19, 2021 at 10:30 PM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
>
> Keep flags intact so that we also test what happens when unknown flags
> are passed to %pGp.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Yafang Shao <laoar.shao@gmail.com>

> ---
>  lib/test_printf.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index a52c1c3a55ba..4531063afd45 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -605,17 +605,15 @@ static const struct page_flags_test pft[] = {
>
>  static void __init
>  page_flags_test(int section, int node, int zone, int last_cpupid,
> -               int kasan_tag, int flags, const char *name, char *cmp_buf)
> +               int kasan_tag, unsigned long flags, const char *name,
> +               char *cmp_buf)
>  {
>         unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag};
> -       unsigned long page_flags = 0;
>         unsigned long size = 0;
>         bool append = false;
>         int i;
>
> -       flags &= PAGEFLAGS_MASK;
> -       if (flags) {
> -               page_flags |= flags;
> +       if (flags & PAGEFLAGS_MASK) {
>                 snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name);
>                 size = strlen(cmp_buf);
>  #if SECTIONS_WIDTH || NODES_WIDTH || ZONES_WIDTH || \
> @@ -635,7 +633,7 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
>                         size = strlen(cmp_buf);
>                 }
>
> -               page_flags |= (values[i] & pft[i].mask) << pft[i].shift;
> +               flags |= (values[i] & pft[i].mask) << pft[i].shift;
>                 snprintf(cmp_buf + size, BUF_SIZE - size, "%s=", pft[i].name);
>                 size = strlen(cmp_buf);
>                 snprintf(cmp_buf + size, BUF_SIZE - size, pft[i].fmt,
> @@ -644,7 +642,7 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
>                 append = true;
>         }
>
> -       test(cmp_buf, "%pGp", &page_flags);
> +       test(cmp_buf, "%pGp", &flags);
>  }
>
>  static void __init
> --
> 2.32.0
>


-- 
Thanks
Yafang

Re: [PATCH v2 2/5] test_printf: Remove separate page_flags variable

[Petr Mladek]

On Tue 2021-10-19 15:26:18, Matthew Wilcox (Oracle) wrote:
> Keep flags intact so that we also test what happens when unknown flags
> are passed to %pGp.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

[PATCH v2 3/5] test_printf: Remove custom appending of '|'

[Matthew Wilcox (Oracle)]

Instead of having an ifdef to decide whether to print a |, use the
'append' functionality of the main loop to print it.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 lib/test_printf.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 4531063afd45..ec584196cb99 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -616,12 +616,7 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
 	if (flags & PAGEFLAGS_MASK) {
 		snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name);
 		size = strlen(cmp_buf);
-#if SECTIONS_WIDTH || NODES_WIDTH || ZONES_WIDTH || \
-	LAST_CPUPID_WIDTH || KASAN_TAG_WIDTH
-		/* Other information also included in page flags */
-		snprintf(cmp_buf + size, BUF_SIZE - size, "|");
-		size = strlen(cmp_buf);
-#endif
+		append = true;
 	}
 
 	for (i = 0; i < ARRAY_SIZE(pft); i++) {
-- 
2.32.0

[PATCH v2 4/5] test_printf: Append strings more efficiently

[Matthew Wilcox (Oracle)]

Use scnprintf instead of snprintf + strlen.

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

diff --git a/lib/test_printf.c b/lib/test_printf.c
index ec584196cb99..d09993fca463 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -614,8 +614,7 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
 	int i;
 
 	if (flags & PAGEFLAGS_MASK) {
-		snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name);
-		size = strlen(cmp_buf);
+		size += scnprintf(cmp_buf + size, BUF_SIZE - size, "%s", name);
 		append = true;
 	}
 
@@ -623,17 +622,14 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
 		if (!pft[i].width)
 			continue;
 
-		if (append) {
-			snprintf(cmp_buf + size, BUF_SIZE - size, "|");
-			size = strlen(cmp_buf);
-		}
+		if (append)
+			size += scnprintf(cmp_buf + size, BUF_SIZE - size, "|");
 
 		flags |= (values[i] & pft[i].mask) << pft[i].shift;
-		snprintf(cmp_buf + size, BUF_SIZE - size, "%s=", pft[i].name);
-		size = strlen(cmp_buf);
-		snprintf(cmp_buf + size, BUF_SIZE - size, pft[i].fmt,
-			 values[i] & pft[i].mask);
-		size = strlen(cmp_buf);
+		size += scnprintf(cmp_buf + size, BUF_SIZE - size, "%s=",
+				pft[i].name);
+		size += scnprintf(cmp_buf + size, BUF_SIZE - size, pft[i].fmt,
+				values[i] & pft[i].mask);
 		append = true;
 	}
 
-- 
2.32.0

Re: [PATCH v2 4/5] test_printf: Append strings more efficiently

[Yafang Shao]

On Tue, Oct 19, 2021 at 10:33 PM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
>
> Use scnprintf instead of snprintf + strlen.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Yafang Shao <laoar.shao@gmail.com>

> ---
>  lib/test_printf.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index ec584196cb99..d09993fca463 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -614,8 +614,7 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
>         int i;
>
>         if (flags & PAGEFLAGS_MASK) {
> -               snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name);
> -               size = strlen(cmp_buf);
> +               size += scnprintf(cmp_buf + size, BUF_SIZE - size, "%s", name);
>                 append = true;
>         }
>
> @@ -623,17 +622,14 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
>                 if (!pft[i].width)
>                         continue;
>
> -               if (append) {
> -                       snprintf(cmp_buf + size, BUF_SIZE - size, "|");
> -                       size = strlen(cmp_buf);
> -               }
> +               if (append)
> +                       size += scnprintf(cmp_buf + size, BUF_SIZE - size, "|");
>
>                 flags |= (values[i] & pft[i].mask) << pft[i].shift;
> -               snprintf(cmp_buf + size, BUF_SIZE - size, "%s=", pft[i].name);
> -               size = strlen(cmp_buf);
> -               snprintf(cmp_buf + size, BUF_SIZE - size, pft[i].fmt,
> -                        values[i] & pft[i].mask);
> -               size = strlen(cmp_buf);
> +               size += scnprintf(cmp_buf + size, BUF_SIZE - size, "%s=",
> +                               pft[i].name);
> +               size += scnprintf(cmp_buf + size, BUF_SIZE - size, pft[i].fmt,
> +                               values[i] & pft[i].mask);
>                 append = true;
>         }
>
> --
> 2.32.0
>


-- 
Thanks
Yafang

Re: [PATCH v2 4/5] test_printf: Append strings more efficiently

[Petr Mladek]

On Tue 2021-10-19 15:26:20, Matthew Wilcox (Oracle) wrote:
> Use scnprintf instead of snprintf + strlen.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

[PATCH v2 5/5] vsprintf: Make %pGp print the hex value

[Matthew Wilcox (Oracle)]

All existing users of %pGp want the hex value as well as the decoded
flag names.  This looks awkward (passing the same parameter to printf
twice), so move that functionality into the core.  If we want, we
can make that optional with flag arguments to %pGp in the future.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 lib/test_printf.c   | 9 +++++++--
 lib/vsprintf.c      | 8 ++++++++
 mm/debug.c          | 2 +-
 mm/memory-failure.c | 8 ++++----
 mm/page_owner.c     | 4 ++--
 mm/slub.c           | 4 ++--
 6 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index d09993fca463..07309c45f327 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -609,10 +609,14 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
 		char *cmp_buf)
 {
 	unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag};
-	unsigned long size = 0;
+	unsigned long size;
 	bool append = false;
 	int i;
 
+	for (i = 0; i < ARRAY_SIZE(values); i++)
+		flags |= (values[i] & pft[i].mask) << pft[i].shift;
+
+	size = scnprintf(cmp_buf, BUF_SIZE, "%#lx(", flags);
 	if (flags & PAGEFLAGS_MASK) {
 		size += scnprintf(cmp_buf + size, BUF_SIZE - size, "%s", name);
 		append = true;
@@ -625,7 +629,6 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
 		if (append)
 			size += scnprintf(cmp_buf + size, BUF_SIZE - size, "|");
 
-		flags |= (values[i] & pft[i].mask) << pft[i].shift;
 		size += scnprintf(cmp_buf + size, BUF_SIZE - size, "%s=",
 				pft[i].name);
 		size += scnprintf(cmp_buf + size, BUF_SIZE - size, pft[i].fmt,
@@ -633,6 +636,8 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
 		append = true;
 	}
 
+	snprintf(cmp_buf + size, BUF_SIZE - size, ")");
+
 	test(cmp_buf, "%pGp", &flags);
 }
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index d7ad44f2c8f5..214098248610 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2023,6 +2023,11 @@ char *format_page_flags(char *buf, char *end, unsigned long flags)
 	bool append = false;
 	int i;
 
+	buf = number(buf, end, flags, default_flag_spec);
+	if (buf < end)
+		*buf = '(';
+	buf++;
+
 	/* Page flags from the main area. */
 	if (main_flags) {
 		buf = format_flags(buf, end, main_flags, pageflag_names);
@@ -2051,6 +2056,9 @@ char *format_page_flags(char *buf, char *end, unsigned long flags)
 
 		append = true;
 	}
+	if (buf < end)
+		*buf = ')';
+	buf++;
 
 	return buf;
 }
diff --git a/mm/debug.c b/mm/debug.c
index fae0f81ad831..714be101dec9 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -162,7 +162,7 @@ static void __dump_page(struct page *page)
 out_mapping:
 	BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1);
 
-	pr_warn("%sflags: %#lx(%pGp)%s\n", type, head->flags, &head->flags,
+	pr_warn("%sflags: %pGp%s\n", type, &head->flags,
 		page_cma ? " CMA" : "");
 	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
 			sizeof(unsigned long), page,
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 3e6449f2102a..e4e122a2e9b1 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2109,14 +2109,14 @@ static int __soft_offline_page(struct page *page)
 			if (!list_empty(&pagelist))
 				putback_movable_pages(&pagelist);
 
-			pr_info("soft offline: %#lx: %s migration failed %d, type %lx (%pGp)\n",
-				pfn, msg_page[huge], ret, page->flags, &page->flags);
+			pr_info("soft offline: %#lx: %s migration failed %d, type %pGp\n",
+				pfn, msg_page[huge], ret, &page->flags);
 			if (ret > 0)
 				ret = -EBUSY;
 		}
 	} else {
-		pr_info("soft offline: %#lx: %s isolation failed, page count %d, type %lx (%pGp)\n",
-			pfn, msg_page[huge], page_count(page), page->flags, &page->flags);
+		pr_info("soft offline: %#lx: %s isolation failed, page count %d, type %pGp\n",
+			pfn, msg_page[huge], page_count(page), &page->flags);
 		ret = -EBUSY;
 	}
 	return ret;
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 62402d22539b..4afc713ca525 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -351,12 +351,12 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
 	pageblock_mt = get_pageblock_migratetype(page);
 	page_mt  = gfp_migratetype(page_owner->gfp_mask);
 	ret += snprintf(kbuf + ret, count - ret,
-			"PFN %lu type %s Block %lu type %s Flags %#lx(%pGp)\n",
+			"PFN %lu type %s Block %lu type %s Flags %pGp\n",
 			pfn,
 			migratetype_names[page_mt],
 			pfn >> pageblock_order,
 			migratetype_names[pageblock_mt],
-			page->flags, &page->flags);
+			&page->flags);
 
 	if (ret >= count)
 		goto err;
diff --git a/mm/slub.c b/mm/slub.c
index 3d2025f7163b..f7ac28646580 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -763,9 +763,9 @@ void print_tracking(struct kmem_cache *s, void *object)
 
 static void print_page_info(struct page *page)
 {
-	pr_err("Slab 0x%p objects=%u used=%u fp=0x%p flags=%#lx(%pGp)\n",
+	pr_err("Slab 0x%p objects=%u used=%u fp=0x%p flags=%pGp\n",
 	       page, page->objects, page->inuse, page->freelist,
-	       page->flags, &page->flags);
+	       &page->flags);
 
 }
 
-- 
2.32.0

Re: [PATCH v2 5/5] vsprintf: Make %pGp print the hex value

[Yafang Shao]

On Tue, Oct 19, 2021 at 10:34 PM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
>
> All existing users of %pGp want the hex value as well as the decoded
> flag names.  This looks awkward (passing the same parameter to printf
> twice), so move that functionality into the core.  If we want, we
> can make that optional with flag arguments to %pGp in the future.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

LGTM

Reviewed-by: Yafang Shao <laoar.shao@gmail.com>

> ---
>  lib/test_printf.c   | 9 +++++++--
>  lib/vsprintf.c      | 8 ++++++++
>  mm/debug.c          | 2 +-
>  mm/memory-failure.c | 8 ++++----
>  mm/page_owner.c     | 4 ++--
>  mm/slub.c           | 4 ++--
>  6 files changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index d09993fca463..07309c45f327 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -609,10 +609,14 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
>                 char *cmp_buf)
>  {
>         unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag};
> -       unsigned long size = 0;
> +       unsigned long size;
>         bool append = false;
>         int i;
>
> +       for (i = 0; i < ARRAY_SIZE(values); i++)
> +               flags |= (values[i] & pft[i].mask) << pft[i].shift;
> +
> +       size = scnprintf(cmp_buf, BUF_SIZE, "%#lx(", flags);
>         if (flags & PAGEFLAGS_MASK) {
>                 size += scnprintf(cmp_buf + size, BUF_SIZE - size, "%s", name);
>                 append = true;
> @@ -625,7 +629,6 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
>                 if (append)
>                         size += scnprintf(cmp_buf + size, BUF_SIZE - size, "|");
>
> -               flags |= (values[i] & pft[i].mask) << pft[i].shift;
>                 size += scnprintf(cmp_buf + size, BUF_SIZE - size, "%s=",
>                                 pft[i].name);
>                 size += scnprintf(cmp_buf + size, BUF_SIZE - size, pft[i].fmt,
> @@ -633,6 +636,8 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
>                 append = true;
>         }
>
> +       snprintf(cmp_buf + size, BUF_SIZE - size, ")");
> +
>         test(cmp_buf, "%pGp", &flags);
>  }
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index d7ad44f2c8f5..214098248610 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -2023,6 +2023,11 @@ char *format_page_flags(char *buf, char *end, unsigned long flags)
>         bool append = false;
>         int i;
>
> +       buf = number(buf, end, flags, default_flag_spec);
> +       if (buf < end)
> +               *buf = '(';
> +       buf++;
> +
>         /* Page flags from the main area. */
>         if (main_flags) {
>                 buf = format_flags(buf, end, main_flags, pageflag_names);
> @@ -2051,6 +2056,9 @@ char *format_page_flags(char *buf, char *end, unsigned long flags)
>
>                 append = true;
>         }
> +       if (buf < end)
> +               *buf = ')';
> +       buf++;
>
>         return buf;
>  }
> diff --git a/mm/debug.c b/mm/debug.c
> index fae0f81ad831..714be101dec9 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -162,7 +162,7 @@ static void __dump_page(struct page *page)
>  out_mapping:
>         BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1);
>
> -       pr_warn("%sflags: %#lx(%pGp)%s\n", type, head->flags, &head->flags,
> +       pr_warn("%sflags: %pGp%s\n", type, &head->flags,
>                 page_cma ? " CMA" : "");
>         print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
>                         sizeof(unsigned long), page,
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 3e6449f2102a..e4e122a2e9b1 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2109,14 +2109,14 @@ static int __soft_offline_page(struct page *page)
>                         if (!list_empty(&pagelist))
>                                 putback_movable_pages(&pagelist);
>
> -                       pr_info("soft offline: %#lx: %s migration failed %d, type %lx (%pGp)\n",
> -                               pfn, msg_page[huge], ret, page->flags, &page->flags);
> +                       pr_info("soft offline: %#lx: %s migration failed %d, type %pGp\n",
> +                               pfn, msg_page[huge], ret, &page->flags);
>                         if (ret > 0)
>                                 ret = -EBUSY;
>                 }
>         } else {
> -               pr_info("soft offline: %#lx: %s isolation failed, page count %d, type %lx (%pGp)\n",
> -                       pfn, msg_page[huge], page_count(page), page->flags, &page->flags);
> +               pr_info("soft offline: %#lx: %s isolation failed, page count %d, type %pGp\n",
> +                       pfn, msg_page[huge], page_count(page), &page->flags);
>                 ret = -EBUSY;
>         }
>         return ret;
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index 62402d22539b..4afc713ca525 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -351,12 +351,12 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
>         pageblock_mt = get_pageblock_migratetype(page);
>         page_mt  = gfp_migratetype(page_owner->gfp_mask);
>         ret += snprintf(kbuf + ret, count - ret,
> -                       "PFN %lu type %s Block %lu type %s Flags %#lx(%pGp)\n",
> +                       "PFN %lu type %s Block %lu type %s Flags %pGp\n",
>                         pfn,
>                         migratetype_names[page_mt],
>                         pfn >> pageblock_order,
>                         migratetype_names[pageblock_mt],
> -                       page->flags, &page->flags);
> +                       &page->flags);
>
>         if (ret >= count)
>                 goto err;
> diff --git a/mm/slub.c b/mm/slub.c
> index 3d2025f7163b..f7ac28646580 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -763,9 +763,9 @@ void print_tracking(struct kmem_cache *s, void *object)
>
>  static void print_page_info(struct page *page)
>  {
> -       pr_err("Slab 0x%p objects=%u used=%u fp=0x%p flags=%#lx(%pGp)\n",
> +       pr_err("Slab 0x%p objects=%u used=%u fp=0x%p flags=%pGp\n",
>                page, page->objects, page->inuse, page->freelist,
> -              page->flags, &page->flags);
> +              &page->flags);
>
>  }
>
> --
> 2.32.0
>


-- 
Thanks
Yafang

Re: [PATCH v2 5/5] vsprintf: Make %pGp print the hex value

[Petr Mladek]

On Tue 2021-10-19 15:26:21, Matthew Wilcox (Oracle) wrote:
> All existing users of %pGp want the hex value as well as the decoded
> flag names.  This looks awkward (passing the same parameter to printf
> twice), so move that functionality into the core.  If we want, we
> can make that optional with flag arguments to %pGp in the future.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

[PATCH] vsprintf: Update %pGp documentation about that it prints hex value

[Petr Mladek]

The commit 23efd0804c0a869dfb1e7 ("vsprintf: Make %pGp print
the hex value") changed the behavior of %pGp printk format.
Update the documentation accordingly.

Fixes: 23efd0804c0a869dfb1e7 ("vsprintf: Make %pGp print the hex value")
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
The commit ID 23efd0804c0a869dfb1e7 is taken from printk/linux.git,
branch for-5.16-vsprintf-pgp. It should get preserved when the branch
is merged into Linus tree.

 Documentation/core-api/printk-formats.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index e08bbe9b0cbf..4eff98e06b4b 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -580,7 +580,7 @@ Flags bitfields such as page flags, gfp_flags
 
 ::
 
-	%pGp	referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff
+	%pGp	0x17ffffc0002036(referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff
 	%pGg	GFP_USER|GFP_DMA32|GFP_NOWARN
 	%pGv	read|exec|mayread|maywrite|mayexec|denywrite
 
-- 
2.26.2

Re: [PATCH] vsprintf: Update %pGp documentation about that it prints hex value

[Yafang Shao]

On Wed, Oct 27, 2021 at 8:48 PM Petr Mladek <pmladek@suse.com> wrote:
>
> The commit 23efd0804c0a869dfb1e7 ("vsprintf: Make %pGp print
> the hex value") changed the behavior of %pGp printk format.
> Update the documentation accordingly.
>
> Fixes: 23efd0804c0a869dfb1e7 ("vsprintf: Make %pGp print the hex value")
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Reviewed-by: Yafang Shao <laoar.shao@gmail.com>

> ---
> The commit ID 23efd0804c0a869dfb1e7 is taken from printk/linux.git,
> branch for-5.16-vsprintf-pgp. It should get preserved when the branch
> is merged into Linus tree.
>
>  Documentation/core-api/printk-formats.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index e08bbe9b0cbf..4eff98e06b4b 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -580,7 +580,7 @@ Flags bitfields such as page flags, gfp_flags
>
>  ::
>
> -       %pGp    referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff
> +       %pGp    0x17ffffc0002036(referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff
>         %pGg    GFP_USER|GFP_DMA32|GFP_NOWARN
>         %pGv    read|exec|mayread|maywrite|mayexec|denywrite
>
> --
> 2.26.2



-- 
Thanks
Yafang

Re: [PATCH] vsprintf: Update %pGp documentation about that it prints hex value

[Yafang Shao]

On Fri, Oct 29, 2021 at 10:15 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> On Wed, Oct 27, 2021 at 8:48 PM Petr Mladek <pmladek@suse.com> wrote:
> >
> > The commit 23efd0804c0a869dfb1e7 ("vsprintf: Make %pGp print
> > the hex value") changed the behavior of %pGp printk format.
> > Update the documentation accordingly.
> >
> > Fixes: 23efd0804c0a869dfb1e7 ("vsprintf: Make %pGp print the hex value")
> > Signed-off-by: Petr Mladek <pmladek@suse.com>
>
> Reviewed-by: Yafang Shao <laoar.shao@gmail.com>
>

With below minor change:
-       %pGp
0x17ffffc0002036(referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff
+       %pGp
0x17ffffc0002036(referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff)

The ')' is lost.

I just checked the data before :(


> > ---
> > The commit ID 23efd0804c0a869dfb1e7 is taken from printk/linux.git,
> > branch for-5.16-vsprintf-pgp. It should get preserved when the branch
> > is merged into Linus tree.
> >
> >  Documentation/core-api/printk-formats.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> > index e08bbe9b0cbf..4eff98e06b4b 100644
> > --- a/Documentation/core-api/printk-formats.rst
> > +++ b/Documentation/core-api/printk-formats.rst
> > @@ -580,7 +580,7 @@ Flags bitfields such as page flags, gfp_flags
> >
> >  ::
> >
> > -       %pGp    referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff
> > +       %pGp    0x17ffffc0002036(referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff
> >         %pGg    GFP_USER|GFP_DMA32|GFP_NOWARN
> >         %pGv    read|exec|mayread|maywrite|mayexec|denywrite
> >
> > --
> > 2.26.2
>
>
>
> --
> Thanks
> Yafang



-- 
Thanks
Yafang

Re: [PATCH] vsprintf: Update %pGp documentation about that it prints hex value

[Petr Mladek]

On Fri 2021-10-29 10:18:29, Yafang Shao wrote:
> On Fri, Oct 29, 2021 at 10:15 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > On Wed, Oct 27, 2021 at 8:48 PM Petr Mladek <pmladek@suse.com> wrote:
> > >
> > > The commit 23efd0804c0a869dfb1e7 ("vsprintf: Make %pGp print
> > > the hex value") changed the behavior of %pGp printk format.
> > > Update the documentation accordingly.
> > >
> > > Fixes: 23efd0804c0a869dfb1e7 ("vsprintf: Make %pGp print the hex value")
> > > Signed-off-by: Petr Mladek <pmladek@suse.com>
> >
> > Reviewed-by: Yafang Shao <laoar.shao@gmail.com>
> >
> 
> With below minor change:
> -       %pGp
> 0x17ffffc0002036(referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff
> +       %pGp
> 0x17ffffc0002036(referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff)
> 
> The ')' is lost.

Great catch!

I have committed the fixed patch into printk/linux.git, branch
for-5.16-vsprintf-pgp.

Best Regards,
Petr

Re: [PATCH v2 0/5] Improvements to %pGp

[Petr Mladek]

On Tue 2021-10-19 15:26:16, Matthew Wilcox (Oracle) wrote:
> The first four patches are improvements to page_flags_test().
> The fifth is the patch I originally sent out which exposed some of
> the weaknesses in page_flags_test().
> 
> v2:
>  - use scnprintf (Rasmus)
>  - remove page_flags variable (Kirill)
> 
> Matthew Wilcox (Oracle) (5):
>   test_printf: Make pft array const
>   test_printf: Remove separate page_flags variable
>   test_printf: Remove custom appending of '|'
>   test_printf: Append strings more efficiently
>   vsprintf: Make %pGp print the hex value
> 
>  lib/test_printf.c   | 61 +++++++++++++++++++--------------------------
>  lib/vsprintf.c      |  8 ++++++
>  mm/debug.c          |  2 +-
>  mm/memory-failure.c |  8 +++---
>  mm/page_owner.c     |  4 +--
>  mm/slub.c           |  4 +--
>  6 files changed, 42 insertions(+), 45 deletions(-)

The patchset has been committed into printk/linux.git,
branch for-5.16-vsprintf-pgp.

Best Regards,
Petr