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,
[68599.816764] test_printf: loaded.
[68599.819068] test_printf: all 388 tests passed
[68599.830367] test_printf: unloaded.
This patchset also includes some code cleanup in mm/slub.c.
v6:
- fixes the build failure and test failure reported by kernel test robot
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 | 90 ++++++++++++++++++++---
lib/vsprintf.c | 66 +++++++++++++++--
mm/slub.c | 13 ++--
4 files changed, 149 insertions(+), 22 deletions(-)
--
2.18.2
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/[email protected]/#t
Suggested-by: Vlastimil Babka <[email protected]>
Signed-off-by: Yafang Shao <[email protected]>
Acked-by: Vlastimil Babka <[email protected]>
Reviewed-by: Miaohe Lin <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: David Hildenbrand <[email protected]>
Acked-by: David Rientjes <[email protected]>
Cc: Matthew Wilcox <[email protected]>
---
mm/slub.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index ed3f728c1367..7ed388077633 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -624,7 +624,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
{
@@ -650,7 +650,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);
@@ -707,7 +707,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)
@@ -800,7 +800,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);
@@ -3899,7 +3899,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
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 <[email protected]>
Reviewed-by: David Hildenbrand <[email protected]>
Reviewed-by: Vlastimil Babka <[email protected]>
Acked-by: David Rientjes <[email protected]>
Acked-by: Christoph Lameter <[email protected]>
Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
Reviewed-by: Miaohe Lin <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
mm/slub.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 3021ce9bf1b3..ed3f728c1367 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -650,8 +650,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
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.
On 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,
[68599.816764] test_printf: loaded.
[68599.819068] test_printf: all 388 tests passed
[68599.830367] test_printf: unloaded.
[[email protected]: reported issues in the prev version in test_printf.c]
Signed-off-by: Yafang Shao <[email protected]>
Cc: David Hildenbrand <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Miaohe Lin <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: Petr Mladek <[email protected]>
Cc: kernel test robot <[email protected]>
---
Documentation/core-api/printk-formats.rst | 2 +-
lib/test_printf.c | 90 ++++++++++++++++++++---
lib/vsprintf.c | 66 +++++++++++++++--
3 files changed, 142 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 95a2f82427c7..27b964ec723d 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -577,24 +577,98 @@ netdev_features(void)
{
}
+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[] = {
+ {SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK,
+ 0, "%d", "section"},
+ {NODES_WIDTH, NODES_PGSHIFT, NODES_MASK,
+ 0, "%d", "node"},
+ {ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK,
+ 0, "%d", "zone"},
+ {LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK,
+ 0, "%#x", "lastcpupid"},
+ {KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK,
+ 0, "%#x", "kasantag"},
+};
+
+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 values[] = {section, node, zone, last_cpupid, kasan_tag};
+ unsigned long page_flags = 0;
+ unsigned long size = 0;
+ bool append = false;
+ int i;
+
+ flags &= BIT(NR_PAGEFLAGS) - 1;
+ if (flags) {
+ page_flags |= flags;
+ 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
+ }
+
+ /* 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;
+
+ if (append) {
+ snprintf(cmp_buf + size, BUF_SIZE - size, "|");
+ size = strlen(cmp_buf);
+ }
+
+ page_flags |= (pft[i].value & 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);
+ size = strlen(cmp_buf);
+ append = true;
+ }
+
+ 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;
@@ -609,10 +683,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 41ddc353ebb8..92e6085eef15 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, default_str_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
On Fri, Mar 19, 2021 at 6:13 PM Yafang Shao <[email protected]> wrote:
>
> 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,
> [68599.816764] test_printf: loaded.
> [68599.819068] test_printf: all 388 tests passed
> [68599.830367] test_printf: unloaded.
>
> This patchset also includes some code cleanup in mm/slub.c.
>
> v6:
> - fixes the build failure and test failure reported by kernel test robot
>
> 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 | 90 ++++++++++++++++++++---
> lib/vsprintf.c | 66 +++++++++++++++--
> mm/slub.c | 13 ++--
> 4 files changed, 149 insertions(+), 22 deletions(-)
>
> --
> 2.18.2
>
Hi Petr,
Any comments on this version ?
Thanks
Yafang
On Fri 2021-03-19 18:15:19, Yafang Shao wrote:
> On Fri, Mar 19, 2021 at 6:13 PM Yafang Shao <[email protected]> wrote:
> Hi Petr,
>
> Any comments on this version ?
I have been busy this week with some other work. I am going to review
the original v6 either later today or the following week.
Best Regards,
Petr
PS: I usually react quickly but it is not always possible. There was
no need to resend the entire patchset twice within one week.
On (21/03/19 18:12), Yafang Shao wrote:
>
> 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,
> [68599.816764] test_printf: loaded.
> [68599.819068] test_printf: all 388 tests passed
> [68599.830367] test_printf: unloaded.
>
> This patchset also includes some code cleanup in mm/slub.c.
The series looks good to me. FWIW,
Reviewed-by: Sergey Senozhatsky <[email protected]>
On Fri 2021-03-19 18:12:46, 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.
>
> On 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,
> [68599.816764] test_printf: loaded.
> [68599.819068] test_printf: all 388 tests passed
> [68599.830367] test_printf: unloaded.
>
> [[email protected]: reported issues in the prev version in test_printf.c]
>
> Signed-off-by: Yafang Shao <[email protected]>
Reviewed-by: Petr Mladek <[email protected]>
It looks good. And it seems that the selftest should not longer have
that problems on various architectures and configurations.
I am going to push it.
Best Regards,
Petr
On Fri 2021-03-19 18:12:43, Yafang Shao wrote:
> 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.
>
> This patchset also includes some code cleanup in mm/slub.c.
>
> v6:
> - fixes the build failure and test failure reported by kernel test robot
>
>
> 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 | 90 ++++++++++++++++++++---
> lib/vsprintf.c | 66 +++++++++++++++--
> mm/slub.c | 13 ++--
> 4 files changed, 149 insertions(+), 22 deletions(-)
The patchset has been pushed into printk/linux.git, branch
for-5.13-vsprintf-pgp. It should appear in linux-next again.
The first two patches are the same as in v5 that has already been
pushed earlier. I probably should have replaced only the last patch
but I rebased everything. It was too easy with the b4 tool. And
it is Friday late afternoon here.
Best Regards,
Petr