On Mon, Feb 10, 2020 at 11:50:21AM -0800, John Hubbard wrote:
> On 2/10/20 4:42 AM, Kirill A. Shutemov wrote:
> ...
> >> @@ -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);
> >
> > This is wrong. We want to see mapcount for the tail page, not head.
> >
>
> I see what you mean: page_mapcount(page) sums up both the page's and the
> head page's mapcount in some cases. The function doesn't seem to work
> correctly unless it is fed the tail page.
What makes you think this? It has to be called on the page user provided.
Head or tail.
> Here, even though the "head" variable's meaning is overloaded (=="head page,
> unless the tail page was corrupted, in which case, tail page"), it would still be
> accurate to change that line back to the original line, so that it once again
> reads:
>
> mapcount = PageSlab(page) ? 0 : page_mapcount(page);
PageSlab() can be called for the head page.
>
> Matthew?
>
> (Also, I see that __page_mapcount is EXPORT-ed, which is odd: nothing uses it
> other than page_mapcount.
... and page_mapcount() can be inlined anywhere.
--
Kirill A. Shutemov
