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=-3.8 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS 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 460ACC433DF for ; Mon, 29 Jun 2020 22:35:59 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EDD08206C3 for ; Mon, 29 Jun 2020 22:35:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="BZh9PSjn" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EDD08206C3 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9BE808D0011; Mon, 29 Jun 2020 18:35:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 96D568D000F; Mon, 29 Jun 2020 18:35:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 85B418D0011; Mon, 29 Jun 2020 18:35:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0148.hostedemail.com [216.40.44.148]) by kanga.kvack.org (Postfix) with ESMTP id 7070A8D000F for ; Mon, 29 Jun 2020 18:35:58 -0400 (EDT) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 26F14180AD81A for ; Mon, 29 Jun 2020 22:35:58 +0000 (UTC) X-FDA: 76983708396.23.watch18_2b0d8bd26e73 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin23.hostedemail.com (Postfix) with ESMTP id 0174837604 for ; Mon, 29 Jun 2020 22:35:57 +0000 (UTC) X-HE-Tag: watch18_2b0d8bd26e73 X-Filterd-Recvd-Size: 6677 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf35.hostedemail.com (Postfix) with ESMTP for ; Mon, 29 Jun 2020 22:35:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=+Lvs9c46Y3UqSJRPycT/xhxLe2SyeuYSKqHoRgwqt+o=; b=BZh9PSjn5bHJqj4d3BTG7kJDMr /z5jLZ3oPBih+SAY+fXm5bqUkTveUgviTvqWR9C0UQ/4Q6YjjdaNEsKkIYZQ8tKkDlSID/DGHzUws IKq4CKDtFwTP409rE/Eyg5J0DmF/u+Rn3pAz8dicg/XWo/DycLCrAPtp50Mo4L3WrpuEvYD+aQWnm gpru7KQqtrePcmeS6fQdZxUiaWWIYyiD+cSrZ3t3IMeWPjmNbf9TAvtGTRPkJ4UIHQlXt7fZCM/+K WAhzzhFpuPmMBGXsfIjPFD7QCP7wMYXEQl2LDAO2PMmfTtzFwXHOInBZgI2OdqRWL6xBHfDIyR1xs qAw9Ma/w==; Received: from willy by casper.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1jq2NT-0002yE-1u; Mon, 29 Jun 2020 22:35:51 +0000 Date: Mon, 29 Jun 2020 23:35:50 +0100 From: Matthew Wilcox To: John Hubbard Cc: linux-mm@kvack.org, Andrew Morton , Vlastimil Babka Subject: Re: [PATCH 0/3] Improvements for dump_page() Message-ID: <20200629223550.GK25523@casper.infradead.org> References: <20200629151918.15537-1-willy@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 0174837604 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam03 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 Mon, Jun 29, 2020 at 02:55:21PM -0700, John Hubbard wrote: > Cool! I've got a few minor feedback comments coming for the individual > patches, but an easy question first: > > For the FOLL_PIN pages, I left in a minor mess: the lines are basically > 2x too long. Would you be interested in folding in something approximately > like this, below, to one of your patches? It's just a line break, effectively. > It generates output like this: > > [ 260.545212] page:0000000035202f6e refcount:513 mapcount:1 mapping:0000000000000000 index:0x0 > [ 260.552834] head:0000000035202f6e order:9 compound_mapcount:1 compound_pincount:512 > [ 260.560087] anon flags: 0x17ffe000001000e(referenced|uptodate|dirty|head) > [ 260.565993] raw: 017ffe000001000e ffffffff83649ca0 ffffea0020758008 ffff888890f03839 > [ 260.572888] raw: 0000000000000000 0000000000000000 00000201ffffffff 0000000000000000 > [ 260.579745] page dumped because: gup_benchmark: head page: dump_page test > > (I'm not sure if the indentation is a good idea or not, actually.) > > Or if it's too much fuss, I can send it separately with your series as a > prerequisite: > > diff --git a/mm/debug.c b/mm/debug.c > index fcf3a16902b2..d522bf24e3ff 100644 > --- a/mm/debug.c > +++ b/mm/debug.c > @@ -87,19 +87,22 @@ void __dump_page(struct page *page, const char *reason) > if (compound) > if (hpage_pincount_available(page)) { > pr_warn("page:%p refcount:%d mapcount:%d mapping:%p " > - "index:%#lx head:%p order:%u " > - "compound_mapcount:%d compound_pincount:%d\n", > + "index:%#lx\n", > page, page_ref_count(head), mapcount, > - mapping, page_to_pgoff(page), head, > - compound_order(head), compound_mapcount(page), > + mapping, page_to_pgoff(page)); > + pr_warn(" head:%p order:%u compound_mapcount:%d " > + "compound_pincount:%d\n", > + head, compound_order(head), > + compound_mapcount(page), > compound_pincount(page)); > } else { > pr_warn("page:%p refcount:%d mapcount:%d mapping:%p " > - "index:%#lx head:%p order:%u " > - "compound_mapcount:%d\n", > - page, page_ref_count(head), mapcount, > - mapping, page_to_pgoff(page), head, > - compound_order(head), compound_mapcount(page)); > + "index:%#lx\n", > + page, page_ref_count(head), mapcount, mapping, > + page_to_pgoff(page)); > + pr_warn(" head:%p order:%u compound_mapcount:%d\n", > + head, compound_order(head), > + compound_mapcount(page)); > } Hmm ... If we're going to two lines, then I'd rather do something like this: +++ b/mm/debug.c @@ -84,27 +84,22 @@ void __dump_page(struct page *page, const char *reason) */ mapcount = PageSlab(head) ? 0 : page_mapcount(page); - if (compound) + pr_warn("page:%p refcount:%d mapcount:%d mapping:%p index:%#lx\n", + page, page_ref_count(head), mapcount, mapping, + page_to_pgoff(page)); + if (compound) { if (hpage_pincount_available(page)) { - pr_warn("page:%p refcount:%d mapcount:%d mapping:%p " - "index:%#lx head:%p order:%u " - "compound_mapcount:%d compound_pincount:%d\n", - page, page_ref_count(head), mapcount, - mapping, page_to_pgoff(page), head, - compound_order(head), compound_mapcount(page), - compound_pincount(page)); + pr_warn("head:%p order:%u compound_mapcount:%d compound_pincount:%d\n", + head, compound_order(head), + compound_mapcount(page), + compound_pincount(page)); } else { - pr_warn("page:%p refcount:%d mapcount:%d mapping:%p " - "index:%#lx head:%p order:%u " - "compound_mapcount:%d\n", - page, page_ref_count(head), mapcount, - mapping, page_to_pgoff(page), head, - compound_order(head), compound_mapcount(page)); + pr_warn("head:%p order:%u compound_mapcount:%d\n", + head, compound_order(head), + compound_mapcount(page)); } - else - pr_warn("page:%p refcount:%d mapcount:%d mapping:%p index:%#lx\n", - page, page_ref_count(page), mapcount, - mapping, page_to_pgoff(page)); + } + if (PageKsm(page)) type = "ksm "; else if (PageAnon(page)) I haven't dumped a page with this yet, so I may change my mind, but this seems like a good reduction in the amount of redundant code in this function.