linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] mm, printk: dump full information of page flags in pGp
@ 2021-01-28  2:19 Yafang Shao
  2021-01-28  2:19 ` [PATCH 1/3] mm, slub: use pGp to print page flags Yafang Shao
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Yafang Shao @ 2021-01-28  2:19 UTC (permalink / raw)
  To: david, vbabka, cl, penberg, rientjes, iamjoonsoo.kim, akpm,
	pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, linux
  Cc: linux-mm, linux-kernel, 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.

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

Below is the example of the output in mm/slub.c.
- Before the patchset
[ 6155.716018] INFO: Slab 0x000000004027dd4f objects=33 used=3 fp=0x000000008cd1579c flags=0x17ffffc0010200

- After the patchset
[ 6315.235783] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(Node 0x0,Zone 0x2,Lastcpupid 0x1fffff,slab|head)

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

 lib/vsprintf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 mm/slub.c      | 13 +++++++------
 2 files changed, 48 insertions(+), 7 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/3] mm, slub: use pGp to print page flags
  2021-01-28  2:19 [PATCH 0/3] mm, printk: dump full information of page flags in pGp Yafang Shao
@ 2021-01-28  2:19 ` Yafang Shao
  2021-01-28 10:23   ` Vlastimil Babka
  2021-01-28 22:42   ` David Rientjes
  2021-01-28  2:19 ` [PATCH 2/3] mm, slub: don't combine pr_err with INFO Yafang Shao
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 20+ messages in thread
From: Yafang Shao @ 2021-01-28  2:19 UTC (permalink / raw)
  To: david, vbabka, cl, penberg, rientjes, iamjoonsoo.kim, akpm,
	pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, linux
  Cc: linux-mm, linux-kernel, 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)

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 mm/slub.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 69742ab9a21d..4b9ab267afbc 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.17.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/3] mm, slub: don't combine pr_err with INFO
  2021-01-28  2:19 [PATCH 0/3] mm, printk: dump full information of page flags in pGp Yafang Shao
  2021-01-28  2:19 ` [PATCH 1/3] mm, slub: use pGp to print page flags Yafang Shao
@ 2021-01-28  2:19 ` Yafang Shao
  2021-01-28 10:35   ` Vlastimil Babka
  2021-01-28  2:19 ` [PATCH 3/3] printk: dump full information of page flags in pGp Yafang Shao
  2021-01-28 12:12 ` [PATCH 0/3] mm, " Andy Shevchenko
  3 siblings, 1 reply; 20+ messages in thread
From: Yafang Shao @ 2021-01-28  2:19 UTC (permalink / raw)
  To: david, vbabka, cl, penberg, rientjes, iamjoonsoo.kim, akpm,
	pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, linux
  Cc: linux-mm, linux-kernel, Yafang Shao

It is strange to combine "pr_err" with "INFO", so let's clean them up.
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
[ 6312.639698] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(slab|head)

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

Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 mm/slub.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 4b9ab267afbc..18b4474c8fa2 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("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("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("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("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("ERR: Object 0x%p @offset=%tu\n", p, p - addr);
 			print_tracking(s, p);
 		}
 	}
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/3] printk: dump full information of page flags in pGp
  2021-01-28  2:19 [PATCH 0/3] mm, printk: dump full information of page flags in pGp Yafang Shao
  2021-01-28  2:19 ` [PATCH 1/3] mm, slub: use pGp to print page flags Yafang Shao
  2021-01-28  2:19 ` [PATCH 2/3] mm, slub: don't combine pr_err with INFO Yafang Shao
@ 2021-01-28  2:19 ` Yafang Shao
  2021-01-28  2:35   ` Joe Perches
                     ` (3 more replies)
  2021-01-28 12:12 ` [PATCH 0/3] mm, " Andy Shevchenko
  3 siblings, 4 replies; 20+ messages in thread
From: Yafang Shao @ 2021-01-28  2:19 UTC (permalink / raw)
  To: david, vbabka, cl, penberg, rientjes, iamjoonsoo.kim, akpm,
	pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, linux
  Cc: linux-mm, linux-kernel, 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.

- Before the patch,
[ 6312.639698] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(slab|head)

- After the patch,
[ 6315.235783] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(Node 0x0,Zone 0x2,Lastcpupid 0x1fffff,slab|head)

Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 lib/vsprintf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3b53c73580c5..bd809f4f1b82 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1916,6 +1916,46 @@ char *format_flags(char *buf, char *end, unsigned long flags,
 	return buf;
 }
 
