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

[John Hubbard <jhubbard@nvidia.com>]

On 2020-06-29 15:35, Matthew Wilcox wrote:
...
> 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.
> 

That is a much better fix. I hated myself for the code duplication there. :)
I've pasted in a sample run in the other thread (patch 1/3).


thanks,
-- 
John Hubbard
NVIDIA