[PATCH 0/2] Improve dump_page() output for slab pages

[PATCH 0/2] Improve dump_page() output for slab pages

[Sukrit Bhatnagar]

While using dump_page() on a range of pages, I noticed that there were some
PG_slab pages that were also showing as PG_anon pages, according to the
function output.

[    7.071985] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x102768
[    7.072602] head: order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[    7.073085] anon flags: 0x8000000000000840(slab|head|zone=2)
[    7.073777] raw: 8000000000000840 ffff8881000419c0 0000000000000000 dead000000000001

It was also printing the "page_type" field for slab pages, but that was fixed in
a very recent commit:
    8f790d0c7cfe (mm: improve dumping of mapcount and page_type)

Given that the slab pages cannot be mapped to userspace, this output seems
misleading.

In dump_page(), folio_test_anon() is used, which checks the "mapping" field.
But the struct slab was separated from struct page.
So accessing the mapping field through a struct page pointer, which actually
points to a struct slab, will result in garbage memory access and the PG_anon
test can return true.

It seems that other parts of the kernel MM make the check for slab before
checking for anon, but dump_page() is not doing that.

On the other hand, the struct slab has kmem_cache which maintains another set
of flags. It would be nice to have these flags added as a part of the debug
output, and to have a convenient way to print them.

(The long chain of pointer dereferences for cache flags looks messy, but I
assume it should be fine for a debug function.)

Sukrit Bhatnagar (2):
  mm: printk: introduce new format %pGs for slab flags
  mm: debug: print correct information for slab folios

 Documentation/core-api/printk-formats.rst |  2 +
 include/linux/slab.h                      |  5 ++
 include/trace/events/mmflags.h            | 67 +++++++++++++++++++++++
 lib/test_printf.c                         | 13 +++++
 lib/vsprintf.c                            | 22 ++++++++
 mm/debug.c                                | 12 +++-
 mm/internal.h                             |  1 +
 7 files changed, 121 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH 1/2] mm: printk: introduce new format %pGs for slab flags

[Sukrit Bhatnagar]

The slab pages have their own flags (apart from PG_slab set in struct page),
kept in the kmem_cache's flag field. These flags are visible to the users of
slab cache and are needed when creating one. It will be useful to be able to
print these slab flags, mainly for debugging purposes, if the folio tests true
for slab.

Add printk format specifier for kmem_cache flags.

Signed-off-by: Sukrit Bhatnagar <Sukrit.Bhatnagar@sony.com>
---
 Documentation/core-api/printk-formats.rst |  2 +
 include/linux/slab.h                      |  5 ++
 include/trace/events/mmflags.h            | 67 +++++++++++++++++++++++
 lib/test_printf.c                         | 13 +++++
 lib/vsprintf.c                            | 22 ++++++++
 mm/debug.c                                |  5 ++
 mm/internal.h                             |  1 +
 7 files changed, 115 insertions(+)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 4451ef501936..060af5df7a2c 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -583,6 +583,7 @@ Flags bitfields such as page flags, page_type, gfp_flags
 
 	%pGp	0x17ffffc0002036(referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff)
 	%pGt	0xffffff7f(buddy)
+	%pGs	0x10310(HWCACHE_ALIGN|PANIC|TYPESAFE_BY_RCU|CMPXCHG_DOUBLE)
 	%pGg	GFP_USER|GFP_DMA32|GFP_NOWARN
 	%pGv	read|exec|mayread|maywrite|mayexec|denywrite
 
@@ -592,6 +593,7 @@ character. Currently supported are:
 
         - p - [p]age flags, expects value of type (``unsigned long *``)
         - t - page [t]ype, expects value of type (``unsigned int *``)
+        - s - [s]lab flags, expects value of type (``slab_flags_t *``)
         - v - [v]ma_flags, expects value of type (``unsigned long *``)
         - g - [g]fp_flags, expects value of type (``gfp_t *``)
 
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 7247e217e21b..b1ca372f5ee1 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -21,6 +21,10 @@
 #include <linux/cleanup.h>
 #include <linux/hash.h>
 
