[PATCH v5 0/3] mm, vsprintf: dump full information of page flags in pGp

[PATCH v5 0/3] mm, vsprintf: dump full information of page flags in pGp

[Yafang Shao]

The existed pGp shows the names of page flags only, rather than the full
information including section, node, zone, last cpuipid and kasan tag.
While it is not easy to parse these information manually because there
are so many flavors. We'd better interpret them in printf.

To be compitable with the existed format of pGp, the new introduced ones
also use '|' as the separator, then the user tools parsing pGp won't
need to make change, suggested by Matthew. The new added information is
tracked onto the end of the existed one, e.g.
[ 8838.835456] Slab 0x000000002828b78a objects=33 used=3 fp=0x00000000d04efc88 flags=0x17ffffc0010200(slab|head|node=0|zone=2|lastcpupid=0x1fffff)

The documentation and test cases are also updated. The result of the 
test cases as follows,
[11585.830272] test_printf: loaded.
[11585.830454] test_printf: all 388 tests passed
[11585.831401] test_printf: unloaded.

This patchset also includes some code cleanup in mm/slub.c.

v5:
- remove the bitmap and better name the struct, per Petr 

v4:
- extend %pGp instead of introducing new format, per Matthew

v3:
- coding improvement, per Joe and Andy
- the possible impact on debugfs and the fix of it, per Joe and Matthew
- introduce new format instead of changing pGp, per Andy

v2:
- various coding improvement, per Joe, Miaohe, Vlastimil and Andy
- remove the prefix completely in patch #2, per Vlastimil
- Update the test cases, per Andy

Yafang Shao (3):
  mm, slub: use pGp to print page flags
  mm, slub: don't combine pr_err with INFO
  vsprintf: dump full information of page flags in pGp

 Documentation/core-api/printk-formats.rst |  2 +-
 lib/test_printf.c                         | 60 +++++++++++++++++----
 lib/vsprintf.c                            | 66 +++++++++++++++++++++--
 mm/slub.c                                 | 13 ++---
 4 files changed, 119 insertions(+), 22 deletions(-)

-- 
2.18.2

[PATCH v5 1/3] mm, slub: use pGp to print page flags

[Yafang Shao]

As pGp has been already introduced in printk, we'd better use it to make
the output human readable.

Before this change, the output is,
[ 6155.716018] INFO: Slab 0x000000004027dd4f objects=33 used=3 fp=0x000000008cd1579c flags=0x17ffffc0010200

While after this change, the output is,
[ 8846.517809] INFO: Slab 0x00000000f42a2c60 objects=33 used=3 fp=0x0000000060d32ca8 flags=0x17ffffc0010200(slab|head)

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: David Rientjes <rientjes@google.com>
Acked-by: Christoph Lameter <cl@linux.com>
Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 mm/slub.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 7ecbbbe5bc0c..ddd429832577 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -641,8 +641,9 @@ void print_tracking(struct kmem_cache *s, void *object)
 
 static void print_page_info(struct page *page)
 {
-	pr_err("INFO: Slab 0x%p objects=%u used=%u fp=0x%p flags=0x%04lx\n",
-	       page, page->objects, page->inuse, page->freelist, page->flags);
+	pr_err("INFO: Slab 0x%p objects=%u used=%u fp=0x%p flags=%#lx(%pGp)\n",
+	       page, page->objects, page->inuse, page->freelist,
+	       page->flags, &page->flags);
 
 }
 
-- 
2.18.2

[PATCH v5 2/3] mm, slub: don't combine pr_err with INFO

[Yafang Shao]

It is strange to combine "pr_err" with "INFO", so let's remove the
prefix completely.
This patch is motivated by David's comment[1].

- before the patch
[ 8846.517809] INFO: Slab 0x00000000f42a2c60 objects=33 used=3 fp=0x0000000060d32ca8 flags=0x17ffffc0010200(slab|head)

- after the patch
[ 6343.396602] Slab 0x000000004382e02b objects=33 used=3 fp=0x000000009ae06ffc flags=0x17ffffc0010200(slab|head)

[1]. https://lore.kernel.org/linux-mm/b9c0f2b6-e9b0-0c36-ebdd-2bc684c5a762@redhat.com/#t

