[PATCH] page-types: kernel pageflags mode

[PATCH] page-types: kernel pageflags mode

[Alex Chiang]

An earlier commit taught page-types the -d|--describe argument, which
allows the user to describe page flags passed on the command line:

  # ./Documentation/vm/page-types -d 0x4000
  0x0000000000004000  ______________b___________________  swapbacked

In -d mode, page-types expects the page flag bits in the order generated
by the kernel function get_uflags().

However, those bits are rearranged compared to what is actually stored
in struct page->flags. A kernel developer dumping a page's flags
using printk, e.g., may get misleading results in -d mode.

Teach page-types the -k mode, which parses and describes the bits in
the internal kernel order:

  # ./Documentation/vm/page-types -k 0x4000
  0x0000000000004000  ______________H_________  compound_head

Note that the recommended way to build page-types is from the top-level
kernel source directory. This ensures that it will get the same CONFIG_*
defines used to build the kernel source.

  # make Documentation/vm/

The implication is that attempting to use page-types -k on a kernel
with different CONFIG_* settings may lead to surprising and misleading
results. To retain sanity, always use the page-types built out of the
kernel tree you are actually testing.

Cc: fengguang.wu@intel.com
Cc: Haicheng Li <haicheng.li@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

Applies on top of mmotm.

 Documentation/vm/Makefile     |    2 +
 Documentation/vm/page-types.c |  117 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 113 insertions(+), 6 deletions(-)

diff --git a/Documentation/vm/Makefile b/Documentation/vm/Makefile
index 5bd269b..1bebc43 100644
--- a/Documentation/vm/Makefile
+++ b/Documentation/vm/Makefile
@@ -1,6 +1,8 @@
 # kbuild trick to avoid linker error. Can be omitted if a module is built.
 obj- := dummy.o
 
+HOSTCFLAGS_page-types.o += $(LINUXINCLUDE)
+
 # List of programs to build
 hostprogs-y := slabinfo page-types
 
diff --git a/Documentation/vm/page-types.c b/Documentation/vm/page-types.c
index 7a7d9ba..b0f129c 100644
--- a/Documentation/vm/page-types.c
+++ b/Documentation/vm/page-types.c
@@ -100,7 +100,7 @@
 #define BIT(name)		(1ULL << KPF_##name)
 #define BITS_COMPOUND		(BIT(COMPOUND_HEAD) | BIT(COMPOUND_TAIL))
 
