From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 956A9C433DF for ; Thu, 2 Jul 2020 14:59:19 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3550120737 for ; Thu, 2 Jul 2020 14:59:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3550120737 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id AA3D36B0096; Thu, 2 Jul 2020 10:59:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A54936B0098; Thu, 2 Jul 2020 10:59:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 993446B0099; Thu, 2 Jul 2020 10:59:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0228.hostedemail.com [216.40.44.228]) by kanga.kvack.org (Postfix) with ESMTP id 844386B0096 for ; Thu, 2 Jul 2020 10:59:18 -0400 (EDT) Received: from smtpin06.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 0B718180AD806 for ; Thu, 2 Jul 2020 14:59:18 +0000 (UTC) X-FDA: 76993443996.06.swim15_44176ad26e8a Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin06.hostedemail.com (Postfix) with ESMTP id D00FE1004A598 for ; Thu, 2 Jul 2020 14:59:17 +0000 (UTC) X-HE-Tag: swim15_44176ad26e8a X-Filterd-Recvd-Size: 7563 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf19.hostedemail.com (Postfix) with ESMTP for ; Thu, 2 Jul 2020 14:59:17 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 9C2A9AD77; Thu, 2 Jul 2020 14:59:15 +0000 (UTC) Subject: Re: [PATCH 1/3] mm: Print head flags in dump_page To: Matthew Wilcox Cc: John Hubbard , linux-mm@kvack.org, Andrew Morton , "Kirill A. Shutemov" , Mike Kravetz References: <20200629151918.15537-1-willy@infradead.org> <20200629151918.15537-2-willy@infradead.org> <20200629225134.GL25523@casper.infradead.org> <29baf5ca-1187-e00a-ee5c-5f08f7b69683@nvidia.com> <49e5da43-88bd-bac5-4dcc-5f5abb0d9a1b@suse.cz> <20200630115950.GM25523@casper.infradead.org> From: Vlastimil Babka Message-ID: Date: Thu, 2 Jul 2020 16:59:14 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: <20200630115950.GM25523@casper.infradead.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: D00FE1004A598 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam01 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: 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< 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 <> > [ 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. > {1UL << PG_mappedtodisk, "mappedtodisk" }, \ > {1UL << PG_reclaim, "reclaim" }, \ > {1UL << PG_swapbacked, "swapbacked" }, \ > +++ b/mm/debug.c > @@ -54,7 +54,7 @@ void __dump_page(struct page *page, const char *reason) > * state for debugging, it should be fine to accept a bit of > * inaccuracy here due to racing. > */ > - bool page_cma = is_migrate_cma_page(page); > + char *cma = is_migrate_cma_page(page) ? "|cma" : ""; > int mapcount; > char *type = ""; > > @@ -71,6 +71,10 @@ void __dump_page(struct page *page, const char *reason) > if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) { > /* Corrupt page, cannot call page_mapping */ > mapping = page->mapping; > + if ((unsigned long)mapping & PAGE_MAPPING_ANON) > + mapping = NULL; > + mapping = (struct address_space *) > + ((unsigned long)mapping & ~PAGE_MAPPING_FLAGS); > head = page; > compound = false; > } else { > [...] > if (PageKsm(page)) > - type = "ksm "; > + type = "ksm|"; > else if (PageAnon(page)) > - type = "anon "; > - else if (mapping) { > + type = "anon|"; > + > + BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1); > + pr_warn("flags: %#lx(%s%pGp%s)\n", page->flags, type, &page->flags, > + cma); > + > + if (mapping) { > const struct inode *host; > const struct address_space_operations *a_ops; > @@ -163,11 +167,6 @@ void __dump_page(struct page *page, const char *reason) > } > } > out_mapping: > - BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1); > - > - pr_warn("%sflags: %#lx(%pGp)%s\n", type, head->flags, &head->flags, > - page_cma ? " CMA" : ""); > - > hex_only: > print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32, > sizeof(unsigned long), page, > > Can also delete the 'out_mapping' label this way. > > (I really like it that we're debating this ... it feels like we've been > in a bit of a push-pull with the debug patches over the years) >