Suggested-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Cc: Matthew Wilcox <willy@infradead.org>
---
 mm/slub.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index ddd429832577..d66a7ebd21fe 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -615,7 +615,7 @@ static void print_track(const char *s, struct track *t, unsigned long pr_time)
 	if (!t->addr)
 		return;
 
-	pr_err("INFO: %s in %pS age=%lu cpu=%u pid=%d\n",
+	pr_err("%s in %pS age=%lu cpu=%u pid=%d\n",
 	       s, (void *)t->addr, pr_time - t->when, t->cpu, t->pid);
 #ifdef CONFIG_STACKTRACE
 	{
@@ -641,7 +641,7 @@ void print_tracking(struct kmem_cache *s, void *object)
 
 static void print_page_info(struct page *page)
 {
-	pr_err("INFO: 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=%#lx(%pGp)\n",
 	       page, page->objects, page->inuse, page->freelist,
 	       page->flags, &page->flags);
 
@@ -698,7 +698,7 @@ static void print_trailer(struct kmem_cache *s, struct page *page, u8 *p)
 
 	print_page_info(page);
 
-	pr_err("INFO: Object 0x%p @offset=%tu fp=0x%p\n\n",
+	pr_err("Object 0x%p @offset=%tu fp=0x%p\n\n",
 	       p, p - addr, get_freepointer(s, p));
 
 	if (s->flags & SLAB_RED_ZONE)
@@ -791,7 +791,7 @@ static int check_bytes_and_report(struct kmem_cache *s, struct page *page,
 		end--;
 
 	slab_bug(s, "%s overwritten", what);
-	pr_err("INFO: 0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
+	pr_err("0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
 					fault, end - 1, fault - addr,
 					fault[0], value);
 	print_trailer(s, page, object);
@@ -3855,7 +3855,7 @@ static void list_slab_objects(struct kmem_cache *s, struct page *page,
 	for_each_object(p, s, addr, page->objects) {
 
 		if (!test_bit(__obj_to_index(s, addr, p), map)) {
-			pr_err("INFO: Object 0x%p @offset=%tu\n", p, p - addr);
+			pr_err("Object 0x%p @offset=%tu\n", p, p - addr);
 			print_tracking(s, p);
 		}
 	}
-- 
2.18.2

Re: [PATCH v5 2/3] mm, slub: don't combine pr_err with INFO

[David Rientjes]

On Mon, 15 Feb 2021, Yafang Shao wrote:

> It is strange to combine "pr_err" with "INFO", so let's remove the
> prefix completely.
> This patch is motivated by David's comment[1].
> 
> - before the patch
> [ 8846.517809] INFO: Slab 0x00000000f42a2c60 objects=33 used=3 fp=0x0000000060d32ca8 flags=0x17ffffc0010200(slab|head)
> 
> - after the patch
> [ 6343.396602] Slab 0x000000004382e02b objects=33 used=3 fp=0x000000009ae06ffc flags=0x17ffffc0010200(slab|head)
> 
> [1]. https://lore.kernel.org/linux-mm/b9c0f2b6-e9b0-0c36-ebdd-2bc684c5a762@redhat.com/#t
> 
> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Cc: Matthew Wilcox <willy@infradead.org>

Acked-by: David Rientjes <rientjes@google.com>

[PATCH v5 3/3] vsprintf: dump full information of page flags in pGp

[Yafang Shao]

Currently the pGp only shows the names of page flags, rather than
the full information including section, node, zone, last cpupid and
kasan tag. While it is not easy to parse these information manually
because there're so many flavors. Let's interpret them in pGp as well.

To be compitable with the existed format of pGp, the new introduced ones
also use '|' as the separator, then the user tools parsing pGp won't
need to make change, suggested by Matthew. The new information is
tracked onto the end of the existed one.

One example of the output in mm/slub.c as follows,
- Before the patch,
[ 6343.396602] Slab 0x000000004382e02b objects=33 used=3 fp=0x000000009ae06ffc flags=0x17ffffc0010200(slab|head)

- After the patch,
[ 8448.272530] Slab 0x0000000090797883 objects=33 used=3 fp=0x00000000790f1c26 flags=0x17ffffc0010200(slab|head|node=0|zone=2|lastcpupid=0x1fffff)

The documentation and test cases are also updated. The output of the
test cases as follows,
[11585.830272] test_printf: loaded.
[11585.830454] test_printf: all 388 tests passed
[11585.831401] test_printf: unloaded.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Joe Perches <joe@perches.com>
Cc: Miaohe Lin <linmiaohe@huawei.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Petr Mladek <pmladek@suse.com>
---
 Documentation/core-api/printk-formats.rst |  2 +-
 lib/test_printf.c                         | 60 +++++++++++++++++----
 lib/vsprintf.c                            | 66 +++++++++++++++++++++--
 3 files changed, 112 insertions(+), 16 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 160e710d992f..00d07c7eefd4 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -540,7 +540,7 @@ Flags bitfields such as page flags, gfp_flags
 
 ::
 
-	%pGp	referenced|uptodate|lru|active|private
+	%pGp	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
 
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 7ac87f18a10f..148773dfe97a 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -569,24 +569,68 @@ netdev_features(void)
 {
 }
 
+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)
+{
+	unsigned long page_flags = 0;
+	unsigned long size = 0;
+
+	flags &= BIT(NR_PAGEFLAGS) - 1;
+	if (flags) {
+		page_flags |= flags;
+		snprintf(cmp_buf + size, BUF_SIZE - size, "%s|", name);
+		size = strlen(cmp_buf);
+	}
+
+#ifdef SECTION_IN_PAGE_FLAGS
+	page_flags |= (sec & SECTIONS_MASK) << SECTIONS_PGSHIFT;
+	snprintf(cmp_buf + size, BUF_SIZE - size, "section=%#x|", sec);
+	size = strlen(cmp_buf);
+#endif
+
+	page_flags |= ((node & NODES_MASK) << NODES_PGSHIFT) |
+			((zone & ZONES_MASK) << ZONES_PGSHIFT);
+	snprintf(cmp_buf + size, BUF_SIZE - size, "node=%d|zone=%d", node, zone);
+	size = strlen(cmp_buf);
+
+#ifndef LAST_CPUPID_NOT_IN_PAGE_FLAGS
+	page_flags |= (last_cpupid & LAST_CPUPID_MASK) << LAST_CPUPID_PGSHIFT;
+	snprintf(cmp_buf + size, BUF_SIZE - size, "|lastcpupid=%#x", last_cpupid);
+	size = strlen(cmp_buf);
+#endif
+
+#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
+	page_flags |= (tag & KASAN_TAG_MASK) << KASAN_TAG_PGSHIFT;
+	snprintf(cmp_buf + size, BUF_SIZE - size, "|kasantag=%#x", tag);
+	size = strlen(cmp_buf);
+#endif
+
+	test(cmp_buf, "%pGp", &page_flags);
+}
+
 static void __init
 flags(void)
 {
 	unsigned long flags;
-	gfp_t gfp;
 	char *cmp_buffer;
+	gfp_t gfp;
+
+	cmp_buffer = kmalloc(BUF_SIZE, GFP_KERNEL);
+	if (!cmp_buffer)
+		return;
 
 	flags = 0;
-	test("", "%pGp", &flags);
+	page_flags_test(0, 0, 0, 0, 0, flags, "", cmp_buffer);
 
-	/* Page flags should filter the zone id */
 	flags = 1UL << NR_PAGEFLAGS;
-	test("", "%pGp", &flags);
+	page_flags_test(0, 0, 0, 0, 0, flags, "", cmp_buffer);
 
 	flags |= 1UL << PG_uptodate | 1UL << PG_dirty | 1UL << PG_lru
 		| 1UL << PG_active | 1UL << PG_swapbacked;
-	test("uptodate|dirty|lru|active|swapbacked", "%pGp", &flags);
-
+	page_flags_test(1, 1, 1, 0x1fffff, 1, flags,
+			"uptodate|dirty|lru|active|swapbacked",
+			cmp_buffer);
 
 	flags = VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC
 			| VM_DENYWRITE;
@@ -601,10 +645,6 @@ flags(void)
 	gfp = __GFP_ATOMIC;
 	test("__GFP_ATOMIC", "%pGg", &gfp);
 
-	cmp_buffer = kmalloc(BUF_SIZE, GFP_KERNEL);
-	if (!cmp_buffer)
-		return;
-
 	/* Any flags not translated by the table should remain numeric */
 	gfp = ~__GFP_BITS_MASK;
 	snprintf(cmp_buffer, BUF_SIZE, "%#lx", (unsigned long) gfp);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3b53c73580c5..0dc776f7dfa4 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1916,6 +1916,66 @@ char *format_flags(char *buf, char *end, unsigned long flags,
 	return buf;
 }
 
+struct page_flags_fields {
+	int width;
+	int shift;
+	int mask;
+	const struct printf_spec *spec;
+	const char *name;
+};
+
+static const struct page_flags_fields pff[] = {
+	{SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK,
+	 &default_dec_spec, "section"},
+	{NODES_WIDTH, NODES_PGSHIFT, NODES_MASK,
+	 &default_dec_spec, "node"},
+	{ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK,
+	 &default_dec_spec, "zone"},
+	{LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK,
+	 &default_flag_spec, "lastcpupid"},
+	{KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK,
+	 &default_flag_spec, "kasantag"},
+};
+
+static
+char *format_page_flags(char *buf, char *end, unsigned long flags)
+{
+	unsigned long main_flags = flags & (BIT(NR_PAGEFLAGS) - 1);
+	bool append = false;
+	int i;
+
+	/* Page flags from the main area. */
+	if (main_flags) {
+		buf = format_flags(buf, end, main_flags, pageflag_names);
+		append = true;
+	}
+
+	/* Page flags from the fields area */
+	for (i = 0; i < ARRAY_SIZE(pff); i++) {
+		/* Skip undefined fields. */
+		if (!pff[i].width)
+			continue;
+
+		/* Format: Flag Name + '=' (equals sign) + Number + '|' (separator) */
+		if (append) {
+			if (buf < end)
+				*buf = '|';
+			buf++;
+		}
+
+		buf = string(buf, end, pff[i].name, *pff[i].spec);
+		if (buf < end)
+			*buf = '=';
+		buf++;
+		buf = number(buf, end, (flags >> pff[i].shift) & pff[i].mask,
+			     *pff[i].spec);
+
+		append = true;
+	}
+
+	return buf;
+}
+
 static noinline_for_stack
 char *flags_string(char *buf, char *end, void *flags_ptr,
 		   struct printf_spec spec, const char *fmt)
@@ -1928,11 +1988,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
 
 	switch (fmt[1]) {
 	case 'p':
-		flags = *(unsigned long *)flags_ptr;
-		/* Remove zone id */
-		flags &= (1UL << NR_PAGEFLAGS) - 1;
-		names = pageflag_names;
-		break;
+		return format_page_flags(buf, end, *(unsigned long *)flags_ptr);
 	case 'v':
 		flags = *(unsigned long *)flags_ptr;
 		names = vmaflag_names;
-- 
2.18.2

Re: [PATCH v5 3/3] vsprintf: dump full information of page flags in pGp

[Yafang Shao]

On Mon, Feb 15, 2021 at 11:53 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> Currently the pGp only shows the names of page flags, rather than
> the full information including section, node, zone, last cpupid and
> kasan tag. While it is not easy to parse these information manually
> because there're so many flavors. Let's interpret them in pGp as well.
>
> To be compitable with the existed format of pGp, the new introduced ones
> also use '|' as the separator, then the user tools parsing pGp won't
> need to make change, suggested by Matthew. The new information is
> tracked onto the end of the existed one.
>
> One example of the output in mm/slub.c as follows,
> - Before the patch,
> [ 6343.396602] Slab 0x000000004382e02b objects=33 used=3 fp=0x000000009ae06ffc flags=0x17ffffc0010200(slab|head)
>
> - After the patch,
> [ 8448.272530] Slab 0x0000000090797883 objects=33 used=3 fp=0x00000000790f1c26 flags=0x17ffffc0010200(slab|head|node=0|zone=2|lastcpupid=0x1fffff)
>
> The documentation and test cases are also updated. The output of the
> test cases as follows,
> [11585.830272] test_printf: loaded.
> [11585.830454] test_printf: all 388 tests passed
> [11585.831401] test_printf: unloaded.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Miaohe Lin <linmiaohe@huawei.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Petr Mladek <pmladek@suse.com>
> ---
>  Documentation/core-api/printk-formats.rst |  2 +-
>  lib/test_printf.c                         | 60 +++++++++++++++++----
>  lib/vsprintf.c                            | 66 +++++++++++++++++++++--
>  3 files changed, 112 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index 160e710d992f..00d07c7eefd4 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -540,7 +540,7 @@ Flags bitfields such as page flags, gfp_flags
>
>  ::
>
> -       %pGp    referenced|uptodate|lru|active|private
> +       %pGp    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
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 7ac87f18a10f..148773dfe97a 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -569,24 +569,68 @@ netdev_features(void)
>  {
>  }
>
> +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)
> +{
> +       unsigned long page_flags = 0;
> +       unsigned long size = 0;
> +
> +       flags &= BIT(NR_PAGEFLAGS) - 1;
> +       if (flags) {
> +               page_flags |= flags;
> +               snprintf(cmp_buf + size, BUF_SIZE - size, "%s|", name);
> +               size = strlen(cmp_buf);
> +       }
> +
> +#ifdef SECTION_IN_PAGE_FLAGS
> +       page_flags |= (sec & SECTIONS_MASK) << SECTIONS_PGSHIFT;
> +       snprintf(cmp_buf + size, BUF_SIZE - size, "section=%#x|", sec);
> +       size = strlen(cmp_buf);
> +#endif
> +
> +       page_flags |= ((node & NODES_MASK) << NODES_PGSHIFT) |
> +                       ((zone & ZONES_MASK) << ZONES_PGSHIFT);
> +       snprintf(cmp_buf + size, BUF_SIZE - size, "node=%d|zone=%d", node, zone);
> +       size = strlen(cmp_buf);
> +
> +#ifndef LAST_CPUPID_NOT_IN_PAGE_FLAGS
> +       page_flags |= (last_cpupid & LAST_CPUPID_MASK) << LAST_CPUPID_PGSHIFT;
> +       snprintf(cmp_buf + size, BUF_SIZE - size, "|lastcpupid=%#x", last_cpupid);
> +       size = strlen(cmp_buf);
> +#endif
> +
> +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
> +       page_flags |= (tag & KASAN_TAG_MASK) << KASAN_TAG_PGSHIFT;
> +       snprintf(cmp_buf + size, BUF_SIZE - size, "|kasantag=%#x", tag);
> +       size = strlen(cmp_buf);
> +#endif
> +
> +       test(cmp_buf, "%pGp", &page_flags);
> +}
> +
>  static void __init
>  flags(void)
>  {
>         unsigned long flags;
> -       gfp_t gfp;
>         char *cmp_buffer;
> +       gfp_t gfp;
> +
> +       cmp_buffer = kmalloc(BUF_SIZE, GFP_KERNEL);
> +       if (!cmp_buffer)
> +               return;
>
>         flags = 0;
> -       test("", "%pGp", &flags);
> +       page_flags_test(0, 0, 0, 0, 0, flags, "", cmp_buffer);
>
> -       /* Page flags should filter the zone id */
>         flags = 1UL << NR_PAGEFLAGS;
> -       test("", "%pGp", &flags);
> +       page_flags_test(0, 0, 0, 0, 0, flags, "", cmp_buffer);
>
>         flags |= 1UL << PG_uptodate | 1UL << PG_dirty | 1UL << PG_lru
>                 | 1UL << PG_active | 1UL << PG_swapbacked;
> -       test("uptodate|dirty|lru|active|swapbacked", "%pGp", &flags);
> -
> +       page_flags_test(1, 1, 1, 0x1fffff, 1, flags,
> +                       "uptodate|dirty|lru|active|swapbacked",
> +                       cmp_buffer);
>
>         flags = VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC
>                         | VM_DENYWRITE;
> @@ -601,10 +645,6 @@ flags(void)
>         gfp = __GFP_ATOMIC;
>         test("__GFP_ATOMIC", "%pGg", &gfp);
>
> -       cmp_buffer = kmalloc(BUF_SIZE, GFP_KERNEL);
> -       if (!cmp_buffer)
> -               return;
> -
>         /* Any flags not translated by the table should remain numeric */
>         gfp = ~__GFP_BITS_MASK;
>         snprintf(cmp_buffer, BUF_SIZE, "%#lx", (unsigned long) gfp);
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 3b53c73580c5..0dc776f7dfa4 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1916,6 +1916,66 @@ char *format_flags(char *buf, char *end, unsigned long flags,
>         return buf;
>  }
>
> +struct page_flags_fields {
> +       int width;
> +       int shift;
> +       int mask;
> +       const struct printf_spec *spec;
> +       const char *name;
> +};
> +
> +static const struct page_flags_fields pff[] = {
> +       {SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK,
> +        &default_dec_spec, "section"},
> +       {NODES_WIDTH, NODES_PGSHIFT, NODES_MASK,
> +        &default_dec_spec, "node"},
> +       {ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK,
> +        &default_dec_spec, "zone"},
> +       {LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK,
> +        &default_flag_spec, "lastcpupid"},
> +       {KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK,
> +        &default_flag_spec, "kasantag"},
> +};
> +
> +static
> +char *format_page_flags(char *buf, char *end, unsigned long flags)
> +{
> +       unsigned long main_flags = flags & (BIT(NR_PAGEFLAGS) - 1);
> +       bool append = false;
> +       int i;
> +
> +       /* Page flags from the main area. */
> +       if (main_flags) {
> +               buf = format_flags(buf, end, main_flags, pageflag_names);
> +               append = true;
> +       }
> +
> +       /* Page flags from the fields area */
> +       for (i = 0; i < ARRAY_SIZE(pff); i++) {
> +               /* Skip undefined fields. */
> +               if (!pff[i].width)
> +                       continue;
> +
> +               /* Format: Flag Name + '=' (equals sign) + Number + '|' (separator) */
> +               if (append) {
> +                       if (buf < end)
> +                               *buf = '|';
> +                       buf++;
> +               }
> +
> +               buf = string(buf, end, pff[i].name, *pff[i].spec);
> +               if (buf < end)
> +                       *buf = '=';
> +               buf++;
> +               buf = number(buf, end, (flags >> pff[i].shift) & pff[i].mask,
> +                            *pff[i].spec);
> +
> +               append = true;
> +       }
> +
> +       return buf;
> +}
> +
>  static noinline_for_stack
>  char *flags_string(char *buf, char *end, void *flags_ptr,
>                    struct printf_spec spec, const char *fmt)
> @@ -1928,11 +1988,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
>
>         switch (fmt[1]) {
>         case 'p':
> -               flags = *(unsigned long *)flags_ptr;
> -               /* Remove zone id */
> -               flags &= (1UL << NR_PAGEFLAGS) - 1;
> -               names = pageflag_names;
> -               break;
> +               return format_page_flags(buf, end, *(unsigned long *)flags_ptr);
>         case 'v':
>                 flags = *(unsigned long *)flags_ptr;
>                 names = vmaflag_names;
> --
> 2.18.2
>

Hello,

Any comments on this new version ?

-- 
Thanks
Yafang

Re: [PATCH v5 3/3] vsprintf: dump full information of page flags in pGp

[Petr Mladek]

Hello,

first, I am sorry for the late reply. I have marked the thread as
proceed by mistake last week...


On Mon 2021-02-15 23:51:41, Yafang Shao wrote:
> Currently the pGp only shows the names of page flags, rather than
> the full information including section, node, zone, last cpupid and
> kasan tag. While it is not easy to parse these information manually
> because there're so many flavors. Let's interpret them in pGp as well.
> 
> To be compitable with the existed format of pGp, the new introduced ones
> also use '|' as the separator, then the user tools parsing pGp won't
> need to make change, suggested by Matthew. The new information is
> tracked onto the end of the existed one.
> 
> One example of the output in mm/slub.c as follows,
> - Before the patch,
> [ 6343.396602] Slab 0x000000004382e02b objects=33 used=3 fp=0x000000009ae06ffc flags=0x17ffffc0010200(slab|head)
> 
> - After the patch,
> [ 8448.272530] Slab 0x0000000090797883 objects=33 used=3 fp=0x00000000790f1c26 flags=0x17ffffc0010200(slab|head|node=0|zone=2|lastcpupid=0x1fffff)
> 
> The documentation and test cases are also updated. The output of the
> test cases as follows,
> [11585.830272] test_printf: loaded.
> [11585.830454] test_printf: all 388 tests passed
> [11585.831401] test_printf: unloaded.
> 
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> +static
> +char *format_page_flags(char *buf, char *end, unsigned long flags)
> +{
> +	unsigned long main_flags = flags & (BIT(NR_PAGEFLAGS) - 1);
> +	bool append = false;
> +	int i;
> +
> +	/* Page flags from the main area. */
> +	if (main_flags) {
> +		buf = format_flags(buf, end, main_flags, pageflag_names);
> +		append = true;
> +	}
> +
> +	/* Page flags from the fields area */
> +	for (i = 0; i < ARRAY_SIZE(pff); i++) {
> +		/* Skip undefined fields. */
> +		if (!pff[i].width)
> +			continue;
> +
> +		/* Format: Flag Name + '=' (equals sign) + Number + '|' (separator) */
> +		if (append) {
> +			if (buf < end)
> +				*buf = '|';
> +			buf++;
> +		}
> +
> +		buf = string(buf, end, pff[i].name, *pff[i].spec);

I have found one more small issue.

The purpose of the flag-specific printk_spec is to define the format
how the value is printed. The name of the flag should be printed
using default_str_spec.

It works because the string is printed as-is with both
default_dec_spec and default_flag_spec. But it would be better
to use the string format.

> +		if (buf < end)
> +			*buf = '=';
> +		buf++;
> +		buf = number(buf, end, (flags >> pff[i].shift) & pff[i].mask,
> +			     *pff[i].spec);
> +
> +		append = true;
> +	}
> +
> +	return buf;
> +}

Otherwise, the patch looks to me. The issue is cosmetic and might be
fixed either by re-spinning just this patch or by a followup patch.
Either way, feel free to use:

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

Another question where to push this change. It is pity the we
finalized it in the middle of the merge window. It has to spend
at least few days in linux-next.

I would like to hear from Andy before I push it into linux-next.
There is still theoretical chance to get it into 5.12 when Linus
prolongs the merge window by one week. it has been delayed by
a long lasting power outage.

Best Regards,
Petr

Re: [PATCH v5 3/3] vsprintf: dump full information of page flags in pGp

[Matthew Wilcox]

On Mon, Feb 22, 2021 at 01:38:17PM +0100, Petr Mladek wrote:
> Another question where to push this change. It is pity the we
> finalized it in the middle of the merge window. It has to spend
> at least few days in linux-next.
> 
> I would like to hear from Andy before I push it into linux-next.
> There is still theoretical chance to get it into 5.12 when Linus
> prolongs the merge window by one week. it has been delayed by
> a long lasting power outage.

Usually new code has to be in linux-next by -rc5 or so.  This is
definitely too late for 5.12, but it's nice and early for 5.13!

Re: [PATCH v5 3/3] vsprintf: dump full information of page flags in pGp

[Petr Mladek]

On Mon 2021-02-22 13:39:56, Matthew Wilcox wrote:
> On Mon, Feb 22, 2021 at 01:38:17PM +0100, Petr Mladek wrote:
> > Another question where to push this change. It is pity the we
> > finalized it in the middle of the merge window. It has to spend
> > at least few days in linux-next.
> > 
> > I would like to hear from Andy before I push it into linux-next.
> > There is still theoretical chance to get it into 5.12 when Linus
> > prolongs the merge window by one week. it has been delayed by
> > a long lasting power outage.
> 
> Usually new code has to be in linux-next by -rc5 or so.  This is
> definitely too late for 5.12, but it's nice and early for 5.13!

OK, I am going to queue it for 5.13 the following week unless anyone
speaks against it in the meantime.

Best Regards,
Petr

Re: [PATCH v5 3/3] vsprintf: dump full information of page flags in pGp

[Petr Mladek]

On Wed 2021-03-03 16:43:25, Petr Mladek wrote:
> On Mon 2021-02-22 13:39:56, Matthew Wilcox wrote:
> > On Mon, Feb 22, 2021 at 01:38:17PM +0100, Petr Mladek wrote:
> > > Another question where to push this change. It is pity the we
> > > finalized it in the middle of the merge window. It has to spend
> > > at least few days in linux-next.
> > > 
> > > I would like to hear from Andy before I push it into linux-next.
> > > There is still theoretical chance to get it into 5.12 when Linus
> > > prolongs the merge window by one week. it has been delayed by
> > > a long lasting power outage.
> > 
> > Usually new code has to be in linux-next by -rc5 or so.  This is
> > definitely too late for 5.12, but it's nice and early for 5.13!
> 
> OK, I am going to queue it for 5.13 the following week unless anyone
> speaks against it in the meantime.

The patchset is committed in printk/linux.git,
branch for-5.13-vsprintf-pgp.

Best Regards,
Petr

Re: [PATCH v5 3/3] vsprintf: dump full information of page flags in pGp

[Yafang Shao]

On Mon, Feb 22, 2021 at 8:38 PM Petr Mladek <pmladek@suse.com> wrote:
>
> Hello,
>
> first, I am sorry for the late reply. I have marked the thread as
> proceed by mistake last week...
>
>
> On Mon 2021-02-15 23:51:41, Yafang Shao wrote:
> > Currently the pGp only shows the names of page flags, rather than
> > the full information including section, node, zone, last cpupid and
> > kasan tag. While it is not easy to parse these information manually
> > because there're so many flavors. Let's interpret them in pGp as well.
> >
> > To be compitable with the existed format of pGp, the new introduced ones
> > also use '|' as the separator, then the user tools parsing pGp won't
> > need to make change, suggested by Matthew. The new information is
> > tracked onto the end of the existed one.
> >
> > One example of the output in mm/slub.c as follows,
> > - Before the patch,
> > [ 6343.396602] Slab 0x000000004382e02b objects=33 used=3 fp=0x000000009ae06ffc flags=0x17ffffc0010200(slab|head)
> >
> > - After the patch,
> > [ 8448.272530] Slab 0x0000000090797883 objects=33 used=3 fp=0x00000000790f1c26 flags=0x17ffffc0010200(slab|head|node=0|zone=2|lastcpupid=0x1fffff)
> >
> > The documentation and test cases are also updated. The output of the
> > test cases as follows,
> > [11585.830272] test_printf: loaded.
> > [11585.830454] test_printf: all 388 tests passed
> > [11585.831401] test_printf: unloaded.
> >
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > +static
> > +char *format_page_flags(char *buf, char *end, unsigned long flags)
> > +{
> > +     unsigned long main_flags = flags & (BIT(NR_PAGEFLAGS) - 1);
> > +     bool append = false;
> > +     int i;
> > +
> > +     /* Page flags from the main area. */
> > +     if (main_flags) {
> > +             buf = format_flags(buf, end, main_flags, pageflag_names);
> > +             append = true;
> > +     }
> > +
> > +     /* Page flags from the fields area */
> > +     for (i = 0; i < ARRAY_SIZE(pff); i++) {
> > +             /* Skip undefined fields. */
> > +             if (!pff[i].width)
> > +                     continue;
> > +
> > +             /* Format: Flag Name + '=' (equals sign) + Number + '|' (separator) */
> > +             if (append) {
> > +                     if (buf < end)
> > +                             *buf = '|';
> > +                     buf++;
> > +             }
> > +
> > +             buf = string(buf, end, pff[i].name, *pff[i].spec);
>
> I have found one more small issue.
>
> The purpose of the flag-specific printk_spec is to define the format
> how the value is printed. The name of the flag should be printed
> using default_str_spec.
>
> It works because the string is printed as-is with both
> default_dec_spec and default_flag_spec. But it would be better
> to use the string format.
>

Thanks for the explanation.

> > +             if (buf < end)
> > +                     *buf = '=';
> > +             buf++;
> > +             buf = number(buf, end, (flags >> pff[i].shift) & pff[i].mask,
> > +                          *pff[i].spec);
> > +
> > +             append = true;
> > +     }
> > +
> > +     return buf;
> > +}
>
> Otherwise, the patch looks to me. The issue is cosmetic and might be
> fixed either by re-spinning just this patch or by a followup patch.

I will send a separate followup patch.

> Either way, feel free to use:
>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
>

Thanks

> Another question where to push this change. It is pity the we
> finalized it in the middle of the merge window. It has to spend
> at least few days in linux-next.
>
> I would like to hear from Andy before I push it into linux-next.
> There is still theoretical chance to get it into 5.12 when Linus
> prolongs the merge window by one week. it has been delayed by
> a long lasting power outage.
>
> Best Regards,
> Petr



-- 
Thanks
Yafang