-static const char *page_flag_names[] = {
+static const char *page_flag_names_proc[] = {
 	[KPF_LOCKED]		= "L:locked",
 	[KPF_ERROR]		= "E:error",
 	[KPF_REFERENCED]	= "R:referenced",
@@ -140,6 +140,103 @@ static const char *page_flag_names[] = {
 	[KPF_SLUB_DEBUG]	= "E:slub_debug",
 };
 
+enum pageflags {
+	PG_locked,              /* Page is locked. Don't touch. */
+	PG_error,
+	PG_referenced,
+	PG_uptodate,
+	PG_dirty,
+	PG_lru,
+	PG_active,
+	PG_slab,
+	PG_owner_priv_1,        /* Owner use. If pagecache, fs may use*/
+	PG_arch_1,
+	PG_reserved,
+	PG_private,             /* If pagecache, has fs-private data */
+	PG_private_2,           /* If pagecache, has fs aux data */
+	PG_writeback,           /* Page is under writeback */
+#ifdef CONFIG_PAGEFLAGS_EXTENDED
+	PG_head,                /* A head page */
+	PG_tail,                /* A tail page */
+#else
+	PG_compound,            /* A compound page */
+#endif
+	PG_swapcache,           /* Swap page: swp_entry_t in private */
+	PG_mappedtodisk,        /* Has blocks allocated on-disk */
+	PG_reclaim,             /* To be reclaimed asap */
+	PG_buddy,               /* Page is free, on buddy lists */
+	PG_swapbacked,          /* Page is backed by RAM/swap */
+	PG_unevictable,         /* Page is "unevictable"  */
+#ifdef CONFIG_MMU
+	PG_mlocked,             /* Page is vma mlocked */
+#endif
+#ifdef CONFIG_ARCH_USES_PG_UNCACHED
+	PG_uncached,            /* Page has been mapped as uncached */
+#endif
+#ifdef CONFIG_MEMORY_FAILURE
+	PG_hwpoison,            /* hardware poisoned page. Don't touch */
+#endif
+	__NR_PAGEFLAGS,
+
+	/* Filesystems */
+	PG_checked = PG_owner_priv_1,
+
+	/* Two page bits are conscripted by FS-Cache to maintain local caching
+	 * state.  These bits are set on pages belonging to the netfs's inodes
+	 * when those inodes are being locally cached.
+	 */
+	PG_fscache = PG_private_2,      /* page backed by cache */
+
+	/* XEN */
+	PG_pinned = PG_owner_priv_1,
+	PG_savepinned = PG_dirty,
+
+	/* SLOB */
+	PG_slob_free = PG_private,
+
+	/* SLUB */
+	PG_slub_frozen = PG_active,
+	PG_slub_debug = PG_error,
+};
+
+static const char *page_flag_names_kernel[] = {
+	[PG_locked]		= "L:locked",
+	[PG_error]		= "E:error",
+	[PG_referenced]		= "R:referenced",
+	[PG_uptodate]		= "U:uptodate",
+	[PG_dirty]		= "D:dirty",
+	[PG_lru]		= "l:lru",
+	[PG_active]		= "A:active",
+	[PG_slab]		= "S:slab",
+	[PG_owner_priv_1]	= "O:owner_private",
+	[PG_arch_1]		= "h:arch",
+	[PG_reserved]		= "r:reserved",
+	[PG_private]		= "P:private",
+	[PG_private_2]		= "p:private_2",
+	[PG_writeback]		= "W:writeback",
+#ifdef CONFIG_PAGEFLAGS_EXTENDED
+	[PG_head]		= "H:compound_head",
+	[PG_tail]		= "T:compound_tail",
+#else
+	[PG_compound]		= "C:compound",
+#endif
+	[PG_swapcache]		= "s:swapcache",
+	[PG_mappedtodisk]	= "d:mappedtodisk",
+	[PG_reclaim]		= "I:reclaim",
+	[PG_buddy]		= "B:buddy",
+	[PG_swapbacked]		= "b:swapbacked",
+	[PG_unevictable]	= "u:unevictable",
+#ifdef CONFIG_MMU
+	[PG_mlocked]		= "m:mlocked",
+#endif
+#ifdef CONFIG_ARCH_USES_PG_UNCACHED
+	[PG_uncached]		= "c:uncached",
+#endif
+#ifdef CONFIG_MEMORY_FAILURE
+	[PG_hwpoison]		= "X:hwpoison",
+#endif
+};
+
 
 /*
  * data structures
@@ -186,6 +283,8 @@ static unsigned long	total_pages;
 static unsigned long	nr_pages[HASH_SIZE];
 static uint64_t 	page_flags[HASH_SIZE];
 
+static char **page_flag_names = (char **)page_flag_names_proc;
+static int page_flag_nr = KPF_SLUB_DEBUG + 1;
 
 /*
  * helper functions
@@ -297,7 +396,7 @@ static char *page_flag_name(uint64_t flags)
 	int present;
 	int i, j;
 
-	for (i = 0, j = 0; i < ARRAY_SIZE(page_flag_names); i++) {
+	for (i = 0, j = 0; i < page_flag_nr; i++) {
 		present = (flags >> i) & 1;
 		if (!page_flag_names[i]) {
 			if (present)
@@ -315,7 +414,7 @@ static char *page_flag_longname(uint64_t flags)
 	static char buf[1024];
 	int i, n;
 
-	for (i = 0, n = 0; i < ARRAY_SIZE(page_flag_names); i++) {
+	for (i = 0, n = 0; i < page_flag_nr; i++) {
 		if (!page_flag_names[i])
 			continue;
 		if ((flags >> i) & 1)
@@ -675,6 +774,7 @@ static void usage(void)
 "page-types [options]\n"
 "            -r|--raw                   Raw mode, for kernel developers\n"
 "            -d|--describe flags        Describe flags\n"
+"            -k|--kernel describe flags Describe flags, kernel internal order\n"
 "            -a|--addr    addr-spec     Walk a range of pages\n"
 "            -b|--bits    bits-spec     Walk pages with specified bits\n"
 "            -p|--pid     pid           Walk process address space\n"
@@ -705,7 +805,7 @@ static void usage(void)
 "bit-names:\n"
 	);
 
-	for (i = 0, j = 0; i < ARRAY_SIZE(page_flag_names); i++) {
+	for (i = 0, j = 0; i < page_flag_nr; i++) {
 		if (!page_flag_names[i])
 			continue;
 		printf("%16s%s", page_flag_names[i] + 2,
@@ -836,7 +936,7 @@ static uint64_t parse_flag_name(const char *str, int len)
 	if (len <= 8 && !strncmp(str, "compound", len))
 		return BITS_COMPOUND;
 
-	for (i = 0; i < ARRAY_SIZE(page_flag_names); i++) {
+	for (i = 0; i < page_flag_nr; i++) {
 		if (!page_flag_names[i])
 			continue;
 		if (!strncmp(str, page_flag_names[i] + 2, len))
@@ -906,6 +1006,7 @@ static const struct option opts[] = {
 	{ "addr"      , 1, NULL, 'a' },
 	{ "bits"      , 1, NULL, 'b' },
 	{ "describe"  , 1, NULL, 'd' },
+	{ "kernel"    , 1, NULL, 'k' },
 	{ "list"      , 0, NULL, 'l' },
 	{ "list-each" , 0, NULL, 'L' },
 	{ "no-summary", 0, NULL, 'N' },
@@ -922,7 +1023,7 @@ int main(int argc, char *argv[])
 	page_size = getpagesize();
 
 	while ((c = getopt_long(argc, argv,
-				"rp:f:a:b:d:lLNXxh", opts, NULL)) != -1) {
+				"rp:f:a:b:d:k:lLNXxh", opts, NULL)) != -1) {
 		switch (c) {
 		case 'r':
 			opt_raw = 1;
@@ -939,6 +1040,10 @@ int main(int argc, char *argv[])
 		case 'b':
 			parse_bits_mask(optarg);
 			break;
+		case 'k':
+			/* Fall-through to case 'd' */
+			page_flag_names = (char **)page_flag_names_kernel;
+			page_flag_nr = __NR_PAGEFLAGS;
 		case 'd':
 			describe_flags(optarg);
 			exit(0);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] page-types: kernel pageflags mode

[Randy Dunlap]

On Fri, 04 Dec 2009 14:29:48 -0700 Alex Chiang wrote:

> An earlier commit taught page-types the -d|--describe argument, which
> allows the user to describe page flags passed on the command line:
> 
>   # ./Documentation/vm/page-types -d 0x4000
>   0x0000000000004000  ______________b___________________  swapbacked
> 
> In -d mode, page-types expects the page flag bits in the order generated
> by the kernel function get_uflags().
> 
> However, those bits are rearranged compared to what is actually stored
> in struct page->flags. A kernel developer dumping a page's flags
> using printk, e.g., may get misleading results in -d mode.
> 
> Teach page-types the -k mode, which parses and describes the bits in
> the internal kernel order:
> 
>   # ./Documentation/vm/page-types -k 0x4000
>   0x0000000000004000  ______________H_________  compound_head
> 
> Note that the recommended way to build page-types is from the top-level
> kernel source directory. This ensures that it will get the same CONFIG_*
> defines used to build the kernel source.
> 
>   # make Documentation/vm/
> 
> The implication is that attempting to use page-types -k on a kernel
> with different CONFIG_* settings may lead to surprising and misleading
> results. To retain sanity, always use the page-types built out of the
> kernel tree you are actually testing.
> 
> Cc: fengguang.wu@intel.com
> Cc: Haicheng Li <haicheng.li@intel.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: Randy Dunlap <randy.dunlap@oracle.com>
> Signed-off-by: Alex Chiang <achiang@hp.com>
> ---
> 
> Applies on top of mmotm.
> 
>  Documentation/vm/Makefile     |    2 +
>  Documentation/vm/page-types.c |  117 +++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 113 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/vm/Makefile b/Documentation/vm/Makefile
> index 5bd269b..1bebc43 100644
> --- a/Documentation/vm/Makefile
> +++ b/Documentation/vm/Makefile
> @@ -1,6 +1,8 @@
>  # kbuild trick to avoid linker error. Can be omitted if a module is built.
>  obj- := dummy.o
>  
> +HOSTCFLAGS_page-types.o += $(LINUXINCLUDE)
> +
>  # List of programs to build
>  hostprogs-y := slabinfo page-types

I can ack the Makefile part.  Thanks for the patch.

Not that I expect this patch to change for this, but I think that we
need to move tools (like this one) from Documentation/ to tools/ and
possibly move examples from Documentation/ to samples/ or tools/.



---
~Randy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[RFC] print symbolic page flag names in bad_page()

[Wu Fengguang]

Hi Alex,

On Sat, Dec 05, 2009 at 05:29:48AM +0800, Alex Chiang wrote:
[...]
> Teach page-types the -k mode, which parses and describes the bits in
> the internal kernel order:
> 
>   # ./Documentation/vm/page-types -k 0x4000
>   0x0000000000004000  ______________H_________  compound_head
[...]
> The implication is that attempting to use page-types -k on a kernel
> with different CONFIG_* settings may lead to surprising and misleading
> results. To retain sanity, always use the page-types built out of the
> kernel tree you are actually testing.

This is useful feature, however not as convenient if the kernel can
print its page flag names directly :)
(especially when the dmesg comes from some end user)