+/*
+ * In case of any changes, please don't forget to update the flags
+ * in include/linux/events/mmflags.h
+ */
 enum _slab_flag_bits {
 	_SLAB_CONSISTENCY_CHECKS,
 	_SLAB_RED_ZONE,
@@ -64,6 +68,7 @@ enum _slab_flag_bits {
 
 #define __SLAB_FLAG_BIT(nr)	((slab_flags_t __force)(1U << (nr)))
 #define __SLAB_FLAG_UNUSED	((slab_flags_t __force)(0U))
+#define SLAB_FLAG_MASK		((slab_flags_t __force)(1U << _SLAB_FLAGS_LAST_BIT) - 1)
 
 /*
  * Flags to pass to kmem_cache_create().
diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
index e46d6e82765e..1457bc23206f 100644
--- a/include/trace/events/mmflags.h
+++ b/include/trace/events/mmflags.h
@@ -141,6 +141,73 @@ IF_HAVE_PG_ARCH_X(arch_3)
 	DEF_PAGETYPE_NAME(table),					\
 	DEF_PAGETYPE_NAME(buddy)
 
+#ifdef CONFIG_DEBUG_OBJECTS
+#define IF_HAVE_SLAB_DEBUG_OBJECTS(_name) {1UL << _SLAB_##_name, __stringify(_name)},
+#else
+#define IF_HAVE_SLAB_DEBUG_OBJECTS(_name)
+#endif
+
+#ifdef CONFIG_FAILSLAB
+#define IF_HAVE_SLAB_FAILSLAB(_name) {1U << _SLAB_##_name, __stringify(_name)},
+#else
+#define IF_HAVE_SLAB_FAILSLAB(_name)
+#endif
+
+#ifdef CONFIG_MEMCG_KMEM
+#define IF_HAVE_SLAB_MEMCG_KMEM(_name) {1U << _SLAB_##_name, __stringify(_name)},
+#else
+#define IF_HAVE_SLAB_MEMCG_KMEM(_name)
+#endif
+
+#ifdef CONFIG_KASAN_GENERIC
+#define IF_HAVE_SLAB_KASAN_GENERIC(_name) {1UL << _SLAB_##_name, __stringify(_name)},
+#else
+#define IF_HAVE_SLAB_KASAN_GENERIC(_name)
+#endif
+
+#ifdef CONFIG_KFENCE
+#define IF_HAVE_SLAB_KFENCE(_name) {1UL << _SLAB_##_name, __stringify(_name)},
+#else
+#define IF_HAVE_SLAB_KFENCE(_name)
+#endif
+
+#ifdef CONFIG_SLUB_TINY
+#define IF_HAVE_SLAB_SLUB_TINY(_name) {1UL << _SLAB_##_name, __stringify(_name)}
+#else
+#define IF_HAVE_SLAB_SLUB_TINY(_name)
+#endif
+
+#define DEF_SLABFLAG_NAME(_name) { 1UL <<  _SLAB_##_name, __stringify(_name) }
+
+#define __def_slabflag_names						\
+	DEF_SLABFLAG_NAME(CONSISTENCY_CHECKS),				\
+	DEF_SLABFLAG_NAME(RED_ZONE),					\
+	DEF_SLABFLAG_NAME(POISON),					\
+	DEF_SLABFLAG_NAME(KMALLOC),					\
+	DEF_SLABFLAG_NAME(HWCACHE_ALIGN),				\
+	DEF_SLABFLAG_NAME(CACHE_DMA),					\
+	DEF_SLABFLAG_NAME(CACHE_DMA32),					\
+	DEF_SLABFLAG_NAME(STORE_USER),					\
+	DEF_SLABFLAG_NAME(PANIC),					\
+	DEF_SLABFLAG_NAME(TYPESAFE_BY_RCU),				\
+	DEF_SLABFLAG_NAME(TRACE),					\
+IF_HAVE_SLAB_DEBUG_OBJECTS(DEBUG_OBJECTS)				\
+	DEF_SLABFLAG_NAME(NOLEAKTRACE),					\
+	DEF_SLABFLAG_NAME(NO_MERGE),					\
+IF_HAVE_SLAB_FAILSLAB(FAILSLAB)						\
+IF_HAVE_SLAB_MEMCG_KMEM(MEMCG_KMEM)					\
+IF_HAVE_SLAB_KASAN_GENERIC(KASAN_GENERIC)				\
+	DEF_SLABFLAG_NAME(NO_USER_FLAGS),				\
+IF_HAVE_SLAB_KFENCE(KFENCE)						\
+IF_HAVE_SLAB_SLUB_TINY(SLUB_TINY)					\
+	DEF_SLABFLAG_NAME(OBJECT_POISON),				\
+	DEF_SLABFLAG_NAME(CMPXCHG_DOUBLE)
+
+#define show_slab_flags(flags)						\
+	(flags) ? __print_flags(flags, "|",				\
+	__def_slabflag_names						\
+	) : "none"
+
 #if defined(CONFIG_X86)
 #define __VM_ARCH_SPECIFIC_1 {VM_PAT,     "pat"           }
 #elif defined(CONFIG_PPC)
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 69b6a5e177f2..37f3f837bcbf 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -681,6 +681,19 @@ flags(void)
 	flags = VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;
 	test("read|exec|mayread|maywrite|mayexec", "%pGv", &flags);
 
+	flags = 0;
+	scnprintf(cmp_buffer, BUF_SIZE, "%#x(%s)", (unsigned int) flags, "");
+	test(cmp_buffer, "%pGs", &flags);
+
+	flags = 1U << _SLAB_FLAGS_LAST_BIT;
+	scnprintf(cmp_buffer, BUF_SIZE, "%#x(%s)", (unsigned int) flags, "");
+	test(cmp_buffer, "%pGs", &flags);
+
+	flags = SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_NO_USER_FLAGS;
+	scnprintf(cmp_buffer, BUF_SIZE, "%#x(%s)", (unsigned int) flags,
+		  "HWCACHE_ALIGN|PANIC|NO_USER_FLAGS");
+	test(cmp_buffer, "%pGs", &flags);
+
 	gfp = GFP_TRANSHUGE;
 	test("GFP_TRANSHUGE", "%pGg", &gfp);
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 552738f14275..67f3584db58c 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2054,6 +2054,25 @@ char *format_page_flags(char *buf, char *end, unsigned long flags)
 	return buf;
 }
 
+static
+char *format_slab_flags(char *buf, char *end, unsigned int flags)
+{
+	buf = number(buf, end, flags, default_flag_spec);
+	if (buf < end)
+		*buf = '(';
+	buf++;
+
+	flags &= SLAB_FLAG_MASK;
+	if (flags)
+		buf = format_flags(buf, end, (__force unsigned long)flags, slabflag_names);
+
+	if (buf < end)
+		*buf = ')';
+	buf++;
+
+	return buf;
+}
+
 static
 char *format_page_type(char *buf, char *end, unsigned int page_type)
 {
@@ -2088,6 +2107,9 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
 		return format_page_flags(buf, end, *(unsigned long *)flags_ptr);
 	case 't':
 		return format_page_type(buf, end, *(unsigned int *)flags_ptr);
+	case 's':
+		flags = (__force unsigned int)(*(slab_flags_t *)flags_ptr);
+		return format_slab_flags(buf, end, flags);
 	case 'v':
 		flags = *(unsigned long *)flags_ptr;
 		names = vmaflag_names;
diff --git a/mm/debug.c b/mm/debug.c
index 69e524c3e601..2ef516f310e8 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -41,6 +41,11 @@ const struct trace_print_flags pagetype_names[] = {
 	{0, NULL}
 };
 
+const struct trace_print_flags slabflag_names[] = {
+	__def_slabflag_names,
+	{0, NULL}
+};
+
 const struct trace_print_flags gfpflag_names[] = {
 	__def_gfpflag_names,
 	{0, NULL}
diff --git a/mm/internal.h b/mm/internal.h
index 2adabe369403..6a15f4937db8 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1123,6 +1123,7 @@ static inline void flush_tlb_batched_pending(struct mm_struct *mm)
 
 extern const struct trace_print_flags pageflag_names[];
 extern const struct trace_print_flags pagetype_names[];
+extern const struct trace_print_flags slabflag_names[];
 extern const struct trace_print_flags vmaflag_names[];
 extern const struct trace_print_flags gfpflag_names[];
 
-- 
2.34.1

Re: [PATCH 1/2] mm: printk: introduce new format %pGs for slab flags

[kernel test robot]

Hi Sukrit,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on akpm-mm/mm-nonmm-unstable linus/master v6.9 next-20240522]
[cannot apply to vbabka-slab/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sukrit-Bhatnagar/mm-printk-introduce-new-format-pGs-for-slab-flags/20240522-154443
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240522074629.2420423-2-Sukrit.Bhatnagar%40sony.com
patch subject: [PATCH 1/2] mm: printk: introduce new format %pGs for slab flags
config: x86_64-randconfig-123-20240522 (https://download.01.org/0day-ci/archive/20240523/202405230441.A0LFA9SY-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240523/202405230441.A0LFA9SY-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405230441.A0LFA9SY-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> lib/test_printf.c:692:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned long [addressable] [assigned] flags @@     got restricted slab_flags_t @@
   lib/test_printf.c:692:15: sparse:     expected unsigned long [addressable] [assigned] flags
   lib/test_printf.c:692:15: sparse:     got restricted slab_flags_t
   lib/test_printf.c:708:49: sparse: sparse: cast from restricted gfp_t
   lib/test_printf.c:712:58: sparse: sparse: cast from restricted gfp_t
   lib/test_printf.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/umh.h, include/linux/kmod.h, ...):
   include/linux/page-flags.h:240:46: sparse: sparse: self-comparison always evaluates to false
   include/linux/page-flags.h:240:46: sparse: sparse: self-comparison always evaluates to false

vim +692 lib/test_printf.c

   656	
   657	static void __init
   658	flags(void)
   659	{
   660		unsigned long flags;
   661		char *cmp_buffer;
   662		gfp_t gfp;
   663		unsigned int page_type;
   664	
   665		cmp_buffer = kmalloc(BUF_SIZE, GFP_KERNEL);
   666		if (!cmp_buffer)
   667			return;
   668	
   669		flags = 0;
   670		page_flags_test(0, 0, 0, 0, 0, flags, "", cmp_buffer);
   671	
   672		flags = 1UL << NR_PAGEFLAGS;
   673		page_flags_test(0, 0, 0, 0, 0, flags, "", cmp_buffer);
   674	
   675		flags |= 1UL << PG_uptodate | 1UL << PG_dirty | 1UL << PG_lru
   676			| 1UL << PG_active | 1UL << PG_swapbacked;
   677		page_flags_test(1, 1, 1, 0x1fffff, 1, flags,
   678				"uptodate|dirty|lru|active|swapbacked",
   679				cmp_buffer);
   680	
   681		flags = VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;
   682		test("read|exec|mayread|maywrite|mayexec", "%pGv", &flags);
   683	
   684		flags = 0;
   685		scnprintf(cmp_buffer, BUF_SIZE, "%#x(%s)", (unsigned int) flags, "");
   686		test(cmp_buffer, "%pGs", &flags);
   687	
   688		flags = 1U << _SLAB_FLAGS_LAST_BIT;
   689		scnprintf(cmp_buffer, BUF_SIZE, "%#x(%s)", (unsigned int) flags, "");
   690		test(cmp_buffer, "%pGs", &flags);
   691	
 > 692		flags = SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_NO_USER_FLAGS;
   693		scnprintf(cmp_buffer, BUF_SIZE, "%#x(%s)", (unsigned int) flags,
   694			  "HWCACHE_ALIGN|PANIC|NO_USER_FLAGS");
   695		test(cmp_buffer, "%pGs", &flags);
   696	
   697		gfp = GFP_TRANSHUGE;
   698		test("GFP_TRANSHUGE", "%pGg", &gfp);
   699	
   700		gfp = GFP_ATOMIC|__GFP_DMA;
   701		test("GFP_ATOMIC|GFP_DMA", "%pGg", &gfp);
   702	
   703		gfp = __GFP_HIGH;
   704		test("__GFP_HIGH", "%pGg", &gfp);
   705	
   706		/* Any flags not translated by the table should remain numeric */
   707		gfp = ~__GFP_BITS_MASK;
   708		snprintf(cmp_buffer, BUF_SIZE, "%#lx", (unsigned long) gfp);
   709		test(cmp_buffer, "%pGg", &gfp);
   710	
   711		snprintf(cmp_buffer, BUF_SIZE, "__GFP_HIGH|%#lx",
   712								(unsigned long) gfp);
   713		gfp |= __GFP_HIGH;
   714		test(cmp_buffer, "%pGg", &gfp);
   715	
   716		page_type = ~0;
   717		page_type_test(page_type, "", cmp_buffer);
   718	
   719		page_type = 10;
   720		page_type_test(page_type, "", cmp_buffer);
   721	
   722		page_type = ~PG_buddy;
   723		page_type_test(page_type, "buddy", cmp_buffer);
   724	
   725		page_type = ~(PG_table | PG_buddy);
   726		page_type_test(page_type, "table|buddy", cmp_buffer);
   727	
   728		kfree(cmp_buffer);
   729	}
   730	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 2/2] mm: debug: print correct information for slab folios

