On Wed, 26 May 2021, Yang Shi wrote:

On Tue, May 25, 2021 at 4:58 PM Hugh Dickins [off-list ref] wrote:

quoted

On Tue, 25 May 2021, Yang Shi wrote:

quoted

We should be able to make dump_page() print total mapcount, right? The
dump_page() should be just called in some error paths so taking some
extra overhead to dump more information seems harmless, or am I
missing something? Of course, this can be done in a separate patch.

I didn't want to ask that of you, but yes, if you're willing to add
total_mapcount() into dump_page(), I think that would be ideal; and
could be helpful for other cases too.

Looking through total_mapcount(), I think it's safe to call from
dump_page() - I always worry about extending crash info with
something that depends on a maybe-corrupted pointer which would
generate a further crash and either recurse or truncate the output -
but please check that carefully.

Yes, it is possible. If the THP is being split, some VM_BUG_* might be
triggered if total_mapcount() is called. But it is still feasible to
print total mapcount as long as we implement a more robust version for
dump_page().

Oh dear. I think the very last thing the kernel needs is yet another
subtly different variant of *mapcount*().

Do you have a specific VM_BUG_* in mind there?  Of course there's
the VM_BUG_ON_PAGE(PageTail) at the start of it, and you'd want to
print total_mapcount(head) to avoid that one.

Looks like __dump_page() is already careful about "head", checking
whether "page" is within the expected bounds.  Of course, once we're
in serious VM_WARN territory, there might be races which could flip
fields midway: PageTail set by the time it reaches total_mapcount()?
Narrow the race (rather like it does with PageSlab) by testing
PageTail immediately before calling total_mapcount(head)?

Hugh