So how about this patch?
---

mm: introduce dump_page()

- introduce dump_page() to print the page info for debugging some error condition.
- print an extra field: the symbolic names of page->flags
- convert three mm users: bad_page(), print_bad_pte() and memory offline failure. 

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 mm/internal.h       |    2 +
 mm/memory.c         |    8 +---
 mm/memory_hotplug.c |    6 +--
 mm/page_alloc.c     |   75 +++++++++++++++++++++++++++++++++++++++---
 4 files changed, 78 insertions(+), 13 deletions(-)

--- linux-mm.orig/mm/page_alloc.c	2009-12-06 10:11:08.000000000 +0800
+++ linux-mm/mm/page_alloc.c	2009-12-06 11:22:58.000000000 +0800
@@ -48,6 +48,7 @@
 #include <linux/page_cgroup.h>
 #include <linux/debugobjects.h>
 #include <linux/kmemleak.h>
+#include <linux/kernel-page-flags.h>
 #include <trace/events/kmem.h>
 
 #include <asm/tlbflush.h>
@@ -262,10 +263,7 @@ static void bad_page(struct page *page)
 
 	printk(KERN_ALERT "BUG: Bad page state in process %s  pfn:%05lx\n",
 		current->comm, page_to_pfn(page));
-	printk(KERN_ALERT
-		"page:%p flags:%p count:%d mapcount:%d mapping:%p index:%lx\n",
-		page, (void *)page->flags, page_count(page),
-		page_mapcount(page), page->mapping, page->index);
+	dump_page(page);
 
 	dump_stack();
 out:
@@ -5106,3 +5104,72 @@ bool is_free_buddy_page(struct page *pag
 	return order < MAX_ORDER;
 }
 #endif