[Sukrit Bhatnagar]

The function dump_page() prints "anon" even for slab pages.
This is not correct, especially now that struct slab is separated from
struct page, and that the slab pages cannot be mapped to userspace.

[    7.071985] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x102768
[    7.072602] head: order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[    7.073085] anon flags: 0x8000000000000840(slab|head|zone=2)
[    7.073777] raw: 8000000000000840 ffff8881000419c0 0000000000000000 dead000000000001

This debugging output may be misleading, and it is not easy to understand
unless we read the source code.

If the folio tests true for slab, do not print information that does not
apply to it. Instead, print the slab flags stored in the kmem_cache field.

[    7.248722] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888103e6aa87>
[    7.249135] head: order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[    7.249429] slab flags: 0x8000000000000840(slab|head|zone=2)
[    7.249664] cache flags: 0x10310(HWCACHE_ALIGN|PANIC|TYPESAFE_BY_RCU|CMPXCHG_DOUBLE)
[    7.249999] raw: 8000000000000000 ffffea00040f9a01 ffffea00040f9bc8 dead000000000400

Signed-off-by: Sukrit Bhatnagar <Sukrit.Bhatnagar@sony.com>
---
 mm/debug.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/mm/debug.c b/mm/debug.c
index 2ef516f310e8..b6892dd279cb 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -16,6 +16,7 @@
 #include <linux/ctype.h>
 
 #include "internal.h"
