On Wed, 26 May 2021, Yang Shi wrote: > On Tue, May 25, 2021 at 4:58 PM Hugh Dickins <hughd@xxxxxxxxxx> wrote: > > On Tue, 25 May 2021, Yang Shi wrote: > > > > > > 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