+
+static char *page_flag_names[] = {
+	[KPF_LOCKED]		= "L:locked",
+	[KPF_ERROR]		= "E:error",
+	[KPF_REFERENCED]	= "R:referenced",
+	[KPF_UPTODATE]		= "U:uptodate",
+	[KPF_DIRTY]		= "D:dirty",
+	[KPF_LRU]		= "l:lru",
+	[KPF_ACTIVE]		= "A:active",
+	[KPF_SLAB]		= "S:slab",
+	[KPF_WRITEBACK]		= "W:writeback",
+	[KPF_RECLAIM]		= "I:reclaim",
+	[KPF_BUDDY]		= "B:buddy",
+
+	[KPF_MMAP]		= "M:mmap",
+	[KPF_ANON]		= "a:anonymous",
+	[KPF_SWAPCACHE]		= "s:swapcache",
+	[KPF_SWAPBACKED]	= "b:swapbacked",
+	[KPF_COMPOUND_HEAD]	= "H:compound_head",
+	[KPF_COMPOUND_TAIL]	= "T:compound_tail",
+	[KPF_HUGE]		= "G:huge",
+	[KPF_UNEVICTABLE]	= "u:unevictable",
+	[KPF_HWPOISON]		= "X:hwpoison",
+	[KPF_NOPAGE]		= "n:nopage",
+	[KPF_KSM]		= "V:shared",
+
+	[KPF_RESERVED]		= "r:reserved",
+	[KPF_MLOCKED]		= "m:mlocked",
+	[KPF_MAPPEDTODISK]	= "d:mappedtodisk",
+	[KPF_PRIVATE]		= "P:private",
+	[KPF_PRIVATE_2]		= "p:private_2",
+	[KPF_OWNER_PRIVATE]	= "O:owner_private",
+	[KPF_ARCH]		= "h:arch",
+	[KPF_UNCACHED]		= "c:uncached",
+};
+
+static char *page_flags_longname(struct page *page, char *buf, int buflen)
+{
+	int i, n;
+	u64 flags;
+
+	flags = stable_page_flags(page);
+
+	for (i = 0, n = 0; i < ARRAY_SIZE(page_flag_names); i++) {
+		if (!page_flag_names[i])
+			continue;
+		if ((flags >> i) & 1)
+			n += snprintf(buf + n, buflen - n, "%s,",
+					page_flag_names[i] + 2);
+	}
+	if (n)
+		n--;
+	buf[n] = '\0';
+
+	return buf;
+}
+
+void dump_page(struct page *page)
+{
+	char buf[1024];
+
+	printk(KERN_ALERT
+	"page:%p flags:%p(%s) count:%d mapcount:%d mapping:%p index:%lx\n",
+		page, (void *)page->flags,
+		page_flags_longname(page, buf, sizeof(buf)),
+		page_count(page), page_mapcount(page),
+		page->mapping, page->index);
+}
+EXPORT_SYMBOL(dump_page);
--- linux-mm.orig/mm/memory.c	2009-12-06 10:37:47.000000000 +0800
+++ linux-mm/mm/memory.c	2009-12-06 10:57:25.000000000 +0800
@@ -430,12 +430,8 @@ static void print_bad_pte(struct vm_area
 		"BUG: Bad page map in process %s  pte:%08llx pmd:%08llx\n",
 		current->comm,
 		(long long)pte_val(pte), (long long)pmd_val(*pmd));
-	if (page) {
-		printk(KERN_ALERT
-		"page:%p flags:%p count:%d mapcount:%d mapping:%p index:%lx\n",
-		page, (void *)page->flags, page_count(page),
-		page_mapcount(page), page->mapping, page->index);
-	}
+	if (page)
+		dump_page(page);
 	printk(KERN_ALERT
 		"addr:%p vm_flags:%08lx anon_vma:%p mapping:%p index:%lx\n",
 		(void *)addr, vma->vm_flags, vma->anon_vma, mapping, index);
--- linux-mm.orig/mm/internal.h	2009-12-06 10:47:37.000000000 +0800
+++ linux-mm/mm/internal.h	2009-12-06 11:03:53.000000000 +0800
@@ -264,6 +264,8 @@ int __get_user_pages(struct task_struct 
 #define ZONE_RECLAIM_SUCCESS	1
 #endif
 
+void dump_page(struct page *page);
+
 extern int hwpoison_filter(struct page *p);
 
 extern u32 hwpoison_filter_dev_major;
--- linux-mm.orig/mm/memory_hotplug.c	2009-12-06 11:01:20.000000000 +0800
+++ linux-mm/mm/memory_hotplug.c	2009-12-06 11:01:23.000000000 +0800
@@ -678,9 +678,9 @@ do_migrate_range(unsigned long start_pfn
 			if (page_count(page))
 				not_managed++;
 #ifdef CONFIG_DEBUG_VM
-			printk(KERN_INFO "removing from LRU failed"
-					 " %lx/%d/%lx\n",
-				pfn, page_count(page), page->flags);
+			printk(KERN_INFO "removing pfn %lx from LRU failed\n",
+				pfn);
+			dump_page(page);
 #endif
 		}
 	}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC] print symbolic page flag names in bad_page()

[Andi Kleen]

> So how about this patch?

I like it. Decoding the flags by hand is always a very unpleasant experience.
Bonus: dump_page can be called from kgdb too.

-Andi


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC] print symbolic page flag names in bad_page()

[Alex Chiang]

* Andi Kleen <andi@firstfloor.org>:
> > So how about this patch?
> 
> I like it. Decoding the flags by hand is always a very unpleasant experience.
> Bonus: dump_page can be called from kgdb too.

This is fine by me too.

Thanks,
/ac

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC] print symbolic page flag names in bad_page()

[Wu Fengguang]

On Mon, Dec 07, 2009 at 07:00:16AM +0800, Andi Kleen wrote:
> > So how about this patch?
> 
> I like it. Decoding the flags by hand is always a very unpleasant experience.
> Bonus: dump_page can be called from kgdb too.

Thank you.  And I'd like to elaborate a bit more on the rational.

