Re: [PATCH 0/3] Improvements for dump_page()

[John Hubbard <jhubbard@nvidia.com>]

On 2020-06-29 08:19, Matthew Wilcox (Oracle) wrote:
> I've been looking at a lot of calls to dump_page() recently due to the
> THP work, and these three patches make my life easier.  Please consider
> applying.
> 
> Matthew Wilcox (Oracle) (3):
>    mm: Print head flags in dump_page
>    mm: Print the inode number in dump_page
>    mm: Print hashed address of struct page
> 
>   mm/debug.c | 22 ++++++++++++----------
>   1 file changed, 12 insertions(+), 10 deletions(-)


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));
  		}
  	else
  		pr_warn("page:%p refcount:%d mapcount:%d mapping:%p index:%#lx\n",



thanks,
-- 
John Hubbard
NVIDIA