+struct page_flags_layout {
+	int width;
+	int shift;
+	int mask;
+	char *name;
+};
+
+struct page_flags_layout pfl[] = {
+	{SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK, "Section "},
+	{NODES_WIDTH, NODES_PGSHIFT, NODES_MASK, "Node "},
+	{ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK, "Zone "},
+	{LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK, "Lastcpupid "},
+	{KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK, "Kasantag "},
+};
+
+static
+char *format_layout(char *buf, char *end, unsigned long flags)
+{
+	int i;
+
+	for (i = 0; i < sizeof(pfl) / sizeof(struct page_flags_layout) && buf < end; i++) {
+		if (pfl[i].width == 0)
+			continue;
+
+		buf = string(buf, end, pfl[i].name, default_str_spec);
+
+		if (buf >= end)
+			break;
+		buf = number(buf, end, (flags >> pfl[i].shift) & pfl[i].mask,
+			     default_flag_spec);
+
+		if (buf >= end)
+			break;
+		*buf = ',';
+		buf++;
+	}
+
+	return buf;
+}
+
 static noinline_for_stack
 char *flags_string(char *buf, char *end, void *flags_ptr,
 		   struct printf_spec spec, const char *fmt)
@@ -1929,7 +1969,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
 	switch (fmt[1]) {
 	case 'p':
 		flags = *(unsigned long *)flags_ptr;
-		/* Remove zone id */
+		buf = format_layout(buf, end, flags & ~((1UL << NR_PAGEFLAGS) - 1));
 		flags &= (1UL << NR_PAGEFLAGS) - 1;
 		names = pageflag_names;
 		break;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] printk: dump full information of page flags in pGp
  2021-01-28  2:19 ` [PATCH 3/3] printk: dump full information of page flags in pGp Yafang Shao
@ 2021-01-28  2:35   ` Joe Perches
  2021-01-28  7:42     ` Yafang Shao
  2021-01-28  2:52   ` Miaohe Lin
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 20+ messages in thread
From: Joe Perches @ 2021-01-28  2:35 UTC (permalink / raw)
  To: Yafang Shao, david, vbabka, cl, penberg, rientjes,
	iamjoonsoo.kim, akpm, pmladek, rostedt, sergey.senozhatsky,
	andriy.shevchenko, linux
  Cc: linux-mm, linux-kernel