Making the page-types tool depend on .config is fragile and dangerous.
It would seem to work but silently return wrong results for a newbie
user or a careless hacker.

And it's troublesome even for home made kernels by a kernel developer.

For example, typically I run many kernel trees with different versions and
kconfigs (both of which change frequently) concurrently.  This means I
would have to judge to run "this" page-types or "that" page-types, and
to check if this page-types is uptodate, and if the .config is in sync
with the running kernel image..

An in-kernel dump_page() makes life easier.

Thanks,
Fengguang

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC] print symbolic page flag names in bad_page()

[Ingo Molnar]

* Andi Kleen <andi@firstfloor.org> wrote:

> > So how about this patch?
> 
> I like it. Decoding the flags by hand is always a very unpleasant 
> experience. Bonus: dump_page can be called from kgdb too.

Guys, please do more review:

> +void dump_page(struct page *page)
> +{
> +     char buf[1024];

NAK. This code causes a brutal, +1K kernel stack footprint spike that 
can crash a system _precisely_ when we are trying to dump a (presumably 
rare) condition.

> +EXPORT_SYMBOL(dump_page);

( Small detail: such exports are EXPORT_SYMBOL_GPL - we dont want random 
  external modules start using it. )

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC] print symbolic page flag names in bad_page()

[Mel Gorman]

On Sun, Dec 06, 2009 at 11:46:36AM +0800, Wu Fengguang wrote:
> Hi Alex,
> 
> On Sat, Dec 05, 2009 at 05:29:48AM +0800, Alex Chiang wrote:
> [...]
> > Teach page-types the -k mode, which parses and describes the bits in
> > the internal kernel order:
> > 
> >   # ./Documentation/vm/page-types -k 0x4000
> >   0x0000000000004000  ______________H_________  compound_head
> [...]
> > The implication is that attempting to use page-types -k on a kernel
> > with different CONFIG_* settings may lead to surprising and misleading
> > results. To retain sanity, always use the page-types built out of the
> > kernel tree you are actually testing.
> 
> This is useful feature, however not as convenient if the kernel can
> print its page flag names directly :)
> (especially when the dmesg comes from some end user)
> 
> So how about this patch?

There is a neat way of declaring strings that are used for the symbolic
printing of flags in a bitfield. It's orientated around ftrace so not
immediately usable for printk. Maybe the approaches can be merged but
minimally, it'd be nice to match the declaration style. An example is
show_gfp_flags in include/event/trace/kmem.h

> ---
> 
> mm: introduce dump_page()
> 
> - introduce dump_page() to print the page info for debugging some error condition.
> - print an extra field: the symbolic names of page->flags
> - convert three mm users: bad_page(), print_bad_pte() and memory offline failure. 
> 
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> ---
>  mm/internal.h       |    2 +
>  mm/memory.c         |    8 +---
>  mm/memory_hotplug.c |    6 +--
>  mm/page_alloc.c     |   75 +++++++++++++++++++++++++++++++++++++++---
>  4 files changed, 78 insertions(+), 13 deletions(-)
> 
> --- linux-mm.orig/mm/page_alloc.c	2009-12-06 10:11:08.000000000 +0800
> +++ linux-mm/mm/page_alloc.c	2009-12-06 11:22:58.000000000 +0800
> @@ -48,6 +48,7 @@
>  #include <linux/page_cgroup.h>
>  #include <linux/debugobjects.h>
>  #include <linux/kmemleak.h>
> +#include <linux/kernel-page-flags.h>
>  #include <trace/events/kmem.h>
>  
>  #include <asm/tlbflush.h>
> @@ -262,10 +263,7 @@ static void bad_page(struct page *page)
>  
>  	printk(KERN_ALERT "BUG: Bad page state in process %s  pfn:%05lx\n",
>  		current->comm, page_to_pfn(page));
> -	printk(KERN_ALERT
> -		"page:%p flags:%p count:%d mapcount:%d mapping:%p index:%lx\n",
> -		page, (void *)page->flags, page_count(page),
> -		page_mapcount(page), page->mapping, page->index);
> +	dump_page(page);
>  
>  	dump_stack();
>  out:
> @@ -5106,3 +5104,72 @@ bool is_free_buddy_page(struct page *pag
>  	return order < MAX_ORDER;
>  }
>  #endif
> +
> +static char *page_flag_names[] = {
> +	[KPF_LOCKED]		= "L:locked",
> +	[KPF_ERROR]		= "E:error",
> +	[KPF_REFERENCED]	= "R:referenced",
> +	[KPF_UPTODATE]		= "U:uptodate",
> +	[KPF_DIRTY]		= "D:dirty",
> +	[KPF_LRU]		= "l:lru",
> +	[KPF_ACTIVE]		= "A:active",
> +	[KPF_SLAB]		= "S:slab",
> +	[KPF_WRITEBACK]		= "W:writeback",
> +	[KPF_RECLAIM]		= "I:reclaim",
> +	[KPF_BUDDY]		= "B:buddy",
> +
> +	[KPF_MMAP]		= "M:mmap",
> +	[KPF_ANON]		= "a:anonymous",
> +	[KPF_SWAPCACHE]		= "s:swapcache",
> +	[KPF_SWAPBACKED]	= "b:swapbacked",
> +	[KPF_COMPOUND_HEAD]	= "H:compound_head",
> +	[KPF_COMPOUND_TAIL]	= "T:compound_tail",
> +	[KPF_HUGE]		= "G:huge",
> +	[KPF_UNEVICTABLE]	= "u:unevictable",
> +	[KPF_HWPOISON]		= "X:hwpoison",
> +	[KPF_NOPAGE]		= "n:nopage",
> +	[KPF_KSM]		= "V:shared",
> +
> +	[KPF_RESERVED]		= "r:reserved",
> +	[KPF_MLOCKED]		= "m:mlocked",
> +	[KPF_MAPPEDTODISK]	= "d:mappedtodisk",
> +	[KPF_PRIVATE]		= "P:private",
> +	[KPF_PRIVATE_2]		= "p:private_2",
> +	[KPF_OWNER_PRIVATE]	= "O:owner_private",
> +	[KPF_ARCH]		= "h:arch",
> +	[KPF_UNCACHED]		= "c:uncached",
> +};
> +
> +static char *page_flags_longname(struct page *page, char *buf, int buflen)
> +{
> +	int i, n;
> +	u64 flags;
> +
> +	flags = stable_page_flags(page);
> +

I don't see where this is defined. I assume there is a patch dependency
I'm not seeing.

> +	for (i = 0, n = 0; i < ARRAY_SIZE(page_flag_names); i++) {
> +		if (!page_flag_names[i])
> +			continue;

How it is possible to get flags that are not recognised? Surely that
that spark some sort of warning.

> +		if ((flags >> i) & 1)
> +			n += snprintf(buf + n, buflen - n, "%s,",
> +					page_flag_names[i] + 2);

Should that be | instead of , ?
Why page_flag_names[i] + 2?

> +	}
> +	if (n)
> +		n--;
> +	buf[n] = '\0';
> +
> +	return buf;
> +}

