Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: William Kucharski <william.kucharski@oracle.com>
To: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: linux-mm <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	John Hubbard <jhubbard@nvidia.com>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Mike Rapoport <rppt@linux.ibm.com>
Subject: Re: [PATCH v2 0/6] Improvements for dump_page()
Date: Thu, 9 Jul 2020 14:54:53 -0600
Message-ID: <9A372D8D-73B5-4B6F-82BB-07926E39E555@oracle.com> (raw)
In-Reply-To: <20200709202117.7216-1-willy@infradead.org>

Just a formatting suggestion:

> On Jul 9, 2020, at 2:21 PM, Matthew Wilcox (Oracle) <willy@infradead.org> wrote:
> 
> Thanks to everyone for your feedback on v1.  I haven't added any acks
> from v1 because most of the patches have changed substantially from v1.
> 
> Here's a sample dump of a pagecache tail page with all of the patches applied:
> 
> page:000000006d1c49ca refcount:6 mapcount:0 mapping:00000000136b8d90 index:0x109 pfn:0x6c645
> head:000000008bd38076 order:2 compound_mapcount:0 compound_pincount:0
> aops:xfs_address_space_operations ino:800042 dentry name:"fd"
> flags: 0x4000000000012014(uptodate|lru|private|head)
> raw: 4000000000000000 ffffd46ac1b19101 ffffffff00000202 dead000000000004
> raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000
> head: 4000000000012014 ffffd46ac1b1bbc8 ffffd46ac1b1bc08 ffff91976f659560
> head: 0000000000000108 ffff919773220680 00000006ffffffff 0000000000000000
> page dumped because: testing
> 
> Matthew Wilcox (Oracle) (6):
>  mm: Handle page->mapping better in dump_page
>  mm: Dump compound page information on a second line
>  mm: Print head flags in dump_page
>  mm: Switch dump_page to get_kernel_nofault
>  mm: Print the inode number in dump_page
>  mm: Print hashed address of struct page
> 
> mm/debug.c | 75 +++++++++++++++++++++++++-----------------------------
> 1 file changed, 35 insertions(+), 40 deletions(-)

As this output is meant to be taken as a block, it might be more legible
if each line after the first was indented by a space to make that instantly
clear by sight (as git does when generating the list of changed files above)
and perhaps add a prefix/suffix to make the "dumped because" line stand out
a bit more.

A space between the value and flags in the "flags:" would also be nice.

This would then look something like:

page:000000006d1c49ca refcount:6 mapcount:0 mapping:00000000136b8d90 index:0x109 pfn:0x6c645
 head:000000008bd38076 order:2 compound_mapcount:0 compound_pincount:0
 aops:xfs_address_space_operations ino:800042 dentry name:"fd"
 flags: 0x4000000000012014 (uptodate|lru|private|head)
 raw: 4000000000000000 ffffd46ac1b19101 ffffffff00000202 dead000000000004
 raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000
 head: 4000000000012014 ffffd46ac1b1bbc8 ffffd46ac1b1bc08 ffff91976f659560
 head: 0000000000000108 ffff919773220680 00000006ffffffff 0000000000000000
 >> page dumped because: testing <<

However I realize this would mean making changes to things you haven't modified
as part of this patch, so feel free to put my suggestion on the back burner
for now if you like.

      parent reply index

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-09 20:21 Matthew Wilcox (Oracle)
2020-07-09 20:21 ` [PATCH v2 1/6] mm: Handle page->mapping better in dump_page Matthew Wilcox (Oracle)
2020-07-11  1:30   ` John Hubbard
2020-07-11  1:32     ` John Hubbard
2020-07-14 11:06   ` Vlastimil Babka
2020-07-09 20:21 ` [PATCH v2 2/6] mm: Dump compound page information on a second line Matthew Wilcox (Oracle)
2020-07-11  1:35   ` John Hubbard
2020-07-14 12:03   ` Vlastimil Babka
2020-08-04 15:37   ` Qian Cai
2020-08-04 16:14     ` Matthew Wilcox
2020-08-04 18:39       ` [PATCH] mm, dump_page: do not crash with bad compound_page() John Hubbard
2020-08-04 18:48         ` Matthew Wilcox
2020-08-04 19:17           ` John Hubbard
2020-08-04 19:42             ` Matthew Wilcox
2020-07-09 20:21 ` [PATCH v2 3/6] mm: Print head flags in dump_page Matthew Wilcox (Oracle)
2020-07-11  1:40   ` John Hubbard
2020-07-14 12:06   ` Vlastimil Babka
2020-07-09 20:21 ` [PATCH v2 4/6] mm: Switch dump_page to get_kernel_nofault Matthew Wilcox (Oracle)
2020-07-14 12:11   ` Vlastimil Babka
2020-07-09 20:21 ` [PATCH v2 5/6] mm: Print the inode number in dump_page Matthew Wilcox (Oracle)
2020-07-11  2:04   ` John Hubbard
2020-07-14 12:18   ` Vlastimil Babka
2020-07-09 20:21 ` [PATCH v2 6/6] mm: Print hashed address of struct page Matthew Wilcox (Oracle)
2020-07-11  1:48   ` John Hubbard
2020-07-09 20:54 ` [PATCH v2 0/6] Improvements for dump_page() Mike Rapoport
2020-07-09 20:54 ` William Kucharski [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9A372D8D-73B5-4B6F-82BB-07926E39E555@oracle.com \
    --to=william.kucharski@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=jhubbard@nvidia.com \
    --cc=kirill@shutemov.name \
    --cc=linux-mm@kvack.org \
    --cc=rppt@linux.ibm.com \
    --cc=vbabka@suse.cz \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git