On 2/7/20 9:27 AM, Matthew Wilcox wrote: ...
A definite improvement, but I think we could do better. For example, you've changed PageCompound to PageTail here, whereas we really do want to dump some more information for PageHead pages than the plain vanilla order-0 page has. Another thing is that page_mapping() calls compound_head(), so if the page is corrupted, we're going to get a funky pointer dereference.
I spent a bit of time on this reimplementation ... what do you think?
It looks fine to me. I gave it a quick spin, here's the output for a normal and a huge page, and it has everything we want to see:
page:ffffea0010f0d640 refcount:1025 mapcount:1 mapping:0000000021857089 index:0xed anon flags: 0x17ffe0000080036(referenced|uptodate|lru|active|swapbacked) raw: 017ffe0000080036 ffffea0011731f08 ffffea0011730008 ffff8884777272c1 raw: 00000000000000ed 0000000000000000 0000040100000000 0000000000000000 page dumped because: testing dump_page()
page:ffffea0010ef1b80 head:ffffea0010ef0000 refcount:0 mapcount:1 mapping:00000000a8e1c7fa index:0xed order:9 compound_mapcount: 1 anon flags: 0x17ffe0000000000() raw: 017ffe0000000000 ffffea0010ef0001 ffffea0010ef1b88 dead000000000400 raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000 head: 017ffe0000090036 ffffea0011734548 ffffea0010ef8008 ffff8884777271b9 head: 000000000000007f 0000000000000000 00000201ffffffff 0000000000000000 page dumped because: testing dump_page()
- Print the mapping pointer using %p insted of %px. The actual value of the pointer can be read out of the raw page dump and using %p gives a chance to correlate it to earlier printk of the mapping pointer.
- Add the order of the page for compound pages
- Dump the raw head page as well as the raw page being dumped
diff --git a/mm/debug.c b/mm/debug.c index ecccd9f17801..0564d4cb8233 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -44,8 +44,10 @@ const struct trace_print_flags vmaflag_names[] = { void __dump_page(struct page *page, const char *reason) {
- struct page *head = compound_head(page); struct address_space *mapping; bool page_poisoned = PagePoisoned(page);
- bool compound = PageCompound(page); /*
- Accessing the pageblock without the zone lock. It could change to
- "isolate" again in the meantime, but since we are just dumping the
@@ -66,25 +68,32 @@ void __dump_page(struct page *page, const char *reason) goto hex_only; }
- mapping = page_mapping(page);
- if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) {
/* Corrupt page, cannot call page_mapping */mapping = page->mapping;head = page;compound = false;- } else {
mapping = page_mapping(page);- }
/* * Avoid VM_BUG_ON() in page_mapcount(). * page->_mapcount space in struct page is used by sl[aou]b pages to * encode own info. */
- mapcount = PageSlab(page) ? 0 : page_mapcount(page);
- mapcount = PageSlab(head) ? 0 : page_mapcount(head);
- if (PageCompound(page))
pr_warn("page:%px refcount:%d mapcount:%d mapping:%px ""index:%#lx compound_mapcount: %d\n",page, page_ref_count(page), mapcount,
- if (compound)
pr_warn("page:%px head:%px refcount:%d mapcount:%d mapping:%p ""index:%#lx order:%u compound_mapcount: %d\n",page, head, page_ref_count(page), mapcount, page->mapping, page_to_pgoff(page),
compound_mapcount(page));
compound_order(head), compound_mapcount(page));
pr_warn("page:%px refcount:%d mapcount:%d mapping:%px index:%#lx\n",
pr_warn("page:%px refcount:%d mapcount:%d mapping:%p index:%#lx\n", page, page_ref_count(page), mapcount,
page->mapping, page_to_pgoff(page));
mapping, page_to_pgoff(page));@@ -106,6 +115,10 @@ void __dump_page(struct page *page, const char *reason) print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32, sizeof(unsigned long), page, sizeof(struct page), false);
- if (!page_poisoned && compound)
print_hex_dump(KERN_WARNING, "head: ", DUMP_PREFIX_NONE, 32,sizeof(unsigned long), head,sizeof(struct page), false);
Good thought to get the hex dump of the head page in this case, yes.
if (reason) pr_warn("page dumped because: %s\n", reason);
Seeing as how I want to further enhance dump_page() slightly for this series (to include the 3rd struct page's hpage_pincount), would you care to send this as a formal patch that I could insert into this series, to replace patch 5?
thanks,