As the symbolic long name is always going to printk(), why buffer it?
It's slower to call printk() multiple times but it's hardly a
performance-critical path and it avoids the declaration of buf.

> +
> +void dump_page(struct page *page)
> +{
> +	char buf[1024];
> +

Ingo flagged this already.

> +	printk(KERN_ALERT
> +	"page:%p flags:%p(%s) count:%d mapcount:%d mapping:%p index:%lx\n",
> +		page, (void *)page->flags,
> +		page_flags_longname(page, buf, sizeof(buf)),

This can push things like count, mapcount and the like beyond the edge
of 80 columns. While it rarely matters, it can get truncated on serial
consoles depending on how they are being recorded. Can the string go to
a line of it's own?

> +		page_count(page), page_mapcount(page),
> +		page->mapping, page->index);
> +}
> +EXPORT_SYMBOL(dump_page);

Ingo pointed out that this is exported GPL but I'm confused as to why
it's exported at all. What module needs it?

> --- linux-mm.orig/mm/memory.c	2009-12-06 10:37:47.000000000 +0800
> +++ linux-mm/mm/memory.c	2009-12-06 10:57:25.000000000 +0800
> @@ -430,12 +430,8 @@ static void print_bad_pte(struct vm_area
>  		"BUG: Bad page map in process %s  pte:%08llx pmd:%08llx\n",
>  		current->comm,
>  		(long long)pte_val(pte), (long long)pmd_val(*pmd));
> -	if (page) {
> -		printk(KERN_ALERT
> -		"page:%p flags:%p count:%d mapcount:%d mapping:%p index:%lx\n",
> -		page, (void *)page->flags, page_count(page),
> -		page_mapcount(page), page->mapping, page->index);
> -	}
> +	if (page)
> +		dump_page(page);
>  	printk(KERN_ALERT
>  		"addr:%p vm_flags:%08lx anon_vma:%p mapping:%p index:%lx\n",
>  		(void *)addr, vma->vm_flags, vma->anon_vma, mapping, index);
> --- linux-mm.orig/mm/internal.h	2009-12-06 10:47:37.000000000 +0800
> +++ linux-mm/mm/internal.h	2009-12-06 11:03:53.000000000 +0800
> @@ -264,6 +264,8 @@ int __get_user_pages(struct task_struct 
>  #define ZONE_RECLAIM_SUCCESS	1
>  #endif
>  
> +void dump_page(struct page *page);
> +
>  extern int hwpoison_filter(struct page *p);
>  
>  extern u32 hwpoison_filter_dev_major;
> --- linux-mm.orig/mm/memory_hotplug.c	2009-12-06 11:01:20.000000000 +0800
> +++ linux-mm/mm/memory_hotplug.c	2009-12-06 11:01:23.000000000 +0800
> @@ -678,9 +678,9 @@ do_migrate_range(unsigned long start_pfn
>  			if (page_count(page))
>  				not_managed++;
>  #ifdef CONFIG_DEBUG_VM
> -			printk(KERN_INFO "removing from LRU failed"
> -					 " %lx/%d/%lx\n",
> -				pfn, page_count(page), page->flags);
> +			printk(KERN_INFO "removing pfn %lx from LRU failed\n",
> +				pfn);
> +			dump_page(page);

The message is printed at KERN_INFO and the dump at KERN_ALERT. Is this
intentional?

>  #endif
>  		}
>  	}
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [RFC] print symbolic page flag names in bad_page()

[Wu Fengguang]

Hi Mel,

Sorry for the delay!

