Re: [PATCH] mm: be more verbose for alloc_contig_range faliures

From: Minchan Kim <minchan@kernel.org>
To: Michal Hocko <mhocko@suse.com>
Cc: David Hildenbrand <david@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	joaodias@google.com
Subject: Re: [PATCH] mm: be more verbose for alloc_contig_range faliures
Date: Mon, 8 Mar 2021 09:01:10 -0800	[thread overview]
Message-ID: <YEZYVkWV+qygPkT3@google.com> (raw)
In-Reply-To: <YEZPG+9ql2j8O9QS@dhcp22.suse.cz>

On Mon, Mar 08, 2021 at 05:21:47PM +0100, Michal Hocko wrote:
> On Mon 08-03-21 07:58:11, Minchan Kim wrote:
> [...]
> > It's the dynamic debugging facility
> > to enable only when admin want to use it. Otherwise, it's nop
> > unless is't not enabled. Furthermore, it doesn't need to invent
> > custom dump_page implementation(including dump_page_owner) by
> > chaning pr_debug.
> > Could you clarify your requirement?
> > 
> > https://lore.kernel.org/linux-mm/YEEUq8ZRn4WyYWVx@google.com/
> 
> I am not really sure this is the right way to enable dynamic logging.
> Maybe it is. I thought we can go with something as simple as pr_debug.
> You are right that we do not have dump_page with the kernel log level.
> This is rather annoying but a) do we need a full dump_page functionality

Most parts I take care of are

        pr_warn("page:%p refcount:%d mapcount:%d mapping:%p index:%#lx pfn:%#lx\n",
                        page, page_ref_count(head), mapcount, mapping,
                        page_to_pgoff(page), page_to_pfn(page));

        pr_warn("%sflags: %#lx(%pGp)%s\n", type, head->flags, &head->flags,
                page_cma ? " CMA" : "");

And dump_page_owner which was super helpful to find GUP places.

Looks like most of dump_pages.

> and b) can we make it log level aware with the dynamic debug
> infrastructure preserved? If not then then an explicit handling is

If we could make aware of loglevel, we need to enable each by each IOW.
IOW, what we want to enable is mm/page_alloc.c #1492 line, for example.

However, we should enable 
  mm/debug.c # 88 line
  mm/debug.c # 102 line
  mm/debug.c # 121 line
mm/page_owner.c # 448 line
mm/page_owner.c # 450 line
kernel/stacktrace.c #32 line

And so on. Furthermore, user should be aware of that how the kernel code
is changed for those all sites and reconfigure and follow new added
code.

So, I choosed explict handling.

> probably the only way and this should be reviewed by people who are more
> familiar with that framework than me.

> -- 
> Michal Hocko
> SUSE Labs