+#include "slab.h"
 #include <trace/events/migrate.h>
 
 /*
@@ -80,7 +81,9 @@ static void __dump_folio(struct folio *folio, struct page *page,
 	if (folio->memcg_data)
 		pr_warn("memcg:%lx\n", folio->memcg_data);
 #endif
-	if (folio_test_ksm(folio))
+	if (folio_test_slab(folio))
+		type = "slab ";
+	else if (folio_test_ksm(folio))
 		type = "ksm ";
 	else if (folio_test_anon(folio))
 		type = "anon ";
@@ -98,6 +101,8 @@ static void __dump_folio(struct folio *folio, struct page *page,
 		is_migrate_cma_folio(folio, pfn) ? " CMA" : "");
 	if (page_has_type(&folio->page))
 		pr_warn("page_type: %pGt\n", &folio->page.page_type);
+	else if (folio_test_slab(folio))
+		pr_warn("cache flags: %pGs\n", &((struct slab *)&folio->page)->slab_cache->flags);
 
 	print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
 			sizeof(unsigned long), page,
-- 
2.34.1

Re: [PATCH 2/2] mm: debug: print correct information for slab folios

[Matthew Wilcox]

On Wed, May 22, 2024 at 04:46:29PM +0900, Sukrit Bhatnagar wrote:
> The function dump_page() prints "anon" even for slab pages.
> This is not correct, especially now that struct slab is separated from
> struct page, and that the slab pages cannot be mapped to userspace.
> 
> [    7.071985] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x102768
> [    7.072602] head: order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
> [    7.073085] anon flags: 0x8000000000000840(slab|head|zone=2)
> [    7.073777] raw: 8000000000000840 ffff8881000419c0 0000000000000000 dead000000000001
> 
> This debugging output may be misleading, and it is not easy to understand
> unless we read the source code.
> 
> If the folio tests true for slab, do not print information that does not
> apply to it. Instead, print the slab flags stored in the kmem_cache field.
> 
> [    7.248722] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888103e6aa87>
> [    7.249135] head: order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
> [    7.249429] slab flags: 0x8000000000000840(slab|head|zone=2)
> [    7.249664] cache flags: 0x10310(HWCACHE_ALIGN|PANIC|TYPESAFE_BY_RCU|CMPXCHG_DOUBLE)
> [    7.249999] raw: 8000000000000000 ffffea00040f9a01 ffffea00040f9bc8 dead000000000400

You haven't tested this against the current codebase ...

> @@ -98,6 +101,8 @@ static void __dump_folio(struct folio *folio, struct page *page,
>  		is_migrate_cma_folio(folio, pfn) ? " CMA" : "");
>  	if (page_has_type(&folio->page))
>  		pr_warn("page_type: %pGt\n", &folio->page.page_type);
> +	else if (folio_test_slab(folio))
> +		pr_warn("cache flags: %pGs\n", &((struct slab *)&folio->page)->slab_cache->flags);
>  

... because page_has_type() is now true for slab; there is no more
PG_slab.  I think you also want:

	folio_slab(folio)->slab_cache->flags

Anyway, we have print_slab_info() which is currently static in slub.c.
Maybe that needs to become non-static and dump_page() should call that
for slabs?

RE: [PATCH 2/2] mm: debug: print correct information for slab folios

[Sukrit.Bhatnagar]

Hi Matthew,

On 2024-05-22 21:32, Matthew Wilcox wrote:
> On Wed, May 22, 2024 at 04:46:29PM +0900, Sukrit Bhatnagar wrote:
>> If the folio tests true for slab, do not print information that does not
>> apply to it. Instead, print the slab flags stored in the kmem_cache field.
>> 
>> [    7.248722] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888103e6aa87>
>> [    7.249135] head: order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
>> [    7.249429] slab flags: 0x8000000000000840(slab|head|zone=2)
>> [    7.249664] cache flags: 0x10310(HWCACHE_ALIGN|PANIC|TYPESAFE_BY_RCU|CMPXCHG_DOUBLE)
>> [    7.249999] raw: 8000000000000000 ffffea00040f9a01 ffffea00040f9bc8 dead000000000400
> 
> You haven't tested this against the current codebase ...
>
>> @@ -98,6 +101,8 @@ static void __dump_folio(struct folio *folio, struct page *page,
>>  		is_migrate_cma_folio(folio, pfn) ? " CMA" : "");
>>  	if (page_has_type(&folio->page))
>>  		pr_warn("page_type: %pGt\n", &folio->page.page_type);
>> +	else if (folio_test_slab(folio))
>> +		pr_warn("cache flags: %pGs\n", &((struct slab *)&folio->page)->slab_cache->flags);
>> 
> 
> ... because page_has_type() is now true for slab; there is no more
> PG_slab.  I think you also want:
> 
> 	folio_slab(folio)->slab_cache->flags

I didn't notice your other patch about removing PG_slab; it pretty much solves
this issue and much more.
(I had created these patches a few weeks ago.)

> Anyway, we have print_slab_info() which is currently static in slub.c.
> Maybe that needs to become non-static and dump_page() should call that
> for slabs?

Thank you for the suggestions.

print_slab_info() has a slightly different output string format, which does not
match with the dump_page() output style.
Adding it as-it-is looks a bit weird to me.
Other than that, I think it may be useful to print it (which would happen only
when SLAB_DEBUG is enabled).

Also: print_slab_info() is printing the folio's flags. Maybe that needs a change?

--
Sukrit

Re: [PATCH 0/2] Improve dump_page() output for slab pages

[Matthew Wilcox]

On Wed, May 22, 2024 at 04:46:27PM +0900, Sukrit Bhatnagar wrote:
> On the other hand, the struct slab has kmem_cache which maintains another set
> of flags. It would be nice to have these flags added as a part of the debug
> output, and to have a convenient way to print them.

I don't understand why the slab cache flags are the interesting thing.
Seems to me it'd be more useful to print slab->slab_cache->name and
then you'd be able to look up the flags from that, as well as get a lot
more information.

RE: [PATCH 0/2] Improve dump_page() output for slab pages

[Sukrit.Bhatnagar]

Hi Matthew,

On 2024-05-22 23:11, Matthew Wilcox wrote:
> On Wed, May 22, 2024 at 04:46:27PM +0900, Sukrit Bhatnagar wrote:
>> On the other hand, the struct slab has kmem_cache which maintains another set
>> of flags. It would be nice to have these flags added as a part of the debug
>> output, and to have a convenient way to print them.
> 
> I don't understand why the slab cache flags are the interesting thing.
> Seems to me it'd be more useful to print slab->slab_cache->name and
> then you'd be able to look up the flags from that, as well as get a lot
> more information.

I agree on printing slab name instead which enables lookup in sysfs
entries etc.

The reason I added the print for kmem_cache flags was because
dump_page() was doing that for folio/page.

Another thing I noticed in the per-slab sysfs (/sys/kernel/slab/$name)
is that we a few entries for kmem_cache flag enabled/disabled output.
There is no entry however which shows all flags.
The slabinfo in proc does not show flag info either.

Is there a need for showing a formatted string of kmem_cache flags in
the sysfs?

Just trying to salvage the %pGs patch in this series, before I have
to discard it... :)

--
Sukrit