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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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?
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.
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/[email protected]/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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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
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
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