On Mon, Dec 07, 2009 at 05:30:45PM +0800, Mel Gorman wrote:
> On Sun, Dec 06, 2009 at 11:46:36AM +0800, Wu Fengguang wrote:
> > Hi Alex,
> > 
> > On Sat, Dec 05, 2009 at 05:29:48AM +0800, Alex Chiang wrote:
> > [...]
> > > Teach page-types the -k mode, which parses and describes the bits in
> > > the internal kernel order:
> > > 
> > >   # ./Documentation/vm/page-types -k 0x4000
> > >   0x0000000000004000  ______________H_________  compound_head
> > [...]
> > > The implication is that attempting to use page-types -k on a kernel
> > > with different CONFIG_* settings may lead to surprising and misleading
> > > results. To retain sanity, always use the page-types built out of the
> > > kernel tree you are actually testing.
> > 
> > This is useful feature, however not as convenient if the kernel can
> > print its page flag names directly :)
> > (especially when the dmesg comes from some end user)
> > 
> > So how about this patch?
> 
> There is a neat way of declaring strings that are used for the symbolic
> printing of flags in a bitfield. It's orientated around ftrace so not
> immediately usable for printk. Maybe the approaches can be merged but
> minimally, it'd be nice to match the declaration style. An example is
> show_gfp_flags in include/event/trace/kmem.h

Good tip, thanks!

Converting to the (mask, name) array almost doubles the space,
however that makes it usable to ftrace users, so I converted the
plain string array page_flag_names[] to "struct trace_print_flags *".

> > ---
> > 
> > mm: introduce dump_page()
> > 
> > - introduce dump_page() to print the page info for debugging some error condition.
> > - print an extra field: the symbolic names of page->flags
> > - convert three mm users: bad_page(), print_bad_pte() and memory offline failure. 
> > 
> > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> > ---
> >  mm/internal.h       |    2 +
> >  mm/memory.c         |    8 +---
> >  mm/memory_hotplug.c |    6 +--
> >  mm/page_alloc.c     |   75 +++++++++++++++++++++++++++++++++++++++---
> >  4 files changed, 78 insertions(+), 13 deletions(-)
> > 
> > --- linux-mm.orig/mm/page_alloc.c	2009-12-06 10:11:08.000000000 +0800
> > +++ linux-mm/mm/page_alloc.c	2009-12-06 11:22:58.000000000 +0800
> > @@ -48,6 +48,7 @@
> >  #include <linux/page_cgroup.h>
> >  #include <linux/debugobjects.h>
> >  #include <linux/kmemleak.h>
> > +#include <linux/kernel-page-flags.h>
> >  #include <trace/events/kmem.h>
> >  
> >  #include <asm/tlbflush.h>
> > @@ -262,10 +263,7 @@ static void bad_page(struct page *page)
> >  
> >  	printk(KERN_ALERT "BUG: Bad page state in process %s  pfn:%05lx\n",
> >  		current->comm, page_to_pfn(page));
> > -	printk(KERN_ALERT
> > -		"page:%p flags:%p count:%d mapcount:%d mapping:%p index:%lx\n",
> > -		page, (void *)page->flags, page_count(page),
> > -		page_mapcount(page), page->mapping, page->index);
> > +	dump_page(page);
> >  
> >  	dump_stack();
> >  out:
> > @@ -5106,3 +5104,72 @@ bool is_free_buddy_page(struct page *pag
> >  	return order < MAX_ORDER;
> >  }
> >  #endif
> > +
> > +static char *page_flag_names[] = {
> > +	[KPF_LOCKED]		= "L:locked",
> > +	[KPF_ERROR]		= "E:error",
> > +	[KPF_REFERENCED]	= "R:referenced",
> > +	[KPF_UPTODATE]		= "U:uptodate",
> > +	[KPF_DIRTY]		= "D:dirty",
> > +	[KPF_LRU]		= "l:lru",
> > +	[KPF_ACTIVE]		= "A:active",
> > +	[KPF_SLAB]		= "S:slab",
> > +	[KPF_WRITEBACK]		= "W:writeback",
> > +	[KPF_RECLAIM]		= "I:reclaim",
> > +	[KPF_BUDDY]		= "B:buddy",
> > +
> > +	[KPF_MMAP]		= "M:mmap",
> > +	[KPF_ANON]		= "a:anonymous",
> > +	[KPF_SWAPCACHE]		= "s:swapcache",
> > +	[KPF_SWAPBACKED]	= "b:swapbacked",
> > +	[KPF_COMPOUND_HEAD]	= "H:compound_head",
> > +	[KPF_COMPOUND_TAIL]	= "T:compound_tail",
> > +	[KPF_HUGE]		= "G:huge",
> > +	[KPF_UNEVICTABLE]	= "u:unevictable",
> > +	[KPF_HWPOISON]		= "X:hwpoison",
> > +	[KPF_NOPAGE]		= "n:nopage",
> > +	[KPF_KSM]		= "V:shared",
> > +
> > +	[KPF_RESERVED]		= "r:reserved",
> > +	[KPF_MLOCKED]		= "m:mlocked",
> > +	[KPF_MAPPEDTODISK]	= "d:mappedtodisk",
> > +	[KPF_PRIVATE]		= "P:private",
> > +	[KPF_PRIVATE_2]		= "p:private_2",
> > +	[KPF_OWNER_PRIVATE]	= "O:owner_private",
> > +	[KPF_ARCH]		= "h:arch",
> > +	[KPF_UNCACHED]		= "c:uncached",
> > +};
> > +
> > +static char *page_flags_longname(struct page *page, char *buf, int buflen)
> > +{
> > +	int i, n;
> > +	u64 flags;
> > +
> > +	flags = stable_page_flags(page);
> > +
> 
> I don't see where this is defined. I assume there is a patch dependency
> I'm not seeing.

It's posted several days ago:
        http://lkml.org/lkml/2009/12/1/470

It's effectively the internal page flags plus some useful fake ones,
eg. PageAnon(), PageKsm(), PageHuge().

It's not a big deal - developers can more or less deduce that for
themselves. And there are two problems:
- stable_page_flags() currently depends on CONFIG_PROC_PAGE_MONITOR
- stable_page_flags() uses u64 flags, which is incompatible with
  struct trace_print_flags.

So I'll use plain page flags instead.

> > +	for (i = 0, n = 0; i < ARRAY_SIZE(page_flag_names); i++) {
> > +		if (!page_flag_names[i])
> > +			continue;
> 
> How it is possible to get flags that are not recognised? Surely that
> that spark some sort of warning.

OK. I'll print the remained raw bits just like ftrace_print_flags_seq().
(In fact I copied the main logic from ftrace_print_flags_seq() :)

> > +		if ((flags >> i) & 1)
> > +			n += snprintf(buf + n, buflen - n, "%s,",
> > +					page_flag_names[i] + 2);
> 
> Why page_flag_names[i] + 2?

Sorry, the code (and string array) is copied from the user space tool
Documentation/vm/page-types.c ... now completed rewritten based on the
ftrace function.

> Should that be | instead of , ?

page-types chose "," because the pipe symbol "|" is not that user
friendly.  "|" is more developer friendly though, so will use it.

> > +	}
> > +	if (n)
> > +		n--;
> > +	buf[n] = '\0';
> > +
> > +	return buf;
> > +}
> 
> As the symbolic long name is always going to printk(), why buffer it?
> It's slower to call printk() multiple times but it's hardly a
> performance-critical path and it avoids the declaration of buf.
> 
> > +
> > +void dump_page(struct page *page)
> > +{
> > +	char buf[1024];
> > +
> 
> Ingo flagged this already.

Removed now.

> > +	printk(KERN_ALERT
> > +	"page:%p flags:%p(%s) count:%d mapcount:%d mapping:%p index:%lx\n",
> > +		page, (void *)page->flags,
> > +		page_flags_longname(page, buf, sizeof(buf)),
> 
> This can push things like count, mapcount and the like beyond the edge
> of 80 columns. While it rarely matters, it can get truncated on serial
> consoles depending on how they are being recorded. Can the string go to
> a line of it's own?

OK, I make a standalone line for flags (showing both hex and string).

> > +		page_count(page), page_mapcount(page),
> > +		page->mapping, page->index);
> > +}
> > +EXPORT_SYMBOL(dump_page);
> 
> Ingo pointed out that this is exported GPL but I'm confused as to why
> it's exported at all. What module needs it?

