On Tue, Jun 30, 2020 at 10:02:50AM +0200, Vlastimil Babka wrote:
> I would also prefer this approach.It would be also nice to know the tail index.
> As long as page pointer wasn't hashed, it was possible to figure this out, but
> now it's not. Maybe print pfn of both page and head?
> On 6/30/20 1:35 AM, John Hubbard wrote:
> > ...so with that fix, along with your line break approach in the other thread,
> > a tail page dump of a FOLL_PIN page looks like this:
> >
> > [ 38.027987] page:00000000abaef9ae refcount:513 mapcount:1 mapping:0000000000000000 index:0x11
index is the last thing printed on this line. If it's something like
index:0x12345678, you can reduce it mod 1<<order, as printed on the next
line.
> > [ 38.035633] head:00000000675be53c order:9 compound_mapcount:1 compound_pincount:512
> > [ 38.049155] anon flags: 0x17ffe000000000e(referenced|uptodate|dirty)
> > [ 38.055465] raw: 017ffe0000000000 ffffea0020dd0001 ffffea0020dd0448 dead000000000400
> > [ 38.062319] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
> > [ 38.069183] head: 017ffe000001000e ffffffff83649ca0 ffffea0020dd8008 ffff88888e0b6641
> > [ 38.076141] head: 0000000000000000 0000000000000000 00000201ffffffff 0000000000000000
> > [ 38.083102] page dumped because: gup_benchmark: tail page: dump_page test
> >
> > So, good. However, I feel that the "head " prefix approach is slightly
> > preferable, because it's doing less processing (the more code one
> > adds to little-exercised debug paths, the more likely the debugging has
> > bugs) and is instead just printing out what it sees directly. And it seems a little
> > odd to remove the PG_head bit from the output.
> >
> > The "head " prefix approach looks like this:
>
> I would also prefer this approach.It would be also nice to know the tail index.
> As long as page pointer wasn't hashed, it was possible to figure this out, but
> now it's not. Maybe print pfn of both page and head?
>
> > [ 38.027987] page:00000000abaef9ae refcount:513 mapcount:1 mapping:0000000000000000 index:0x11
> > [ 38.035633] head:00000000675be53c order:9 compound_mapcount:1 compound_pincount:512
> > [ 38.049155] head anon flags: 0x17ffe000000000e(referenced|uptodate|dirty|head)
How about ...
[ 38.049155] flags: 0x17ffe000000000e(anon|referenced|uptodate|dirty|compound)
That is, change pageflag_names[] to print 'head' as 'compound' and move the
'anon' or 'ksm' to look like a pageflag. Also CMA. Like this:
+++ b/include/trace/events/mmflags.h
@@ -96,7 +96,7 @@
{1UL << PG_private, "private" }, \
{1UL << PG_private_2, "private_2" }, \
{1UL << PG_writeback, "writeback" }, \
- {1UL << PG_head, "head" }, \
+ {1UL << PG_head, "compound" }, \
{1UL << PG_mappedtodisk, "mappedtodisk" }, \
{1UL << PG_reclaim, "reclaim" }, \
{1UL << PG_swapbacked, "swapbacked" }, \
+++ b/mm/debug.c
@@ -54,7 +54,7 @@ void __dump_page(struct page *page, const char *reason)
* state for debugging, it should be fine to accept a bit of
* inaccuracy here due to racing.
*/
- bool page_cma = is_migrate_cma_page(page);
+ char *cma = is_migrate_cma_page(page) ? "|cma" : "";
int mapcount;
char *type = "";
@@ -71,6 +71,10 @@ void __dump_page(struct page *page, const char *reason)
if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) {
/* Corrupt page, cannot call page_mapping */
mapping = page->mapping;
+ if ((unsigned long)mapping & PAGE_MAPPING_ANON)
+ mapping = NULL;
+ mapping = (struct address_space *)
+ ((unsigned long)mapping & ~PAGE_MAPPING_FLAGS);
head = page;
compound = false;
} else {
[...]
if (PageKsm(page))
- type = "ksm ";
+ type = "ksm|";
else if (PageAnon(page))
- type = "anon ";
- else if (mapping) {
+ type = "anon|";
+
+ BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1);
+ pr_warn("flags: %#lx(%s%pGp%s)\n", page->flags, type, &page->flags,
+ cma);
+
+ if (mapping) {
const struct inode *host;
const struct address_space_operations *a_ops;
@@ -163,11 +167,6 @@ void __dump_page(struct page *page, const char *reason)
}
}
out_mapping:
- BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1);
-
- pr_warn("%sflags: %#lx(%pGp)%s\n", type, head->flags, &head->flags,
- page_cma ? " CMA" : "");
-
hex_only:
print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
sizeof(unsigned long), page,
Can also delete the 'out_mapping' label this way.
(I really like it that we're debating this ... it feels like we've been
in a bit of a push-pull with the debug patches over the years)