On Thu, 2021-01-28 at 10:19 +0800, 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.
[]
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
[]
> @@ -1916,6 +1916,46 @@ char *format_flags(char *buf, char *end, unsigned long flags,
>  	return buf;
>  }
> 
> +struct page_flags_layout {
> +	int width;
> +	int shift;
> +	int mask;
> +	char *name;
> +};
> +
> +struct page_flags_layout pfl[] = {

static const struct page_flags_layout pfl[] = {

> +	{SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK, "Section "},
> +	{NODES_WIDTH, NODES_PGSHIFT, NODES_MASK, "Node "},
> +	{ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK, "Zone "},
> +	{LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK, "Lastcpupid "},
> +	{KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK, "Kasantag "},
> +};
> +
> +static
> +char *format_layout(char *buf, char *end, unsigned long flags)

poor name.  perhaps format_page_flags

> +{
> +	int i;
> +
> +	for (i = 0; i < sizeof(pfl) / sizeof(struct page_flags_layout) && buf < end; i++) {

	for (i = 0; i < ARRAY_SIZE(pfl) && buf < end; i++) {


> @@ -1929,7 +1969,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
>  	switch (fmt[1]) {
>  	case 'p':
>  		flags = *(unsigned long *)flags_ptr;
> -		/* Remove zone id */
> +		buf = format_layout(buf, end, flags & ~((1UL << NR_PAGEFLAGS) - 1));
>  		flags &= (1UL << NR_PAGEFLAGS) - 1;

Perhaps store the bitshift into a temp and use the temp twice

		foo = BIT(NR_PAGEFLAGS) - 1;

		buf = format_layout(buf, end, flags & ~foo);
		flags &= foo;




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] printk: dump full information of page flags in pGp
  2021-01-28  2:19 ` [PATCH 3/3] printk: dump full information of page flags in pGp Yafang Shao
  2021-01-28  2:35   ` Joe Perches
@ 2021-01-28  2:52   ` Miaohe Lin
  2021-01-28  7:44     ` Yafang Shao
  2021-01-28 10:42   ` Vlastimil Babka
  2021-01-28 12:11   ` Andy Shevchenko
  3 siblings, 1 reply; 20+ messages in thread
From: Miaohe Lin @ 2021-01-28  2:52 UTC (permalink / raw)
  To: Yafang Shao
  Cc: linux-mm, linux-kernel, david, vbabka, cl, penberg, rientjes,
	iamjoonsoo.kim, akpm, pmladek, rostedt, sergey.senozhatsky,
	andriy.shevchenko, linux

Hi:
On 2021/1/28 10:19, 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.
> 
> - Before the patch,
> [ 6312.639698] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(slab|head)
> 
> - After the patch,
> [ 6315.235783] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(Node 0x0,Zone 0x2,Lastcpupid 0x1fffff,slab|head)
> 

Thanks. This really helps!

> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  lib/vsprintf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 3b53c73580c5..bd809f4f1b82 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1916,6 +1916,46 @@ char *format_flags(char *buf, char *end, unsigned long flags,
>  	return buf;
>  }
>  
> +struct page_flags_layout {
> +	int width;
> +	int shift;
> +	int mask;
> +	char *name;

Should we add const for name ?

> +};
> +
> +struct page_flags_layout pfl[] = {

Should we add static const for pfl[] as we won't change its value and use it outside this file ?

> +	{SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK, "Section "},
> +	{NODES_WIDTH, NODES_PGSHIFT, NODES_MASK, "Node "},
> +	{ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK, "Zone "},
> +	{LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK, "Lastcpupid "},
> +	{KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK, "Kasantag "},
> +};
> +
> +static
> +char *format_layout(char *buf, char *end, unsigned long flags)
> +{
> +	int i;
> +
> +	for (i = 0; i < sizeof(pfl) / sizeof(struct page_flags_layout) && buf < end; i++) {

I think we can use ARRAY_SIZE here.

> +		if (pfl[i].width == 0)
> +			continue;
> +
> +		buf = string(buf, end, pfl[i].name, default_str_spec);
> +
> +		if (buf >= end)
> +			break;
> +		buf = number(buf, end, (flags >> pfl[i].shift) & pfl[i].mask,
> +			     default_flag_spec);
> +
> +		if (buf >= end)
> +			break;
> +		*buf = ',';
> +		buf++;
> +	}
> +
> +	return buf;
> +}
> +
>  static noinline_for_stack
>  char *flags_string(char *buf, char *end, void *flags_ptr,
>  		   struct printf_spec spec, const char *fmt)
> @@ -1929,7 +1969,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
>  	switch (fmt[1]) {
>  	case 'p':
>  		flags = *(unsigned long *)flags_ptr;
> -		/* Remove zone id */
> +		buf = format_layout(buf, end, flags & ~((1UL << NR_PAGEFLAGS) - 1));
>  		flags &= (1UL << NR_PAGEFLAGS) - 1;
>  		names = pageflag_names;
>  		break;
>
Many thanks.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] printk: dump full information of page flags in pGp
  2021-01-28  2:35   ` Joe Perches
@ 2021-01-28  7:42     ` Yafang Shao
  0 siblings, 0 replies; 20+ messages in thread
From: Yafang Shao @ 2021-01-28  7:42 UTC (permalink / raw)
  To: Joe Perches
  Cc: David Hildenbrand, Vlastimil Babka, Christoph Lameter, penberg,
	David Rientjes, iamjoonsoo.kim, Andrew Morton, Petr Mladek,
	Steven Rostedt, Sergey Senozhatsky, andriy.shevchenko,
	Rasmus Villemoes, Linux MM, LKML

On Thu, Jan 28, 2021 at 10:35 AM Joe Perches <joe@perches.com> wrote:
>
> On Thu, 2021-01-28 at 10:19 +0800, 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.
> []
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> []
> > @@ -1916,6 +1916,46 @@ char *format_flags(char *buf, char *end, unsigned long flags,
> >       return buf;
> >  }
> >
> > +struct page_flags_layout {
> > +     int width;
> > +     int shift;
> > +     int mask;
> > +     char *name;
> > +};
> > +
> > +struct page_flags_layout pfl[] = {
>
> static const struct page_flags_layout pfl[] = {

Sure.

>
> > +     {SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK, "Section "},
> > +     {NODES_WIDTH, NODES_PGSHIFT, NODES_MASK, "Node "},
> > +     {ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK, "Zone "},
> > +     {LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK, "Lastcpupid "},
> > +     {KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK, "Kasantag "},
> > +};
> > +
> > +static
> > +char *format_layout(char *buf, char *end, unsigned long flags)
>
> poor name.  perhaps format_page_flags
>

Thanks for the suggestion.

> > +{
> > +     int i;
> > +
> > +     for (i = 0; i < sizeof(pfl) / sizeof(struct page_flags_layout) && buf < end; i++) {
>
>         for (i = 0; i < ARRAY_SIZE(pfl) && buf < end; i++) {
>

Sure.


>
> > @@ -1929,7 +1969,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
> >       switch (fmt[1]) {
> >       case 'p':
> >               flags = *(unsigned long *)flags_ptr;
> > -             /* Remove zone id */
> > +             buf = format_layout(buf, end, flags & ~((1UL << NR_PAGEFLAGS) - 1));
> >               flags &= (1UL << NR_PAGEFLAGS) - 1;
>
> Perhaps store the bitshift into a temp and use the temp twice
>
>                 foo = BIT(NR_PAGEFLAGS) - 1;
>
>                 buf = format_layout(buf, end, flags & ~foo);
>                 flags &= foo;
>
>

Thanks for the suggestion. I will change them all.


-- 
Thanks
Yafang


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] printk: dump full information of page flags in pGp
  2021-01-28  2:52   ` Miaohe Lin
@ 2021-01-28  7:44     ` Yafang Shao
  0 siblings, 0 replies; 20+ messages in thread
From: Yafang Shao @ 2021-01-28  7:44 UTC (permalink / raw)
  To: Miaohe Lin
  Cc: Linux MM, LKML, David Hildenbrand, Vlastimil Babka,
	Christoph Lameter, penberg, David Rientjes, iamjoonsoo.kim,
	Andrew Morton, Petr Mladek, Steven Rostedt, Sergey Senozhatsky,
	andriy.shevchenko, Rasmus Villemoes

On Thu, Jan 28, 2021 at 10:52 AM Miaohe Lin <linmiaohe@huawei.com> wrote:
>
> Hi:
> On 2021/1/28 10:19, 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.
> >
> > - Before the patch,
> > [ 6312.639698] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(slab|head)
> >
> > - After the patch,
> > [ 6315.235783] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(Node 0x0,Zone 0x2,Lastcpupid 0x1fffff,slab|head)
> >
>
> Thanks. This really helps!
>
> > Cc: David Hildenbrand <david@redhat.com>
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> >  lib/vsprintf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 41 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 3b53c73580c5..bd809f4f1b82 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1916,6 +1916,46 @@ char *format_flags(char *buf, char *end, unsigned long flags,
> >       return buf;
> >  }
> >
> > +struct page_flags_layout {
> > +     int width;
> > +     int shift;
> > +     int mask;
> > +     char *name;
>
> Should we add const for name ?
>

Good suggestion.

> > +};
> > +
> > +struct page_flags_layout pfl[] = {
>
> Should we add static const for pfl[] as we won't change its value and use it outside this file ?
>

Sure.

> > +     {SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK, "Section "},
> > +     {NODES_WIDTH, NODES_PGSHIFT, NODES_MASK, "Node "},
> > +     {ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK, "Zone "},
> > +     {LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK, "Lastcpupid "},
> > +     {KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK, "Kasantag "},
> > +};
> > +
> > +static
> > +char *format_layout(char *buf, char *end, unsigned long flags)
> > +{
> > +     int i;
> > +
> > +     for (i = 0; i < sizeof(pfl) / sizeof(struct page_flags_layout) && buf < end; i++) {
>
> I think we can use ARRAY_SIZE here.
>

Sure.

> > +             if (pfl[i].width == 0)
> > +                     continue;
> > +
> > +             buf = string(buf, end, pfl[i].name, default_str_spec);
> > +
> > +             if (buf >= end)
> > +                     break;
> > +             buf = number(buf, end, (flags >> pfl[i].shift) & pfl[i].mask,
> > +                          default_flag_spec);
> > +
> > +             if (buf >= end)
> > +                     break;
> > +             *buf = ',';
> > +             buf++;
> > +     }
> > +
> > +     return buf;
> > +}
> > +
> >  static noinline_for_stack
> >  char *flags_string(char *buf, char *end, void *flags_ptr,
> >                  struct printf_spec spec, const char *fmt)
> > @@ -1929,7 +1969,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
> >       switch (fmt[1]) {
> >       case 'p':
> >               flags = *(unsigned long *)flags_ptr;
> > -             /* Remove zone id */
> > +             buf = format_layout(buf, end, flags & ~((1UL << NR_PAGEFLAGS) - 1));
> >               flags &= (1UL << NR_PAGEFLAGS) - 1;
> >               names = pageflag_names;
> >               break;
> >
> Many thanks.



-- 
Thanks
Yafang


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] mm, slub: use pGp to print page flags
  2021-01-28  2:19 ` [PATCH 1/3] mm, slub: use pGp to print page flags Yafang Shao
@ 2021-01-28 10:23   ` Vlastimil Babka
  2021-01-28 22:42   ` David Rientjes
  1 sibling, 0 replies; 20+ messages in thread
From: Vlastimil Babka @ 2021-01-28 10:23 UTC (permalink / raw)
  To: Yafang Shao, david, cl, penberg, rientjes, iamjoonsoo.kim, akpm,
	pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, linux
  Cc: linux-mm, linux-kernel

On 1/28/21 3:19 AM, Yafang Shao wrote:
> 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)
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>

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

> ---
>  mm/slub.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 69742ab9a21d..4b9ab267afbc 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);
>  
>  }
>  
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/3] mm, slub: don't combine pr_err with INFO
  2021-01-28  2:19 ` [PATCH 2/3] mm, slub: don't combine pr_err with INFO Yafang Shao
@ 2021-01-28 10:35   ` Vlastimil Babka
  2021-01-28 13:06     ` Yafang Shao
  0 siblings, 1 reply; 20+ messages in thread
From: Vlastimil Babka @ 2021-01-28 10:35 UTC (permalink / raw)
  To: Yafang Shao, david, cl, penberg, rientjes, iamjoonsoo.kim, akpm,
	pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, linux
  Cc: linux-mm, linux-kernel

On 1/28/21 3:19 AM, Yafang Shao wrote:
> It is strange to combine "pr_err" with "INFO", so let's clean them up.
> 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
> [ 6312.639698] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(slab|head)
> 
> [1]. https://lore.kernel.org/linux-mm/b9c0f2b6-e9b0-0c36-ebdd-2bc684c5a762@redhat.com/#t
> 
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>

These are usually printed as part of slab_bug() with its prominent banner. In
that sense it's additional details, thus INFO. The details itself are not error,
thus ERR makes little sense imho. How about removing the prefix completely, or
just replacing with an ident to make it visually part of the BUG report.

> ---
>  mm/slub.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 4b9ab267afbc..18b4474c8fa2 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("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("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("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("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("ERR: Object 0x%p @offset=%tu\n", p, p - addr);
>  			print_tracking(s, p);
>  		}
>  	}
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] printk: dump full information of page flags in pGp
  2021-01-28  2:19 ` [PATCH 3/3] printk: dump full information of page flags in pGp Yafang Shao
  2021-01-28  2:35   ` Joe Perches
  2021-01-28  2:52   ` Miaohe Lin
@ 2021-01-28 10:42   ` Vlastimil Babka
  2021-01-28 13:07     ` Yafang Shao
  2021-01-28 12:11   ` Andy Shevchenko
  3 siblings, 1 reply; 20+ messages in thread
From: Vlastimil Babka @ 2021-01-28 10:42 UTC (permalink / raw)
  To: Yafang Shao, david, cl, penberg, rientjes, iamjoonsoo.kim, akpm,
	pmladek, rostedt, sergey.senozhatsky, andriy.shevchenko, linux
  Cc: linux-mm, linux-kernel

On 1/28/21 3:19 AM, 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.
> 
> - Before the patch,
> [ 6312.639698] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(slab|head)
> 
> - After the patch,
> [ 6315.235783] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(Node 0x0,Zone 0x2,Lastcpupid 0x1fffff,slab|head)

node, zone could be decimal?
Thanks

> 
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  lib/vsprintf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 3b53c73580c5..bd809f4f1b82 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1916,6 +1916,46 @@ char *format_flags(char *buf, char *end, unsigned long flags,
>  	return buf;
>  }
>  
> +struct page_flags_layout {
> +	int width;
> +	int shift;
> +	int mask;
> +	char *name;
> +};
> +
> +struct page_flags_layout pfl[] = {
> +	{SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK, "Section "},
> +	{NODES_WIDTH, NODES_PGSHIFT, NODES_MASK, "Node "},
> +	{ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK, "Zone "},
> +	{LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK, "Lastcpupid "},
> +	{KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK, "Kasantag "},
> +};
> +
> +static
> +char *format_layout(char *buf, char *end, unsigned long flags)
> +{
> +	int i;
> +
> +	for (i = 0; i < sizeof(pfl) / sizeof(struct page_flags_layout) && buf < end; i++) {
> +		if (pfl[i].width == 0)
> +			continue;
> +
> +		buf = string(buf, end, pfl[i].name, default_str_spec);
> +
> +		if (buf >= end)
> +			break;
> +		buf = number(buf, end, (flags >> pfl[i].shift) & pfl[i].mask,
> +			     default_flag_spec);
> +
> +		if (buf >= end)
> +			break;
> +		*buf = ',';
> +		buf++;
> +	}
> +
> +	return buf;
> +}
> +
>  static noinline_for_stack
>  char *flags_string(char *buf, char *end, void *flags_ptr,
>  		   struct printf_spec spec, const char *fmt)
> @@ -1929,7 +1969,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
>  	switch (fmt[1]) {
>  	case 'p':
>  		flags = *(unsigned long *)flags_ptr;
> -		/* Remove zone id */
> +		buf = format_layout(buf, end, flags & ~((1UL << NR_PAGEFLAGS) - 1));
>  		flags &= (1UL << NR_PAGEFLAGS) - 1;
>  		names = pageflag_names;
>  		break;
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] printk: dump full information of page flags in pGp
  2021-01-28  2:19 ` [PATCH 3/3] printk: dump full information of page flags in pGp Yafang Shao
                     ` (2 preceding siblings ...)
  2021-01-28 10:42   ` Vlastimil Babka
@ 2021-01-28 12:11   ` Andy Shevchenko
  2021-01-28 13:18     ` Yafang Shao
  3 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2021-01-28 12:11 UTC (permalink / raw)
  To: Yafang Shao
  Cc: david, vbabka, cl, penberg, rientjes, iamjoonsoo.kim, akpm,
	pmladek, rostedt, sergey.senozhatsky, linux, linux-mm,
	linux-kernel

On Thu, Jan 28, 2021 at 10:19:47AM +0800, 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.
> 
> - Before the patch,
> [ 6312.639698] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(slab|head)
> 
> - After the patch,
> [ 6315.235783] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(Node 0x0,Zone 0x2,Lastcpupid 0x1fffff,slab|head)

> +	int i;
> +
> +	for (i = 0; i < sizeof(pfl) / sizeof(struct page_flags_layout) && buf < end; i++) {

'buf < end' is redundant.

> +		if (pfl[i].width == 0)
> +			continue;
> +
> +		buf = string(buf, end, pfl[i].name, default_str_spec);

> +		if (buf >= end)
> +			break;

Can you rather use usual patter, i.e.

	if (buf < end) {
		...do something...
	}
	buf++; // or whatever increase should be done

Moreover, number() and string() IIRC have the proper checks embedded into them.

> +		buf = number(buf, end, (flags >> pfl[i].shift) & pfl[i].mask,
> +			     default_flag_spec);

> +		if (buf >= end)
> +			break;
> +		*buf = ',';
> +		buf++;

Here is a very standard pattern can be used, see code around

		if (buf < end)
			*buf = ',';
		buf++;

> +	}

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] mm, printk: dump full information of page flags in pGp
  2021-01-28  2:19 [PATCH 0/3] mm, printk: dump full information of page flags in pGp Yafang Shao
                   ` (2 preceding siblings ...)
  2021-01-28  2:19 ` [PATCH 3/3] printk: dump full information of page flags in pGp Yafang Shao
@ 2021-01-28 12:12 ` Andy Shevchenko
  2021-01-28 13:19   ` Yafang Shao
  3 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2021-01-28 12:12 UTC (permalink / raw)
  To: Yafang Shao
  Cc: david, vbabka, cl, penberg, rientjes, iamjoonsoo.kim, akpm,
	pmladek, rostedt, sergey.senozhatsky, linux, linux-mm,
	linux-kernel

On Thu, Jan 28, 2021 at 10:19:44AM +0800, 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.
> 
> This patchset also includes some code cleanup in mm/slub.c.
> 
> Below is the example of the output in mm/slub.c.
> - Before the patchset
> [ 6155.716018] INFO: Slab 0x000000004027dd4f objects=33 used=3 fp=0x000000008cd1579c flags=0x17ffffc0010200
> 
> - After the patchset
> [ 6315.235783] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(Node 0x0,Zone 0x2,Lastcpupid 0x1fffff,slab|head)


Please, add a corresponding test cases to test_printf.c. W/o test cases NAK.

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/3] mm, slub: don't combine pr_err with INFO
  2021-01-28 10:35   ` Vlastimil Babka
@ 2021-01-28 13:06     ` Yafang Shao
  0 siblings, 0 replies; 20+ messages in thread
From: Yafang Shao @ 2021-01-28 13:06 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: David Hildenbrand, Christoph Lameter, penberg, David Rientjes,
	iamjoonsoo.kim, Andrew Morton, Petr Mladek, Steven Rostedt,
	Sergey Senozhatsky, andriy.shevchenko, Rasmus Villemoes,
	Linux MM, LKML

On Thu, Jan 28, 2021 at 6:35 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 1/28/21 3:19 AM, Yafang Shao wrote:
> > It is strange to combine "pr_err" with "INFO", so let's clean them up.
> > 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
> > [ 6312.639698] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(slab|head)
> >
> > [1]. https://lore.kernel.org/linux-mm/b9c0f2b6-e9b0-0c36-ebdd-2bc684c5a762@redhat.com/#t
> >
> > Cc: David Hildenbrand <david@redhat.com>
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
>
> These are usually printed as part of slab_bug() with its prominent banner. In
> that sense it's additional details, thus INFO. The details itself are not error,
> thus ERR makes little sense imho. How about removing the prefix completely, or
> just replacing with an ident to make it visually part of the BUG report.
>

Thanks for the explanation. I will remove the prefix completely in the
next version.

> > ---
> >  mm/slub.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 4b9ab267afbc..18b4474c8fa2 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("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("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("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("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("ERR: Object 0x%p @offset=%tu\n", p, p - addr);
> >                       print_tracking(s, p);
> >               }
> >       }
> >
>


-- 
Thanks
Yafang


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] printk: dump full information of page flags in pGp
  2021-01-28 10:42   ` Vlastimil Babka
@ 2021-01-28 13:07     ` Yafang Shao
  0 siblings, 0 replies; 20+ messages in thread
From: Yafang Shao @ 2021-01-28 13:07 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: David Hildenbrand, Christoph Lameter, penberg, David Rientjes,
	iamjoonsoo.kim, Andrew Morton, Petr Mladek, Steven Rostedt,
	Sergey Senozhatsky, andriy.shevchenko, Rasmus Villemoes,
	Linux MM, LKML

On Thu, Jan 28, 2021 at 6:42 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 1/28/21 3:19 AM, 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.
> >
> > - Before the patch,
> > [ 6312.639698] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(slab|head)
> >
> > - After the patch,
> > [ 6315.235783] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(Node 0x0,Zone 0x2,Lastcpupid 0x1fffff,slab|head)
>
> node, zone could be decimal?

Make sense to me. Will change it.

>
> >
> > Cc: David Hildenbrand <david@redhat.com>
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> >  lib/vsprintf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 41 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 3b53c73580c5..bd809f4f1b82 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1916,6 +1916,46 @@ char *format_flags(char *buf, char *end, unsigned long flags,
> >       return buf;
> >  }
> >
> > +struct page_flags_layout {
> > +     int width;
> > +     int shift;
> > +     int mask;
> > +     char *name;
> > +};
> > +
> > +struct page_flags_layout pfl[] = {
> > +     {SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK, "Section "},
> > +     {NODES_WIDTH, NODES_PGSHIFT, NODES_MASK, "Node "},
> > +     {ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK, "Zone "},
> > +     {LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK, "Lastcpupid "},
> > +     {KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK, "Kasantag "},
> > +};
> > +
> > +static
> > +char *format_layout(char *buf, char *end, unsigned long flags)
> > +{
> > +     int i;
> > +
> > +     for (i = 0; i < sizeof(pfl) / sizeof(struct page_flags_layout) && buf < end; i++) {
> > +             if (pfl[i].width == 0)
> > +                     continue;
> > +
> > +             buf = string(buf, end, pfl[i].name, default_str_spec);
> > +
> > +             if (buf >= end)
> > +                     break;
> > +             buf = number(buf, end, (flags >> pfl[i].shift) & pfl[i].mask,
> > +                          default_flag_spec);
> > +
> > +             if (buf >= end)
> > +                     break;
> > +             *buf = ',';
> > +             buf++;
> > +     }
> > +
> > +     return buf;
> > +}
> > +
> >  static noinline_for_stack
> >  char *flags_string(char *buf, char *end, void *flags_ptr,
> >                  struct printf_spec spec, const char *fmt)
> > @@ -1929,7 +1969,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
> >       switch (fmt[1]) {
> >       case 'p':
> >               flags = *(unsigned long *)flags_ptr;
> > -             /* Remove zone id */
> > +             buf = format_layout(buf, end, flags & ~((1UL << NR_PAGEFLAGS) - 1));
> >               flags &= (1UL << NR_PAGEFLAGS) - 1;
> >               names = pageflag_names;
> >               break;
> >
>


-- 
Thanks
Yafang


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] printk: dump full information of page flags in pGp
  2021-01-28 12:11   ` Andy Shevchenko
@ 2021-01-28 13:18     ` Yafang Shao
  2021-01-28 14:50       ` Andy Shevchenko
  0 siblings, 1 reply; 20+ messages in thread
From: Yafang Shao @ 2021-01-28 13:18 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: David Hildenbrand, Vlastimil Babka, Christoph Lameter, penberg,
	David Rientjes, iamjoonsoo.kim, Andrew Morton, Petr Mladek,
	Steven Rostedt, Sergey Senozhatsky, Rasmus Villemoes, Linux MM,
	LKML

On Thu, Jan 28, 2021 at 8:11 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Jan 28, 2021 at 10:19:47AM +0800, 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.
> >
> > - Before the patch,
> > [ 6312.639698] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(slab|head)
> >
> > - After the patch,
> > [ 6315.235783] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(Node 0x0,Zone 0x2,Lastcpupid 0x1fffff,slab|head)
>
> > +     int i;
> > +
> > +     for (i = 0; i < sizeof(pfl) / sizeof(struct page_flags_layout) && buf < end; i++) {
>
> 'buf < end' is redundant.
>

Thanks for pointing this out.

> > +             if (pfl[i].width == 0)
> > +                     continue;
> > +
> > +             buf = string(buf, end, pfl[i].name, default_str_spec);
>
> > +             if (buf >= end)
> > +                     break;
>
> Can you rather use usual patter, i.e.
>
>         if (buf < end) {
>                 ...do something...
>         }
>         buf++; // or whatever increase should be done
>
> Moreover, number() and string() IIRC have the proper checks embedded into them.
>

I will take a look at the detail in these two functions.

> > +             buf = number(buf, end, (flags >> pfl[i].shift) & pfl[i].mask,
> > +                          default_flag_spec);
>
> > +             if (buf >= end)
> > +                     break;
> > +             *buf = ',';
> > +             buf++;
>
> Here is a very standard pattern can be used, see code around
>
>                 if (buf < end)
>                         *buf = ',';
>                 buf++;
>
> > +     }
>

Thanks for the explanation.
I will change it as you suggested.


-- 
Thanks
Yafang


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/3] mm, printk: dump full information of page flags in pGp
  2021-01-28 12:12 ` [PATCH 0/3] mm, " Andy Shevchenko
@ 2021-01-28 13:19   ` Yafang Shao
  0 siblings, 0 replies; 20+ messages in thread
From: Yafang Shao @ 2021-01-28 13:19 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: David Hildenbrand, Vlastimil Babka, Christoph Lameter, penberg,
	David Rientjes, iamjoonsoo.kim, Andrew Morton, Petr Mladek,
	Steven Rostedt, Sergey Senozhatsky, Rasmus Villemoes, Linux MM,
	LKML

On Thu, Jan 28, 2021 at 8:12 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Jan 28, 2021 at 10:19:44AM +0800, 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.
> >
> > This patchset also includes some code cleanup in mm/slub.c.
> >
> > Below is the example of the output in mm/slub.c.
> > - Before the patchset
> > [ 6155.716018] INFO: Slab 0x000000004027dd4f objects=33 used=3 fp=0x000000008cd1579c flags=0x17ffffc0010200
> >
> > - After the patchset
> > [ 6315.235783] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(Node 0x0,Zone 0x2,Lastcpupid 0x1fffff,slab|head)
>
>
> Please, add a corresponding test cases to test_printf.c. W/o test cases NAK.
>

Sure. Will add the test cases in the next version.


-- 
Thanks
Yafang


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] printk: dump full information of page flags in pGp
  2021-01-28 13:18     ` Yafang Shao
@ 2021-01-28 14:50       ` Andy Shevchenko
  2021-01-29  1:21         ` Yafang Shao
  0 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2021-01-28 14:50 UTC (permalink / raw)
  To: Yafang Shao
  Cc: David Hildenbrand, Vlastimil Babka, Christoph Lameter, penberg,
	David Rientjes, iamjoonsoo.kim, Andrew Morton, Petr Mladek,
	Steven Rostedt, Sergey Senozhatsky, Rasmus Villemoes, Linux MM,
	LKML

On Thu, Jan 28, 2021 at 09:18:24PM +0800, Yafang Shao wrote:
> On Thu, Jan 28, 2021 at 8:11 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:

...

> Thanks for the explanation.
> I will change it as you suggested.

You are welcome!

Just note, that kasprintf() should work for this as well as for the rest
printf() cases. That's why it's very important to return proper buffer pointer.

Also read `man snprintf(3)` RETURN VALUE section to understand it.

-- 
With Best Regards,
Andy Shevchenko




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/3] mm, slub: use pGp to print page flags
  2021-01-28  2:19 ` [PATCH 1/3] mm, slub: use pGp to print page flags Yafang Shao
  2021-01-28 10:23   ` Vlastimil Babka
@ 2021-01-28 22:42   ` David Rientjes
  1 sibling, 0 replies; 20+ messages in thread
From: David Rientjes @ 2021-01-28 22:42 UTC (permalink / raw)
  To: Yafang Shao
  Cc: david, vbabka, cl, penberg, iamjoonsoo.kim, akpm, pmladek,
	rostedt, sergey.senozhatsky, andriy.shevchenko, linux, linux-mm,
	linux-kernel

On Thu, 28 Jan 2021, Yafang Shao wrote:

> 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)
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/3] printk: dump full information of page flags in pGp
  2021-01-28 14:50       ` Andy Shevchenko
@ 2021-01-29  1:21         ` Yafang Shao
  0 siblings, 0 replies; 20+ messages in thread
From: Yafang Shao @ 2021-01-29  1:21 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: David Hildenbrand, Vlastimil Babka, Christoph Lameter, penberg,
	David Rientjes, iamjoonsoo.kim, Andrew Morton, Petr Mladek,
	Steven Rostedt, Sergey Senozhatsky, Rasmus Villemoes, Linux MM,
	LKML

On Thu, Jan 28, 2021 at 10:50 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Jan 28, 2021 at 09:18:24PM +0800, Yafang Shao wrote:
> > On Thu, Jan 28, 2021 at 8:11 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
>
> ...
>
> > Thanks for the explanation.
> > I will change it as you suggested.
>
> You are welcome!
>
> Just note, that kasprintf() should work for this as well as for the rest
> printf() cases. That's why it's very important to return proper buffer pointer.
>
> Also read `man snprintf(3)` RETURN VALUE section to understand it.
>

Thanks for the information. I will take a look at it.

-- 
Thanks
Yafang


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-01-29  1:22 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-28  2:19 [PATCH 0/3] mm, printk: dump full information of page flags in pGp Yafang Shao
2021-01-28  2:19 ` [PATCH 1/3] mm, slub: use pGp to print page flags Yafang Shao
2021-01-28 10:23   ` Vlastimil Babka
2021-01-28 22:42   ` David Rientjes
2021-01-28  2:19 ` [PATCH 2/3] mm, slub: don't combine pr_err with INFO Yafang Shao
2021-01-28 10:35   ` Vlastimil Babka
2021-01-28 13:06     ` Yafang Shao
2021-01-28  2:19 ` [PATCH 3/3] printk: dump full information of page flags in pGp Yafang Shao
2021-01-28  2:35   ` Joe Perches
2021-01-28  7:42     ` Yafang Shao
2021-01-28  2:52   ` Miaohe Lin
2021-01-28  7:44     ` Yafang Shao
2021-01-28 10:42   ` Vlastimil Babka
2021-01-28 13:07     ` Yafang Shao
2021-01-28 12:11   ` Andy Shevchenko
2021-01-28 13:18     ` Yafang Shao
2021-01-28 14:50       ` Andy Shevchenko
2021-01-29  1:21         ` Yafang Shao
2021-01-28 12:12 ` [PATCH 0/3] mm, " Andy Shevchenko
2021-01-28 13:19   ` Yafang Shao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).