I'm testing dump_page() in a kernel module ;)  Will remove the export for now.

> > --- linux-mm.orig/mm/memory.c	2009-12-06 10:37:47.000000000 +0800
> > +++ linux-mm/mm/memory.c	2009-12-06 10:57:25.000000000 +0800
> > @@ -430,12 +430,8 @@ static void print_bad_pte(struct vm_area
> >  		"BUG: Bad page map in process %s  pte:%08llx pmd:%08llx\n",
> >  		current->comm,
> >  		(long long)pte_val(pte), (long long)pmd_val(*pmd));
> > -	if (page) {
> > -		printk(KERN_ALERT
> > -		"page:%p flags:%p count:%d mapcount:%d mapping:%p index:%lx\n",
> > -		page, (void *)page->flags, page_count(page),
> > -		page_mapcount(page), page->mapping, page->index);
> > -	}
> > +	if (page)
> > +		dump_page(page);
> >  	printk(KERN_ALERT
> >  		"addr:%p vm_flags:%08lx anon_vma:%p mapping:%p index:%lx\n",
> >  		(void *)addr, vma->vm_flags, vma->anon_vma, mapping, index);
> > --- linux-mm.orig/mm/internal.h	2009-12-06 10:47:37.000000000 +0800
> > +++ linux-mm/mm/internal.h	2009-12-06 11:03:53.000000000 +0800
> > @@ -264,6 +264,8 @@ int __get_user_pages(struct task_struct 
> >  #define ZONE_RECLAIM_SUCCESS	1
> >  #endif
> >  
> > +void dump_page(struct page *page);
> > +
> >  extern int hwpoison_filter(struct page *p);
> >  
> >  extern u32 hwpoison_filter_dev_major;
> > --- linux-mm.orig/mm/memory_hotplug.c	2009-12-06 11:01:20.000000000 +0800
> > +++ linux-mm/mm/memory_hotplug.c	2009-12-06 11:01:23.000000000 +0800
> > @@ -678,9 +678,9 @@ do_migrate_range(unsigned long start_pfn
> >  			if (page_count(page))
> >  				not_managed++;
> >  #ifdef CONFIG_DEBUG_VM
> > -			printk(KERN_INFO "removing from LRU failed"
> > -					 " %lx/%d/%lx\n",
> > -				pfn, page_count(page), page->flags);
> > +			printk(KERN_INFO "removing pfn %lx from LRU failed\n",
> > +				pfn);
> > +			dump_page(page);
> 
> The message is printed at KERN_INFO and the dump at KERN_ALERT. Is this
> intentional?

Good catch, it looks more consistent to convert the original KERN_INFO
to KERN_ALERT.

Updated patch follows, thanks for the helpful review!

Thanks,
Fengguang

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>