On Thu, Jul 02, 2020 at 04:59:14PM +0200, Vlastimil Babka wrote:
> On 6/30/20 1:59 PM, Matthew Wilcox wrote:
> > 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.
>
> Hmm, I guess that works as long as head pages really have index aligned to 1 <<
> order ... they should, right?
>
> But does it fail for HugeTLB? CC Kirill (git blamed) and Mike.
> page_to_pgoff() has for PageHeadHuge this:
> return page->index << compound_order(page)
>
> but page_to_index() does simply
> pgoff = compound_head(page)->index;
> pgoff += page - compound_head(page);
>
> Shouldn't it also do a <<compound_order(head) for HugeTLB?
PageHeadHuge() is only true for head pages, so we are fine here.
>
> >> > [ 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" }, \
>
> Dunno about this, "compound" used to always mean "head or tail" AFAIK.
Yeah. I'm not convinced by the change either. show_page_flags() also uses
this difinitions and I think it can get confusing in the trace.
--
Kirill A. Shutemov
