Re: [PATCH 1/3] mm: Print head flags in dump_page

["Kirill A. Shutemov" <kirill@shutemov.name>]

On Thu, Jul 02, 2020 at 06:19:08PM +0200, Vlastimil Babka wrote:
> On 7/2/20 5:53 PM, Kirill A. Shutemov wrote:
> > 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.
> 
> Really? We are calculatting index (pgoff) of tail page, which should be index of
> head page plus n for n'th tail page; the unit is base pages.
> But ff HugeTLB head pages use the unit of huge page in page->index, and
> page_to_pgoff() translates it to unit of base pages, then we should do the same
> when calculating the index of tail page, no? Otherwise we are adding up units of
> huge pages (from head->index) with units of base page (n'th tail) and get
> garbage as a result, AFAICS?

You are right. I guess we can get away with this because nobody calls
page_to_pgoff() on tail pages of hugetlb page. Except when something goes
wrong and dump_page() has to deal with it.

I'm not sure if it's worth fixing and whether the fix should be inside
page_to_pgoff().

The best fix would be to remove hugetlb pages special-casing: it has to
have ->index in base pagesize, not huge page.

-- 
 Kirill A. Shutemov