Re: [RFC v2 2/3] mm: introduce show_page_types() to provide human-readable page_type

[Hyeonggon Yoo <42.hyeyoo@gmail.com>]

On Sun, Nov 06, 2022 at 01:23:15PM -0500, Steven Rostedt wrote:
> On Sun,  6 Nov 2022 23:03:54 +0900
> Hyeonggon Yoo <42.hyeyoo@gmail.com> wrote:
> 
> > Some page flags are not actually set in 'flags' field. To provide
> > better understanding of tracepoint output, introduce show_page_types()
> > that shows page flags from page_type.
> > 
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Andrey Konovalov <andreyknvl@gmail.com>
> > Cc: Marco Elver <elver@google.com>
> > Cc: Vasily Averin <vasily.averin@linux.dev>
> > Cc: NeilBrown <neilb@suse.de>
> > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> > ---
> >  include/trace/events/mmflags.h  | 12 ++++++++++++
> >  include/trace/events/page_ref.h | 10 ++++++++--
> >  2 files changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
> > index 72c11a16f771..a8dfb98a4dd6 100644
> > --- a/include/trace/events/mmflags.h
> > +++ b/include/trace/events/mmflags.h
> > @@ -136,6 +136,18 @@ IF_HAVE_PG_SKIP_KASAN_POISON(PG_skip_kasan_poison, "skip_kasan_poison")
> >  	__def_pageflag_names						\
> >  	) : "none"
> >  
> > +#define __def_pagetype_names						\
> > +	{PG_slab,			"slab"		},		\
> > +	{PG_offline,			"offline"	},		\
> > +	{PG_guard,			"guard"		},		\
> > +	{PG_table,			"table"		},		\
> > +	{PG_buddy,			"buddy"		}
> > +
> > +#define show_page_types(page_type)					\
> > +	page_type_has_type(page_type) ?					\
> > +		__print_flags((~page_type), "|", __def_pagetype_names)	\
> > +	: "none"
> > +
> >  #if defined(CONFIG_X86)
> >  #define __VM_ARCH_SPECIFIC_1 {VM_PAT,     "pat"           }
> >  #elif defined(CONFIG_PPC)
> > diff --git a/include/trace/events/page_ref.h b/include/trace/events/page_ref.h
> > index 8a99c1cd417b..b00d23e90e93 100644
> > --- a/include/trace/events/page_ref.h
> > +++ b/include/trace/events/page_ref.h
> > @@ -21,6 +21,7 @@ DECLARE_EVENT_CLASS(page_ref_mod_template,
> >  		__field(unsigned long, flags)
> >  		__field(int, count)
> >  		__field(int, mapcount)
> > +		__field(unsigned int, page_type)
> 
> Be careful were you add int fields for 64 bit machines.
> 
> The above is going to add 4 bytes of nothing in the ring buffer for
> each event. Please try to keep ints together by 2s, especially between
> long and pointer fields.

Oh, I wasn't aware of it. I think I can use the field 'mapcount' to
represent both mapcount and page_type as they have same offset and size
within the structure.

Just wondering where this constraint came from...

> 
> >  		__field(void *, mapping)
> >  		__field(int, mt)
> >  		__field(int, val)
> > @@ -31,14 +32,16 @@ DECLARE_EVENT_CLASS(page_ref_mod_template,
> >  		__entry->flags = page->flags;
> >  		__entry->count = page_ref_count(page);
> >  		__entry->mapcount = page_mapcount(page);
> > +		__entry->page_type = page->page_type;
> >  		__entry->mapping = page->mapping;
> >  		__entry->mt = get_pageblock_migratetype(page);
> >  		__entry->val = v;
> >  	),
> >  
> > -	TP_printk("pfn=0x%lx flags=%s count=%d mapcount=%d mapping=%p mt=%d val=%d",
> > +	TP_printk("pfn=0x%lx flags=%s page_type=%s count=%d mapcount=%d mapping=%p mt=%d val=%d",
> >  		__entry->pfn,
> >  		show_page_flags(__entry->flags & PAGEFLAGS_MASK),
> > +		show_page_types(__entry->page_type & PAGE_TYPE_MASK),
> >  		__entry->count,
> >  		__entry->mapcount, __entry->mapping, __entry->mt,
> >  		__entry->val)
> > @@ -69,6 +72,7 @@ DECLARE_EVENT_CLASS(page_ref_mod_and_test_template,
> >  		__field(unsigned long, flags)
> >  		__field(int, count)
> >  		__field(int, mapcount)
> > +		__field(unsigned int, page_type)
> 
> Here too.
> 
> -- Steve
> 
> >  		__field(void *, mapping)
> >  		__field(int, mt)
> >  		__field(int, val)
> > @@ -80,15 +84,17 @@ DECLARE_EVENT_CLASS(page_ref_mod_and_test_template,
> >  		__entry->flags = page->flags;
> >  		__entry->count = page_ref_count(page);
> >  		__entry->mapcount = page_mapcount(page);
> > +		__entry->page_type = page->page_type;
> >  		__entry->mapping = page->mapping;
> >  		__entry->mt = get_pageblock_migratetype(page);
> >  		__entry->val = v;
> >  		__entry->ret = ret;
> >  	),
> >  
> > -	TP_printk("pfn=0x%lx flags=%s count=%d mapcount=%d mapping=%p mt=%d val=%d ret=%d",
> > +	TP_printk("pfn=0x%lx flags=%s page_type=%s count=%d mapcount=%d mapping=%p mt=%d val=%d ret=%d",
> >  		__entry->pfn,
> >  		show_page_flags(__entry->flags & PAGEFLAGS_MASK),
> > +		show_page_types(__entry->page_type & PAGE_TYPE_MASK),
> >  		__entry->count,
> >  		__entry->mapcount, __entry->mapping, __entry->mt,
> >  		__entry->val, __entry->ret)
> 

-- 
Thanks,
Hyeonggon