On Fri, Jan 15, 2021 at 4:39 PM David Hildenbrand [off-list ref] wrote:

On 15.01.21 07:13, Yafang Shao wrote:

quoted

There're totally __NR_PAGEFLAGS page flags, but the type of page->flags is
unsigned long, that makes the value of page->flags a little misleading when
it is printed to the user. We'd better print the real pages flags, instead
of the whole 64bits including the random values in the useless high bits.

No, these are *not* random values. They include the nid, zid, and
section_nr - which are helpful to have at hand when debugging, or
detecting that something might be messed up there.

Thanks for the explanation. I just noticed the page-flags layout in
page-flags-layout.h.

quoted

There're two choices to achieve that, one of which is clear the useless

Again, not useless.

quoted

high bits when we initlize the page->flags, the other is don't print the
high bits when it is showed to the user. The latter one is better because
it is in the slow path and the performance won't be impacted.

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

After that change, the output is,
[ 8843.757770] INFO: Slab 0x00000000f0e98335 objects=33 used=3 fp=0x00000000b643c7d8 flags=0x10200(slab|head)

Nack to the current approach. If you're going to strip this information,
you should expose it differently. E.g., printing page_zonenum() or
page_to_nid(). But still, then we might lose valuable information of
bits stored in there that shouldn't have been set.

How about changing the implementation of pGp in printk() ?
In the new implementation of pGp we can dump the full information of
page->flags, rather than the flag's name only.

For example,
0xXXXXXXXX(node n, nid n, ..., slab|head)

That will make it easier to understand, as it is not easy to look into
the detail of page-flags layout.

quoted

Signed-off-by: Yafang Shao <redacted>
---
 include/linux/page-flags.h | 2 ++
 mm/slub.c                  | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index ec5d029..db5c017 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h

@@ -175,6 +175,8 @@ enum pageflags {
      PG_reported = PG_uptodate,
 };

+#define PAGE_FLAGS(flags) (flags & ((1 << __NR_PAGEFLAGS) - 1))
+
 #ifndef __GENERATING_BOUNDS_H

 struct page; /* forward declaration */

diff --git a/mm/slub.c b/mm/slub.c
index 901a45a..a93a03c 100644
--- a/mm/slub.c
+++ b/mm/slub.c

@@ -643,7 +643,7 @@ 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",
             page, page->objects, page->inuse, page->freelist,
-            page->flags, &page->flags);
+            PAGE_FLAGS(page->flags), &page->flags);

 }

--
Thanks,

David / dhildenb

-- 
Thanks
Yafang